Fix debouncer not scheduling timer when wrapped function raises (#94689)

* Fix debouncer not scheduling timer when callback function raises

* test coro as well

* preen

homeassistant/helpers/debounce.py

@@ -90,11 +90,11 @@ class Debouncer(Generic[_R_co]):
             if self._timer_task:
                 return
 
-            task = self.hass.async_run_hass_job(self._job)
-            if task:
-                await task
-
-            self._schedule_timer()
+            try:
+                if task := self.hass.async_run_hass_job(self._job):
+                    await task
+            finally:
+                self._schedule_timer()
 
     async def _handle_timer_finish(self) -> None:
         """Handle a finished timer."""
@@ -112,14 +112,13 @@ class Debouncer(Generic[_R_co]):
                 return
 
             try:
-                task = self.hass.async_run_hass_job(self._job)
-                if task:
+                if task := self.hass.async_run_hass_job(self._job):
                     await task
             except Exception:  # pylint: disable=broad-except
                 self.logger.exception("Unexpected exception from %s", self.function)
-
-            # Schedule a new timer to prevent new runs during cooldown
-            self._schedule_timer()
+            finally:
+                # Schedule a new timer to prevent new runs during cooldown
+                self._schedule_timer()
 
     async def async_shutdown(self) -> None:
         """Cancel any scheduled call, and prevent new runs."""

tests/helpers/test_debounce.py

@@ -6,7 +6,7 @@ from unittest.mock import AsyncMock
 
 import pytest
 
-from homeassistant.core import HomeAssistant
+from homeassistant.core import HomeAssistant, callback
 from homeassistant.helpers import debounce
 from homeassistant.util.dt import utcnow
 
@@ -69,6 +69,133 @@ async def test_immediate_works(hass: HomeAssistant) -> None:
     assert debouncer._job.target == debouncer.function
 
 
+async def test_immediate_works_with_passed_callback_function_raises(
+    hass: HomeAssistant,
+) -> None:
+    """Test immediate works with a callback function that raises."""
+    calls = []
+
+    @callback
+    def _append_and_raise() -> None:
+        calls.append(None)
+        raise RuntimeError("forced_raise")
+
+    debouncer = debounce.Debouncer(
+        hass,
+        _LOGGER,
+        cooldown=0.01,
+        immediate=True,
+        function=_append_and_raise,
+    )
+
+    # Call when nothing happening
+    with pytest.raises(RuntimeError, match="forced_raise"):
+        await debouncer.async_call()
+    assert len(calls) == 1
+    assert debouncer._timer_task is not None
+    assert debouncer._execute_at_end_of_timer is False
+    assert debouncer._job.target == debouncer.function
+
+    # Call when cooldown active setting execute at end to True
+    await debouncer.async_call()
+    assert len(calls) == 1
+    assert debouncer._timer_task is not None
+    assert debouncer._execute_at_end_of_timer is True
+    assert debouncer._job.target == debouncer.function
+
+    # Canceling debounce in cooldown
+    debouncer.async_cancel()
+    assert debouncer._timer_task is None
+    assert debouncer._execute_at_end_of_timer is False
+    assert debouncer._job.target == debouncer.function
+
+    before_job = debouncer._job
+
+    # Call and let timer run out
+    with pytest.raises(RuntimeError, match="forced_raise"):
+        await debouncer.async_call()
+    assert len(calls) == 2
+    async_fire_time_changed(hass, utcnow() + timedelta(seconds=1))
+    await hass.async_block_till_done()
+    assert len(calls) == 2
+    assert debouncer._timer_task is None
+    assert debouncer._execute_at_end_of_timer is False
+    assert debouncer._job.target == debouncer.function
+    assert debouncer._job == before_job
+
+    # Test calling doesn't execute/cooldown if currently executing.
+    await debouncer._execute_lock.acquire()
+    await debouncer.async_call()
+    assert len(calls) == 2
+    assert debouncer._timer_task is None
+    assert debouncer._execute_at_end_of_timer is False
+    debouncer._execute_lock.release()
+    assert debouncer._job.target == debouncer.function
+
+
+async def test_immediate_works_with_passed_coroutine_raises(
+    hass: HomeAssistant,
+) -> None:
+    """Test immediate works with a coroutine that raises."""
+    calls = []
+
+    async def _append_and_raise() -> None:
+        calls.append(None)
+        raise RuntimeError("forced_raise")
+
+    debouncer = debounce.Debouncer(
+        hass,
+        _LOGGER,
+        cooldown=0.01,
+        immediate=True,
+        function=_append_and_raise,
+    )
+
+    # Call when nothing happening
+    with pytest.raises(RuntimeError, match="forced_raise"):
+        await debouncer.async_call()
+    assert len(calls) == 1
+    assert debouncer._timer_task is not None
+    assert debouncer._execute_at_end_of_timer is False
+    assert debouncer._job.target == debouncer.function
+
+    # Call when cooldown active setting execute at end to True
+    await debouncer.async_call()
+    assert len(calls) == 1
+    assert debouncer._timer_task is not None
+    assert debouncer._execute_at_end_of_timer is True
+    assert debouncer._job.target == debouncer.function
+
+    # Canceling debounce in cooldown
+    debouncer.async_cancel()
+    assert debouncer._timer_task is None
+    assert debouncer._execute_at_end_of_timer is False
+    assert debouncer._job.target == debouncer.function
+
+    before_job = debouncer._job
+
+    # Call and let timer run out
+    with pytest.raises(RuntimeError, match="forced_raise"):
+        await debouncer.async_call()
+    assert len(calls) == 2
+    async_fire_time_changed(hass, utcnow() + timedelta(seconds=1))
+    await hass.async_block_till_done()
+    assert len(calls) == 2
+    assert debouncer._timer_task is None
+    assert debouncer._execute_at_end_of_timer is False
+    assert debouncer._job.target == debouncer.function
+    assert debouncer._job == before_job
+
+    # Test calling doesn't execute/cooldown if currently executing.
+    await debouncer._execute_lock.acquire()
+    await debouncer.async_call()
+    assert len(calls) == 2
+    assert debouncer._timer_task is None
+    assert debouncer._execute_at_end_of_timer is False
+    debouncer._execute_lock.release()
+    assert debouncer._job.target == debouncer.function
+
+
 async def test_not_immediate_works(hass: HomeAssistant) -> None:
     """Test immediate works."""
     calls = []