From 26be0fab78293ee5732a4fd08fa0b5bd1aec9aba Mon Sep 17 00:00:00 2001 From: "J. Nick Koston" Date: Thu, 15 Jun 2023 16:15:49 -1000 Subject: [PATCH] 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 | 19 +++-- tests/helpers/test_debounce.py | 129 +++++++++++++++++++++++++++++- 2 files changed, 137 insertions(+), 11 deletions(-) diff --git a/homeassistant/helpers/debounce.py b/homeassistant/helpers/debounce.py index 2df8965de34..4e5d152135a 100644 --- a/homeassistant/helpers/debounce.py +++ b/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.""" diff --git a/tests/helpers/test_debounce.py b/tests/helpers/test_debounce.py index 4d36679d538..f31453cfe96 100644 --- a/tests/helpers/test_debounce.py +++ b/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 = []