fix(core): Catch beforeSendSpan callback throws - #22836
Merged
Merged
Conversation
Contributor
size-limit report 📦
|
Lms24
force-pushed
the
feat/span-streaming-tree-shaking
branch
from
July 29, 2026 15:37
962dbc1 to
7de446c
Compare
Lms24
force-pushed
the
fix/before-send-span-throw
branch
from
July 29, 2026 15:37
2b3d3e2 to
192e001
Compare
Lms24
force-pushed
the
feat/span-streaming-tree-shaking
branch
from
August 3, 2026 12:58
7de446c to
74a1167
Compare
Lms24
force-pushed
the
fix/before-send-span-throw
branch
from
August 3, 2026 12:58
192e001 to
f1557e3
Compare
Lms24
force-pushed
the
feat/span-streaming-tree-shaking
branch
from
August 3, 2026 14:31
74a1167 to
ee4761d
Compare
Lms24
force-pushed
the
fix/before-send-span-throw
branch
2 times, most recently
from
August 4, 2026 09:23
cd95b76 to
b5688d4
Compare
Lms24
force-pushed
the
feat/span-streaming-tree-shaking
branch
2 times, most recently
from
August 4, 2026 13:33
b568562 to
a43fe54
Compare
Lms24
force-pushed
the
fix/before-send-span-throw
branch
from
August 4, 2026 13:33
b5688d4 to
214a5f3
Compare
Lms24
force-pushed
the
feat/span-streaming-tree-shaking
branch
from
August 4, 2026 13:47
a43fe54 to
a3e7446
Compare
Lms24
force-pushed
the
fix/before-send-span-throw
branch
from
August 4, 2026 13:47
214a5f3 to
8eda355
Compare
Lms24
force-pushed
the
fix/before-send-span-throw
branch
from
August 4, 2026 16:18
8eda355 to
91dd535
Compare
Lms24
changed the title
beforeSendSpan callback escape into user codebeforeSendSpan callback throws
Lms24
force-pushed
the
fix/before-send-span-throw
branch
from
August 6, 2026 16:29
61f06d4 to
8745c98
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
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.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 8745c98. Configure here.
| } | ||
| const processedRootSpanJson = applyBeforeSendSpanCallback(rootSpanJson, beforeSendSpan); | ||
| // update event with processed root span values | ||
| processedEvent = merge(event, convertSpanJsonToTransactionEvent(processedRootSpanJson)); |
There was a problem hiding this comment.
Unmodified root span still merged
Low Severity
When beforeSendSpan throws or returns null for the root span, applyBeforeSendSpanCallback correctly hands back the original span JSON, but the caller always merges the convert round-trip into the transaction. Previously a null return left the event untouched. Because merge replaces contexts.trace wholesale and the converters apply defaults, the “unmodified” fallback can still alter the event.
Reviewed by Cursor Bugbot for commit 8745c98. Configure here.
Member
Author
There was a problem hiding this comment.
yeah I'm willing to take this risk. mutating the originally passed in json is not something we directly recommend but we should still expected. It might even be advantageous here: If users scrub first, and then decide to return null, the scrubbing still gets applied if they mutate in-place. This should be fine to change, especially in v11. I'm not planning on backporting this PR anyway.
… user code Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Lms24
force-pushed
the
fix/before-send-span-throw
branch
from
August 7, 2026 09:54
8745c98 to
282d43b
Compare
Lms24
marked this pull request as ready for review
August 7, 2026 10:47
JPeer264
approved these changes
Aug 7, 2026
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
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.
This PR wraps the
beforeSendSpancallback application incaptureSpan,captureStandaloneSpanWithStaticCallbackand core client transactionbeforeSendSpaninvocations in a try/catch. The error is logged and the span is sent unmodified to Sentry.Spans are captured synchronously from the
afterSpanEndhook, so a throwing callback propagates straight out of whatever user code calledspan.end(). Especially with the requiredbeforeSendSpanmigration fromSpanJSONtoStreamedSpanJSON, this is quite likely to happen.We want to apply similar guards to more user-facing callbacks in the future, so we might be able to reuse/adjust the
applyBeforeSendSpanhelper to more functionality later on.