Uh oh!
There was an error while loading. Please reload this page.
feat(server-utils): Restore caller context for callback tracing channels - #21863
Merged
Conversation
`bindTracingChannelToSpan` only bound the span store on `start`, which covers the synchronous frame but not a callback the library dispatches from a detached async context (e.g. a socket data handler or a `setImmediate` drain). There, native async-context propagation no longer reaches the caller, so work issued inside the callback lost its parent. Stash the caller's store at `start` and re-bind it on `asyncStart`, so callback-style channels run their continuation in the caller's context — the same way a promise's `.then` does natively. It's inert for promise channels, which `publish` `asyncStart` rather than `runStores` it. Migrate the lru-memoizer subscriber onto the helper (`getSpan` returns `undefined`, so no span is created — it only needs the context rebind), dropping its hand-rolled callback re-wrapping.
Contributor
size-limit report 📦
|
logaretm
marked this pull request as ready for review
June 30, 2026 18:29
logaretm
requested review from
JPeer264, andreiborza, isaacs and mydea
and removed request for
a teamJune 30, 2026 18:29
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 910fdfe. Configure here.
Uh oh!
There was an error while loading. Please reload this page.
… caller context The asyncStart producer used `data._sentryCallerStore ?? asyncLocalStorage.getStore()`. When the caller had no active store, `_sentryCallerStore` is `undefined` and the fallback bound whatever was ambient at callback time — for a callback dispatched from a pooled socket handler, that can be another request's store. `start` always runs first and sets `_sentryCallerStore`, so return it verbatim (no fallback), matching the synchronous path: no caller context restores to none, not a foreign one.
isaacs
approved these changes
Jun 30, 2026
isaacs
left a comment
Member
There was a problem hiding this comment.
Works, and makes the mysql integration work. Will update pg PR once these two land.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
bindTracingChannelToSpanbound the span store onstartonly. That covers the synchronous frames and traces run throughtracePromise. In general it covers start wrapped continuations.We noticed that this doesn't work well for some callback-based traced calls, where
asyncStartcan run in a detached context. The main difference between promise-based traced calls and callback-based is callback continuations are run outside thestartrunStores callback. So adding a context rebind for asyncStart would fix that, and would be a safe change overall.I used
lru-memoizeras a POC to test this out. It creates no span, all it wants is the context rebind which is what the helper does anyways but was missing theasyncStartfor that case specifically. With these changes we can drop the hand rolled logic.