fix: propagate contextvars to event handlers#3057
Open
Skn0tt wants to merge 1 commit intomicrosoft:mainfrom
Open
fix: propagate contextvars to event handlers#3057Skn0tt wants to merge 1 commit intomicrosoft:mainfrom
Skn0tt wants to merge 1 commit intomicrosoft:mainfrom
Conversation
Capture the caller's contextvars context when an event handler is
registered, and re-establish it when the handler runs. Without this,
contextvars set in user code (e.g. request IDs in logging frameworks)
were not visible inside event handlers because events are dispatched
from a different greenlet (sync mode) or asyncio task (async mode)
than the one that registered the handler.
Implementation per mode:
* Async-mode coroutine handler: spawn an inner Task inside the
captured context so the Task adopts it (Tasks copy the active
context at construction).
* Async-mode sync handler: run via Context.run.
* Sync mode: temporarily set the EventGreenlet's gr_context to the
captured context for the duration of the handler. Context.run is
not used here because handlers like route.fulfill internally
greenlet.switch, and Context.run does not compose with greenlet
switches.
Fixes microsoft#1816
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #1816
Capture the caller's contextvars context when an event handler is registered, and re-establish it when the handler runs. Without this, contextvars set in user code (e.g. request IDs in logging frameworks) were not visible inside event handlers because events are dispatched from a different greenlet (sync mode) or asyncio task (async mode) than the one that registered the handler.
Implementation per mode
Taskinside the captured context so the Task adopts it (Tasks copy the active context at construction).Context.run.EventGreenlet'sgr_contextto the captured context for the duration of the handler.Context.runis not used here because handlers likeroute.fulfillinternally callgreenlet.switch, andContext.rundoes not compose with greenlet switches (it would unwind on switch and corrupt context state on switch-back).Tests
Added regression tests for both sync and async modes that set a
ContextVar, register a request handler, navigate, and assert the handler observed the value. The async test also asserts the value survives anawait asyncio.sleep(0)boundary inside the handler.Verification