Raise only on return_response=True

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"]
 

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