GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — GH-96704: Add task.get_context(), use it in call_exception_handler() by gvanrossum · Pull Request #96756 · python/cpython

Conversation

@gvanrossum

@gvanrossum gvanrossum commented Sep 11, 2022 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

The point here is for call_exception_handler() to make an effort to run the exception handler in the correct task context (contextvars.Context), if it is called on behalf of a task.

The easiest way seems to be to retrieve the task context from the task passed into the context dict that's being passed to call_exception_handler(). This doesn't always exist, and because Task inherits from Future, it may be stored under either a "task" key or a "future" key. So try both.

We then need to retrieve the contextvars.Context from the task, for which I have added a get_context() method to the Task class (analogous to get_name()). It is possible that the task doesn't have this method (e.g. if the loop is using a 3rd party task implementation that doesn't yet support this new method).

Only if get_context() exists and returns something that has a run() method do we run the exception handler using that contextvars.Context.run() method. Otherwise we use the default contextvars.Context.

We only do this for an exception handler set by the user; the default exception handler should not be interested in the context. (This is because I'm lazy, there are two calls to it and I don't want to bother giving those the same treatment -- we can reconsider if you think the logger might also be interested in the contextvars.Context.)

PS. It's unfortunate that call_exception_handler() has a parameter named context that is a dict, not a contextvars.Context. But nothing we can do it now (this API probably predates contextvars).

  • Issue: asyncio exception handler called with incorrect context #96704

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@gvanrossum
gvanrossum requested a review from kumaraditya303 September 11, 2022 19:38
@bedevere-bot bedevere-bot added the awaiting core review label Sep 11, 2022
@gvanrossum

gvanrossum commented Sep 11, 2022

Copy link
Copy Markdown
Member Author

Possibly we also need to look for a Context in the "handle" key of the context arg, since Handles also have a context.

An alternative would of course be to use self._context.run(loop.call_exception_handler, context) everywhere where it makes sense. But how many locations is that? And it would mean more C code -- there are two calls to call_exception_handler in _asynciomodule.c, and 3rd party task implementations would have to duplicate that code (instead of just the code for get_context(), which is pretty straightforward).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 reviewed Sep 12, 2022
View reviewed changes
Comment thread Lib/asyncio/base_events.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Sep 14, 2022

Copy link
Copy Markdown
Member Author

Possibly we also need to look for a Context in the "handle" key of the context arg, since Handles also have a context.

An alternative would be to use self._context.run(loop.call_exception_handler, context) everywhere where it makes sense. But how many locations is that? And it would mean more C code -- there are two calls to call_exception_handler in _asynciomodule.c, and 3rd party task implementations would have to duplicate that code (instead of just the code for get_context(), which is pretty straightforward).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum
gvanrossum marked this pull request as ready for review September 14, 2022 22:03
@gvanrossum
gvanrossum requested review from 1st1 and asvetlov as code owners September 14, 2022 22:03
@gvanrossum

gvanrossum commented Sep 18, 2022

Copy link
Copy Markdown
Member Author

We still need a mention in the docs for set_exception_handler, and similar code for Handle.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@gvanrossum

gvanrossum commented Oct 4, 2022

Copy link
Copy Markdown
Member Author

I hope you don't mind I am going to attempt a rebase here. I could merge main, but git log looks confusing to me when I do that. :-(

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

gvanrossum and others added 3 commits October 4, 2022 13:47
@gvanrossum
pythonGH-96704: Add task.get_context(), use it in call_exception_hand…
1d2d081
…ler()
@kumaraditya303 @gvanrossum
add two tests
9fbc1d8
@blurb-it @gvanrossum
📜🤖 Added by blurb_it.
bc3000b
@gvanrossum
gvanrossum force-pushed the exc-handler-context branch from a2a0c57 to bc3000b Compare October 4, 2022 20:47
gvanrossum added 4 commits October 4, 2022 15:20
@gvanrossum
Also look in handle for contextvars
bcd1a4a
@gvanrossum
Update NEWS
d9ccfe3
@gvanrossum
Document the {Task,Handle}.get_context() methods
a362fc0
@gvanrossum
Document change for exception handler
0ca5023
@gvanrossum

gvanrossum commented Oct 4, 2022

Copy link
Copy Markdown
Member Author

@kumaraditya303 Ready for final review.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

kumaraditya303
kumaraditya303 approved these changes Oct 5, 2022
View reviewed changes

@kumaraditya303 kumaraditya303 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@gvanrossum
gvanrossum merged commit 8079bef into python:main Oct 5, 2022
@bedevere-bot bedevere-bot removed the awaiting core review label Oct 5, 2022
@gvanrossum
gvanrossum deleted the exc-handler-context branch October 5, 2022 06:49
carljm added a commit to carljm/cpython that referenced this pull request Oct 6, 2022
@carljm
Merge branch 'main' into dictwatch
4bb5d9d
* main: (66 commits)
  pythongh-65961: Raise `DeprecationWarning` when `__package__` differs from `__spec__.parent` (python#97879)
  docs(typing): add "see PEP 675" to LiteralString (python#97926)
  pythongh-97850: Remove all known instances of module_repr() (python#97876)
  I changed my surname early this year (python#96671)
  pythongh-93738: Documentation C syntax (:c:type:<C type> -> :c:expr:<C type>) (python#97768)
  pythongh-91539: improve performance of get_proxies_environment  (python#91566)
  build(deps): bump actions/stale from 5 to 6 (python#97701)
  pythonGH-95172 Make the same version `versionadded` oneline (python#95172)
  pythongh-88050: Fix asyncio subprocess to kill process cleanly when process is blocked (python#32073)
  pythongh-93738: Documentation C syntax (Function glob patterns -> literal markup) (python#97774)
  pythongh-93357: Port test cases to IsolatedAsyncioTestCase, part 2 (python#97896)
  pythongh-95196: Disable incorrect pickling of the C implemented classmethod descriptors (pythonGH-96383)
  pythongh-97758: Fix a crash in getpath_joinpath() called without arguments (pythonGH-97759)
  pythongh-74696: Pass root_dir to custom archivers which support it (pythonGH-94251)
  pythongh-97661: Improve accuracy of sqlite3.Cursor.fetchone docs (python#97662)
  pythongh-87092: bring compiler code closer to a preprocessing-opt-assembler organisation (pythonGH-97644)
  pythonGH-96704: Add {Task,Handle}.get_context(), use it in call_exception_handler() (python#96756)
  pythongh-93738: Documentation C syntax (:c:type:`PyTypeObject*` -> :c:expr:`PyTypeObject*`) (python#97778)
  pythongh-97825: fix AttributeError when calling subprocess.check_output(input=None) with encoding or errors args (python#97826)
  Add re.VERBOSE flag documentation example (python#97678)
  ...
mpage pushed a commit to mpage/cpython that referenced this pull request Oct 11, 2022
@gvanrossum @kumaraditya303 @mpage
pythonGH-96704: Add {Task,Handle}.get_context(), use it in call_excep…
681e059
…tion_handler() (python#96756)

Co-authored-by: Kumar Aditya <[email protected]>
@Reskov Reskov mentioned this pull request Oct 13, 2026
asyncio: default exception handler does not run in the task's context #159303
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kumaraditya303 kumaraditya303 kumaraditya303 approved these changes
@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner
@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@gvanrossum @kumaraditya303 @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.