Repository navigation
Fix awaiting an event handler from a background task: caught exceptions and finished superseding runs #7907
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Awaiting an event handler from a background task now behaves like the inline call: an exception the task catches is no longer also reported to the backend exception handler (no error toast), and a `supersedes=True` handler run that finished before a newer one superseded it returns its result instead of raising `CancelledError`. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5582,3 +5582,131 @@ def ping(self): | |
| BaseStateToken(ident=token, cls=State) | ||
| ) | ||
| assert (await state.get_state(CancelInsideState)).log == ["ping"] | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| ("catches", "expected_results", "expected_reported"), | ||
| [ | ||
| (True, ["ValueError('boom')"], []), | ||
| (False, [], ["ValueError('boom')"]), | ||
| ], | ||
| ) | ||
| async def test_exception_of_handler_awaited_from_background_is_the_callers( | ||
| wired_app: App, | ||
| real_base_state_processor: BaseStateEventProcessor, | ||
| token: str, | ||
| catches: bool, | ||
| expected_results: list[str], | ||
| expected_reported: list[str], | ||
| ): | ||
| """An awaited handler's exception reaches the backend handler only if uncaught. | ||
|
|
||
| The background task awaits with no lock held, so the handler is enqueued. | ||
| Its exception is raised from the await like the inline call's: caught, it | ||
| is not reported; uncaught, the background task's failure reports it once. | ||
|
|
||
| Args: | ||
| wired_app: The App wired to the processor's state manager. | ||
| real_base_state_processor: The unmocked BaseStateEventProcessor. | ||
| token: The client token. | ||
| catches: Whether the background task catches the exception. | ||
| expected_results: The exceptions the background task caught. | ||
| expected_reported: The exceptions the backend exception handler got. | ||
| """ | ||
| results: list[str] = [] | ||
| reported: list[str] = [] | ||
|
|
||
| class BgBoomState(State): | ||
| total: int = 0 | ||
|
|
||
| @event | ||
| def boom(self): | ||
| self.total += 1000 | ||
| msg = "boom" | ||
| raise ValueError(msg) | ||
|
|
||
| @event(background=True) | ||
| async def run(self, catches: bool): | ||
| try: | ||
| await BgBoomState.boom() | ||
| except ValueError as ex: | ||
| if not catches: | ||
| raise | ||
| results.append(repr(ex)) | ||
|
|
||
| real_base_state_processor.backend_exception_handler = lambda ex: reported.append( | ||
| repr(ex) | ||
| ) | ||
| async with real_base_state_processor as processor: | ||
| future = await processor.enqueue( | ||
| token, Event.from_event_type(BgBoomState.run(catches))[0] | ||
| ) | ||
| with contextlib.suppress(ValueError): | ||
| await future.wait_all() | ||
|
|
||
| assert results == expected_results | ||
| assert reported == expected_reported | ||
| async with _read_back(real_base_state_processor, token) as root: | ||
| assert (await root.get_state(BgBoomState)).total == 1000 | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| "processor_state_manager", ["in_process", "redis"], indirect=True | ||
| ) | ||
| async def test_superseding_handler_awaited_from_background_returns_finished_result( | ||
| wired_app: App, | ||
| real_base_state_processor: BaseStateEventProcessor, | ||
| token: str, | ||
| ): | ||
| """A background task gets the result of a superseding handler run that finished. | ||
|
|
||
| The second task waits for the state lock the first search holds, and | ||
| supersedes it as soon as that search has returned. With the in-memory | ||
| manager that is before the first search's future was resolved. | ||
|
|
||
| Args: | ||
| wired_app: The App wired to the processor's state manager. | ||
| real_base_state_processor: The unmocked BaseStateEventProcessor. | ||
| token: The client token. | ||
| """ | ||
| results: list[str] = [] | ||
| started = asyncio.Event() | ||
| release = asyncio.Event() | ||
|
|
||
| class BgSearchState(State): | ||
| query: str = "" | ||
|
|
||
| @event(supersedes=True) | ||
| async def search(self, q: str) -> str: | ||
| if q == "first": | ||
| started.set() | ||
| await release.wait() | ||
| self.query = q | ||
| return q.upper() | ||
|
|
||
| @event(background=True) | ||
| async def run(self, q: str): | ||
| try: | ||
| result = await BgSearchState.search(q) | ||
| except asyncio.CancelledError: | ||
| result = "cancelled" | ||
| results.append(f"{q} -> {result}") | ||
|
|
||
| async with real_base_state_processor as processor: | ||
| first = await processor.enqueue( | ||
| token, Event.from_event_type(BgSearchState.run("first"))[0] | ||
| ) | ||
| await asyncio.wait_for(started.wait(), timeout=5) | ||
| second = await processor.enqueue( | ||
| token, Event.from_event_type(BgSearchState.run("second"))[0] | ||
| ) | ||
| # Let the second task start and queue for the lock the search holds. | ||
| await asyncio.sleep(0.1) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P2: This fixed sleep does not guarantee that the second background run has reached the superseding enqueue, so the test can pass without exercising the race it claims to cover. Synchronize on an event set by the second run immediately before it awaits Prompt for AI agents |
||
| release.set() | ||
| await asyncio.wait_for( | ||
| asyncio.gather(first.wait_all(), second.wait_all()), timeout=5 | ||
| ) | ||
|
|
||
| assert results == ["first -> FIRST", "second -> SECOND"] | ||
| async with _read_back(real_base_state_processor, token) as root: | ||
| assert (await root.get_state(BgSearchState)).query == "second" | ||
Uh oh!
There was an error while loading. Please reload this page.