feat(core): Add safeCallback helper for isolating user-provided callbacks - #23760
Conversation
size-limit report 📦
|
|
bugbot run |
safeCallback helper for isolating user-provided callbackssafeCallback helper for isolating user-provided callbacks
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit bb9a102. Configure here.
| return modifiedSpan; | ||
| } | ||
|
|
||
| if (!hasShownSpanDropWarning) { |
There was a problem hiding this comment.
unrelated to this PR, but we can possibly remove this warning (safe some bytes), this has been this way for some time 🤔 or at least make it a debug.warn gated by the debug flag so you can shake it out?
There was a problem hiding this comment.
yeah i think we can remove it tbh. has been like this since v9 and the type doesn't allow it anyway.
bb9a102 to
9792842
Compare
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 9792842. Configure here.
Lms24
left a comment
There was a problem hiding this comment.
Had one follow-up suggestion for better tree-shaking but otherwise looks good!
| } | ||
|
|
||
| function recover<T>(message: string, error: unknown, fallback: (error: unknown) => T): T { | ||
| DEBUG_BUILD && debug.error(message, error); |
There was a problem hiding this comment.
I think the flag is correct here but the bot review is right: We no longer tree-shake out the actual string passed to this function if DEBUG_BUILD is false. We can still add a ternary to the call-sitest to enable tree shaking, something like
safeCallback(DEBUG_BUILD ? 'full warning/error message' : '', () => {}, () => {})…backs Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
9792842 to
e3f5c00
Compare
… as events (#23770) Wraps `beforeSend`, `beforeSendTransaction`, event processors, `tracesSampler`, `beforeBreadcrumb`, `beforeSendLog` and `beforeSendMetric` in the `safeCallback` helper from #23760. A throwing or rejecting callback no longer escapes into the calling code and is no longer captured as an `internal` error event; the event/breadcrumb/log/metric is dropped, a client report is recorded where a category exists, and the error is logged in debug mode. part of #23755 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

Adds
safeCallback(message, fn, fallback)helper which runs a user-provided callback, and on a sync throw or async rejection logsmessageviadebug.errorand returnsfallback(error)instead of propagating.applyBeforeSendSpanCallbackand the undicisafeExecuteare refactored onto the helper so there is a single implementation.First step of #23755, next PR will wrap the rest of the user-defined callbacks.