Add back a timeout for service calls run from scripts

homeassistant/helpers/script.py

@@ -137,7 +137,7 @@
 _TIMEOUT_MSG = "Timeout reached, abort script."
 
 _SHUTDOWN_MAX_WAIT = 60
-
+_SERVICE_CALL_LIMIT = 10
 
 ACTION_TRACE_NODE_MAX_LEN = 20  # Max length of a trace node for repeated actions
 
@@ -634,20 +634,24 @@
                 task.cancel()
             unsub()
 
-    async def _async_run_long_action(self, long_task: asyncio.Task[_T]) -> _T | None:
+    async def _async_run_long_action(
+        self, long_task: asyncio.Task[_T], timeout: float | None = None
+    ) -> _T | None:
         """Run a long task while monitoring for stop request."""
 
         async def async_cancel_long_task() -> None:
             # Stop long task and wait for it to finish.
             long_task.cancel()
-            with suppress(Exception):
+            with suppress(Exception, asyncio.CancelledError):
                 await long_task
 
         # Wait for long task while monitoring for a stop request.
         stop_task = self._hass.async_create_task(self._stop.wait())
         try:
             await asyncio.wait(
-                {long_task, stop_task}, return_when=asyncio.FIRST_COMPLETED
+                {long_task, stop_task},
+                return_when=asyncio.FIRST_COMPLETED,
+                timeout=timeout,
             )
         # If our task is cancelled, then cancel long task, too. Note that if long task
         # is cancelled otherwise the CancelledError exception will not be raised to
@@ -656,16 +660,18 @@
             await async_cancel_long_task()
             raise
         finally:
+            stopped = stop_task.done()
             stop_task.cancel()
-
         if long_task.cancelled():
             raise asyncio.CancelledError
         if long_task.done():
             # Propagate any exceptions that occurred.
             return long_task.result()
-        # Stopped before long task completed, so cancel it.
+        # Stopped or timed out before long task completed, so cancel it.
         await async_cancel_long_task()
-        return None
+        if stopped:
+            return None
+        raise asyncio.TimeoutError("Timeout while running task")
 
     async def _async_call_service_step(self):
         """Call the service specified in the action."""
@@ -699,6 +705,12 @@
             and params[CONF_SERVICE] == "trigger"
             or params[CONF_DOMAIN] in ("python_script", "script")
         )
+        # If this might start a script then disable the call timeout.
+        # Otherwise use the normal service call limit.
+        if running_script:
+            limit = None
+        else:
+            limit = _SERVICE_CALL_LIMIT
         trace_set_result(params=params, running_script=running_script)
         response_data = await self._async_run_long_action(
             self._hass.async_create_task(
@@ -709,6 +721,7 @@
                     return_response=return_response,
                 )
             ),
+            timeout=limit,
         )
         if response_variable:
             self._variables[response_variable] = response_data

tests/helpers/test_script.py

@@ -561,6 +561,59 @@ def heard_event_cb(event):
     assert len(calls) == 4
 
 
+@patch("homeassistant.helpers.script._SERVICE_CALL_LIMIT", 0.1)
+async def test_service_call_timeout(hass: HomeAssistant) -> None:
+    """Test a service call that takes longer than the timeout."""
+
+    async def async_simulate_long_service(service):
+        """Service that takes not insignificant time."""
+        await asyncio.sleep(10)
+
+    hass.services.async_register("test", "script", async_simulate_long_service)
+
+    sequence = cv.SCRIPT_SCHEMA(
+        [
+            {
+                "service": "test.script",
+                "data_template": {"fire": "{{ fire1 }}"},
+            },
+        ]
+    )
+    script_obj = script.Script(
+        hass, sequence, "Test Name", "test_domain", script_mode="parallel", max_runs=2
+    )
+
+    # The patch at the start of the function sets the service call timeout to
+    # something much smaller than the sleep in the service call.
+    with pytest.raises(asyncio.TimeoutError):
+        await hass.async_create_task(
+            script_obj.async_run(
+                MappingProxyType({"fire1": "1"}),
+                Context(),
+            )
+        )
+
+    assert_action_trace(
+        {
+            "0": [
+                {
+                    "result": {
+                        "params": {
+                            "domain": "test",
+                            "service": "script",
+                            "service_data": {"fire": 1},
+                            "target": {},
+                        },
+                        "running_script": False,
+                    },
+                    "error_type": asyncio.TimeoutError,
+                }
+            ],
+        },
+        expected_script_execution="error",
+    )
+
+
 async def test_activating_scene(
     hass: HomeAssistant, caplog: pytest.LogCaptureFixture
 ) -> None: