Skip to content

Commit ee4c50a

Browse files
committed
Make event emitter log.
This is a design regression, but in the interest of satisfying the HA code bot's requirements I feel I have to. Preventing the event handler from chaining exceptions is not as clean design, especially considering there should never be any exceptions leaking from event handlers in the first place.
1 parent 8088ad5 commit ee4c50a

5 files changed

Lines changed: 144 additions & 12 deletions

File tree

‎.github/workflows/checks.yml‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,6 @@ jobs:
2222
run: |
2323
pip install -r requirements.test.txt
2424
25-
- name: Run mypy checks
26-
run: |
27-
python3 -m mypy --show-error-codes --show-column-numbers src
28-
29-
- name: Run pytest checks
25+
- name: Run lints and tests
3026
run: |
3127
scripts/run-tests.sh

‎requirements.test.txt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
mypy
22
pytest
3+
pytest-asyncio
34
pytest-coverage
4-
zeroconf
5+
zeroconf

‎scripts/run-tests.sh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ export PYTEST_COVERAGE_DATA_FILE="${testsdir}/.coverage"
1010
extra=
1111
[ -z "$@" ] && extra="--cov-fail-under=100"
1212

13+
echo "Linting src..."
14+
python3 -m mypy --show-error-codes --show-column-numbers "${rootdir}/src"
15+
1316
echo "Linting tests..."
1417
mypy "${testsdir}"
1518

‎src/powersensor_local/async_event_emitter.py‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,14 @@
11
"""Small helper class for pub/sub functionality with async handlers."""
2+
import logging
23
from typing import Callable
34

45
class AsyncEventEmitter:
5-
"""Small helper class for pub/sub functionality with async handlers."""
6-
def __init__(self):
7-
self._listeners = {}
6+
"""Small helper class for pub/sub functionality with async handlers.
7+
An optional Logger can be provided, which will be used to log any
8+
unhandled exceptions."""
9+
def __init__(self, logger: logging.Logger | None = None):
10+
self._listeners: dict[str,list[Callable]] = {}
11+
self._logger = logger
812

913
def subscribe(self, event_name: str, callback: Callable):
1014
"""Registers an event handler for the given event key. The handler must
@@ -26,11 +30,22 @@ async def emit(self, event_name: str, *args):
2630
Additional arguments may be supplied with event as appropriate. Each
2731
event handler is awaited before delivering the event to the next.
2832
If an event handler raises an exception, this is funneled through
29-
to an 'exception' event being emitted. This can chain."""
33+
to an 'exception' event being emitted. If no 'exception' listener
34+
is registered, or an exception handler callback raises an exception,
35+
the exception is logged (if a logger was provided), and discarded."""
3036
if self._listeners.get(event_name) is None:
3137
return
3238
for callback in self._listeners[event_name]:
3339
try:
3440
await callback(event_name, *args)
35-
except BaseException as e: # pylint: disable=W0718
36-
await self.emit('exception', e)
41+
except Exception as e:
42+
if 'exception' not in self._listeners:
43+
if self._logger is not None:
44+
self._logger.exception(f"Discarding unhandled exception: {e}")
45+
else:
46+
for handler in self._listeners['exception']:
47+
try:
48+
await handler('exception', e)
49+
except Exception as e2:
50+
if self._logger is not None:
51+
self._logger.exception(f"Exception handling callback raised an exception itself, discarding it: {e2}")

‎tests/test_async_event_emitter.py‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,117 @@
1+
import pytest
2+
from powersensor_local.async_event_emitter import AsyncEventEmitter
3+
from unittest.mock import AsyncMock, MagicMock
4+
5+
@pytest.fixture
6+
def emitter():
7+
return AsyncEventEmitter()
8+
9+
@pytest.mark.asyncio
10+
async def test_basics(emitter):
11+
mock = AsyncMock()
12+
# Ensure it calls it
13+
emitter.subscribe('test', mock)
14+
await emitter.emit('test')
15+
mock.assert_called_once()
16+
# Ensure it can call it again
17+
await emitter.emit('test')
18+
assert(mock.call_count == 2)
19+
# Ensure it doesn't get called after unsubscribe
20+
emitter.unsubscribe('test', mock)
21+
await emitter.emit('test')
22+
assert(mock.call_count == 2)
23+
24+
25+
@pytest.mark.asyncio
26+
async def test_multiple_listeners(emitter):
27+
mock1 = AsyncMock()
28+
mock2 = AsyncMock()
29+
emitter.subscribe('x', mock1)
30+
emitter.subscribe('x', mock2)
31+
await emitter.emit('x')
32+
mock1.assert_called_once()
33+
mock2.assert_called_once()
34+
35+
36+
@pytest.mark.asyncio
37+
async def test_different_events(emitter):
38+
mock1 = AsyncMock()
39+
mock2 = AsyncMock()
40+
emitter.subscribe('x', mock1)
41+
emitter.subscribe('y', mock2)
42+
await emitter.emit('x')
43+
assert(mock1.call_count == 1)
44+
assert(mock2.call_count == 0)
45+
await emitter.emit('y')
46+
assert(mock1.call_count == 1)
47+
assert(mock2.call_count == 1)
48+
49+
50+
@pytest.mark.asyncio
51+
async def test_argument_passing(emitter):
52+
mock = AsyncMock()
53+
emitter.subscribe('test', mock)
54+
await emitter.emit('test', 1, 'two', 3.01)
55+
mock.assert_called_with('test', 1, 'two', 3.01)
56+
57+
58+
@pytest.mark.asyncio
59+
async def test_exception_unhandled():
60+
logger = MagicMock()
61+
emitter = AsyncEventEmitter(logger)
62+
mock = AsyncMock()
63+
emitter.subscribe('e', mock)
64+
mock.side_effect = KeyError('oops')
65+
await emitter.emit('e')
66+
mock.assert_called_once()
67+
logger.exception.assert_called_once_with("Discarding unhandled exception: 'oops'")
68+
69+
70+
@pytest.mark.asyncio
71+
async def test_exception_handler(emitter):
72+
mock = AsyncMock()
73+
emitter.subscribe('e', mock)
74+
e = KeyError('oops')
75+
mock.side_effect = e
76+
mock_exc = AsyncMock()
77+
emitter.subscribe('exception', mock_exc)
78+
await emitter.emit('e')
79+
mock.assert_called_once()
80+
mock_exc.assert_called_once_with('exception', e)
81+
82+
83+
@pytest.mark.asyncio
84+
async def test_exception_handler_exception():
85+
logger = MagicMock()
86+
emitter = AsyncEventEmitter(logger)
87+
mock = AsyncMock()
88+
emitter.subscribe('e', mock)
89+
mock.side_effect = KeyError('oops')
90+
mock_exc = AsyncMock()
91+
emitter.subscribe('exception', mock_exc)
92+
mock_exc.side_effect = ValueError('doh')
93+
await emitter.emit('e')
94+
mock.assert_called_once()
95+
mock_exc.assert_called_once()
96+
logger.exception.assert_called_once_with("Exception handling callback raised an exception itself, discarding it: doh")
97+
98+
99+
@pytest.mark.asyncio
100+
async def test_multiple_exception_handlers():
101+
logger = MagicMock()
102+
emitter = AsyncEventEmitter(logger)
103+
trigger = AsyncMock()
104+
e = ValueError('overflow')
105+
trigger.side_effect = e
106+
emitter.subscribe('e', trigger)
107+
handlers = [ AsyncMock() for _ in range(5) ]
108+
for handler in handlers:
109+
emitter.subscribe('exception', handler)
110+
bad_handlers = handlers[1::2] # pick every other
111+
for handler in bad_handlers:
112+
handler.side_effect = e
113+
await emitter.emit('e')
114+
trigger.assert_called_once()
115+
for handler in handlers:
116+
handler.assert_called_once()
117+
assert(logger.exception.call_count == len(bad_handlers))

0 commit comments

Comments
 (0)