Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use explicit exceptions when cancelling listeners #238

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions tests/api/test_listeners.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import zigpy_znp.types as t
import zigpy_znp.commands as c
from zigpy_znp.api import OneShotResponseListener, CallbackResponseListener
from zigpy_znp.exceptions import ShuttingDown


async def test_resolve(event_loop, mocker):
Expand Down Expand Up @@ -34,24 +35,24 @@ async def test_resolve(event_loop, mocker):
assert (await future) == match

# Cancelling a callback will have no effect
assert not callback_listener.cancel()
callback_listener.set_exception(RuntimeError())

# Cancelling a one-shot listener does not throw any errors
assert one_shot_listener.cancel()
assert one_shot_listener.cancel()
assert one_shot_listener.cancel()
one_shot_listener.set_exception(RuntimeError())
one_shot_listener.set_exception(RuntimeError())
one_shot_listener.set_exception(RuntimeError())


async def test_cancel(event_loop):
# Cancelling a one-shot listener prevents it from being fired
future = event_loop.create_future()
one_shot_listener = OneShotResponseListener([c.SYS.Ping.Rsp(partial=True)], future)
one_shot_listener.cancel()
one_shot_listener.set_exception(RuntimeError())

match = c.SYS.Ping.Rsp(Capabilities=t.MTCapabilities.SYS)
assert not one_shot_listener.resolve(match)

with pytest.raises(asyncio.CancelledError):
with pytest.raises(RuntimeError):
await future


Expand Down Expand Up @@ -95,7 +96,7 @@ async def test_api_cancel_listeners(connected_znp, mocker):
assert not future.done()
znp.close()

with pytest.raises(asyncio.CancelledError):
with pytest.raises(ShuttingDown):
await future

# add_done_callback won't be executed immediately
Expand Down
8 changes: 6 additions & 2 deletions zigpy_znp/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@
CallbackResponseListener,
)
from zigpy_znp.frames import GeneralFrame
from zigpy_znp.exceptions import CommandNotRecognized, InvalidCommandResponse
from zigpy_znp.exceptions import (
ShuttingDown,
CommandNotRecognized,
InvalidCommandResponse,
)
from zigpy_znp.types.nvids import ExNvIds, OsalNvIds

if typing.TYPE_CHECKING:
Expand Down Expand Up @@ -774,7 +778,7 @@ def close(self) -> None:

for _header, listeners in self._listeners.items():
for listener in listeners:
listener.cancel()
listener.set_exception(ShuttingDown())

self._listeners.clear()
self.version = None
Expand Down
6 changes: 5 additions & 1 deletion zigpy_znp/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from zigpy.exceptions import DeliveryError
from zigpy.exceptions import DeliveryError, ControllerException


class InvalidFrame(ValueError):
Expand All @@ -17,3 +17,7 @@ class InvalidCommandResponse(DeliveryError):
def __init__(self, message, response):
super().__init__(message)
self.response = response


class ShuttingDown(ControllerException):
pass
12 changes: 5 additions & 7 deletions zigpy_znp/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def _resolve(self, response: t.CommandBase) -> bool:

raise NotImplementedError() # pragma: no cover

def cancel(self):
def set_exception(self, exc: BaseException) -> None:
"""
Implement by subclasses to cancel the listener.

Expand Down Expand Up @@ -118,11 +118,9 @@ def _resolve(self, response: t.CommandBase) -> bool:
self.future.set_result(response)
return True

def cancel(self):
def set_exception(self, exc: BaseException) -> None:
if not self.future.done():
self.future.cancel()

return True
self.future.set_exception(exc)


@dataclasses.dataclass(frozen=True)
Expand All @@ -149,9 +147,9 @@ def _resolve(self, response: t.CommandBase) -> bool:
# Callbacks are always resolved
return True

def cancel(self):
def set_exception(self, exc: BaseException) -> None:
# You can't cancel a callback
return False
pass


class CatchAllResponse:
Expand Down
Loading