GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — gh-159303: Run asyncio's default exception handler in the task context by drakeo338 · Pull Request #159307 · python/cpython Conversation
|
Documentation build overview
166 files changed ·
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
I checked this out locally: test_base_events passes, and the two new tests fail on main as described. The move into _get_exception_context() is a straight refactor of the existing lookup.
One behaviour change worth documenting: a custom handler used to run directly in the task's context, so a ContextVar.set() inside the handler was visible to the task afterwards. With ctx.copy() that write is now discarded (I confirmed both behaviours with a small script on main and on this branch). I think that's the right call, but a sentence in the versionchanged note would avoid surprising anyone relying on it.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Thanks for checking it locally. Added in 340c4e6: the versionchanged note now says that a ContextVar.set() made inside the handler is no longer visible to the task afterwards, since the handler runs in a copy of the context. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| The handler now runs in a copy of that context. Previously, reporting | ||
| an exception from inside the task's own context raised | ||
| :exc:`RuntimeError` and the handler was skipped. Because the handler |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Small accuracy nit: on main the handler isn't really skipped silently. The RuntimeError from ctx.run() is caught and passed to default_exception_handler() as "Unhandled error in exception handler", with the original context nested under 'context'. Maybe something like:
The handler now runs in a copy of that context. Previously, reporting
an exception from inside the task's own context failed with
:exc:`RuntimeError`, and the handler was not called; the failure was
logged as an unhandled error in the exception handler instead.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| .. versionchanged:: next | ||
|
|
||
| When no exception handler is set, :meth:`call_exception_handler` | ||
| runs this method in a copy of the :class:`contextvars.Context` of | ||
| the task or handle where the exception originated, as for a custom | ||
| handler. |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This note describes how call_exception_handler() behaves rather than default_exception_handler() itself (calling loop.default_exception_handler() directly is unaffected). Consider moving it under loop.call_exception_handler instead, e.g. "When no custom handler is set, the default handler now runs in a copy of the context of the task or handle where the exception originated."
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| self.loop.run_until_complete, self.loop.subprocess_shell, | ||
| asyncio.SubprocessProtocol, 'exit 0', bufsize=4096) | ||
|
|
||
| def test_default_exc_handler_runs_in_task_context(self): |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
This covers the 'task' and 'handle' keys, but not 'future', which is the path the issue's repro actually goes through: "Task exception was never retrieved" is reported from the task finalizer with 'future': task. Could you add that case here too, e.g.
self.loop.call_exception_handler({ 'message': 'msg', 'future': task, 'exception': ValueError()})
(and extend the expected list accordingly). An end-to-end variant along the lines of the issue's repro (a failing task dropped without retrieving its exception, then support.gc_collect()) would also be nice, but the direct call is enough.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
asyncio: default exception handler does not run in the task's context
There was an error while loading. Please reload this page.