diff --git a/homeassistant/components/python_script/__init__.py b/homeassistant/components/python_script/__init__.py index 18f0b848956a..690a5bffbecf 100644 --- a/homeassistant/components/python_script/__init__.py +++ b/homeassistant/components/python_script/__init__.py @@ -114,7 +114,7 @@ def discover_scripts(hass): def python_script_service_handler(call: ServiceCall) -> ServiceResponse: """Handle python script service calls.""" - return execute_script(hass, call.service, call.data) + return execute_script(hass, call.service, call.data, call.return_response) existing = hass.services.services.get(DOMAIN, {}).keys() for existing_service in existing: @@ -147,17 +147,17 @@ def discover_scripts(hass): @bind_hass -def execute_script(hass, name, data=None): +def execute_script(hass, name, data=None, return_response=False): """Execute a script.""" filename = f"{name}.py" raise_if_invalid_filename(filename) with open(hass.config.path(FOLDER, filename), encoding="utf8") as fil: source = fil.read() - return execute(hass, filename, source, data) + return execute(hass, filename, source, data, return_response=return_response) @bind_hass -def execute(hass, filename, source, data=None): +def execute(hass, filename, source, data=None, return_response=False): """Execute Python source.""" compiled = compile_restricted_exec(source, filename=filename) @@ -246,11 +246,17 @@ def execute(hass, filename, source, data=None): f"Expected `output` to be a dictionary, was {output_type}" ) except ScriptError as err: - raise ServiceValidationError(f"Error executing script: {err}") from err + if return_response: + raise ServiceValidationError(f"Error executing script: {err}") from err + logger.error("Error executing script: %s", err) + return None except Exception as err: # pylint: disable=broad-except - raise HomeAssistantError( - f"Error executing script ({type(err).__name__}): {err}" - ) from err + if return_response: + raise HomeAssistantError( + f"Error executing script ({type(err).__name__}): {err}" + ) from err + logger.error("Error executing script: %s", err) + return None return restricted_globals["output"] diff --git a/tests/components/python_script/test_init.py b/tests/components/python_script/test_init.py index c5622b9eae6b..ee7fedee0d5c 100644 --- a/tests/components/python_script/test_init.py +++ b/tests/components/python_script/test_init.py @@ -122,26 +122,56 @@ this is not valid Python assert "Error loading script test.py" in caplog.text -async def test_execute_runtime_error(hass: HomeAssistant) -> None: +async def test_execute_runtime_error( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Test compile error logs error.""" + caplog.set_level(logging.ERROR) + source = """ +raise Exception('boom') + """ + + hass.async_add_executor_job(execute, hass, "test.py", source, {}) + await hass.async_block_till_done() + + assert "Error executing script: boom" in caplog.text + + +async def test_execute_runtime_error_with_response(hass: HomeAssistant) -> None: """Test compile error logs error.""" source = """ raise Exception('boom') """ - task = hass.async_add_executor_job(execute, hass, "test.py", source, {}) + task = hass.async_add_executor_job(execute, hass, "test.py", source, {}, True) await hass.async_block_till_done() assert type(task.exception()) == HomeAssistantError assert "Error executing script (Exception): boom" in str(task.exception()) -async def test_accessing_async_methods(hass: HomeAssistant) -> None: +async def test_accessing_async_methods( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Test compile error logs error.""" + caplog.set_level(logging.ERROR) + source = """ +hass.async_stop() + """ + + hass.async_add_executor_job(execute, hass, "test.py", source, {}) + await hass.async_block_till_done() + + assert "Not allowed to access async methods" in caplog.text + + +async def test_accessing_async_methods_with_response(hass: HomeAssistant) -> None: """Test compile error logs error.""" source = """ hass.async_stop() """ - task = hass.async_add_executor_job(execute, hass, "test.py", source, {}) + task = hass.async_add_executor_job(execute, hass, "test.py", source, {}, True) await hass.async_block_till_done() assert type(task.exception()) == ServiceValidationError @@ -165,7 +195,25 @@ logger.info('Logging from inside script: %s %s' % (mydict["a"], mylist[2])) assert "Logging from inside script: 1 3" in caplog.text -async def test_accessing_forbidden_methods(hass: HomeAssistant) -> None: +async def test_accessing_forbidden_methods( + hass: HomeAssistant, caplog: pytest.LogCaptureFixture +) -> None: + """Test compile error logs error.""" + caplog.set_level(logging.ERROR) + + for source, name in { + "hass.stop()": "HomeAssistant.stop", + "dt_util.set_default_time_zone()": "module.set_default_time_zone", + "datetime.non_existing": "module.non_existing", + "time.tzset()": "TimeWrapper.tzset", + }.items(): + caplog.records.clear() + hass.async_add_executor_job(execute, hass, "test.py", source, {}) + await hass.async_block_till_done() + assert f"Not allowed to access {name}" in caplog.text + + +async def test_accessing_forbidden_methods_with_response(hass: HomeAssistant) -> None: """Test compile error logs error.""" for source, name in { "hass.stop()": "HomeAssistant.stop", @@ -173,7 +221,7 @@ async def test_accessing_forbidden_methods(hass: HomeAssistant) -> None: "datetime.non_existing": "module.non_existing", "time.tzset()": "TimeWrapper.tzset", }.items(): - task = hass.async_add_executor_job(execute, hass, "test.py", source, {}) + task = hass.async_add_executor_job(execute, hass, "test.py", source, {}, True) await hass.async_block_till_done() assert type(task.exception()) == ServiceValidationError