fix: an exception in BeforeSend or BeforeSendTransaction now drops the item instead of sending it

[jamescrosswell]

Work item 3 ("align BeforeSend / BeforeSendTransaction with the matrix") of #5535, the last one. Now that #5606 and #5607 have landed this targets main directly.

Three BeforeSend routes did the same thing when a user's callback threw: swallow the exception and send the item anyway.

Path	Before	Now
Managed BeforeSend	breadcrumb with the exception attached, event sent	error log naming the callback, event dropped, callback_error / error
Managed BeforeSendTransaction	same	error log naming the callback, transaction dropped, callback_error / transaction + span
Android native BeforeSend	original unscrubbed Java event sent	error log naming the callback, event dropped by the Java SDK

For the two managed callbacks this deviated from the spec twice — a failure MUST NOT be attached to the item, and the matrix says filters drop, with the rationale spelled out: "filters drop because a callback that failed part-way may not have applied the redaction the user wrote it to apply."

That's the realistic case in .NET, where the commonest job for BeforeSend is PII scrubbing. A callback that threw mid-scrub meant both the partially-redacted event and the exception message and stack trace we stapled to it were sent — a user's redaction failure turning into two kinds of unintended data landing in their org. Hence handling this as a bug rather than holding it for a major.

Both managed paths now match DoBeforeSendFeedback, which was already conformant, and use the callback_error reason #5607 introduced for the revised spec's "a failure must be distinguishable from an explicit drop".

The Android bridge

Not in #5535's conformance table, and not a regression from this PR, but the same gap and a worse outcome — so it's fixed here rather than left behind a Closes.

BeforeSendCallback.Execute wraps its whole body in a try and returns e on failure — the original Java event, not the managed one the callback was mutating. So a scrubber that threw part-way didn't send a partially-redacted event; it sent the completely unredacted native one. That catch arrived in #4022 aimed at serialization safety ("native types tend to move before dotnet does"), and swallowing the user callback was collateral.

The fix keeps those two concerns apart. The user callback is invoked one level in, inside BeforeSendWrapper, so guarding it there returns null for a user failure — which the Java SDK turns into a drop — and leaves Execute's catch doing only the serialization fail-open it was written for.

No managed client report is recorded on that path: sentry-java records the drop itself when the callback returns null (SentryClient.java#L171-177), and native events are the native SDK's to account for. It records before_send rather than callback_error, so on this route a failure isn't distinguishable from an explicit drop — tracked as item 2 of #5634 and blocked on getsentry/sentry-java#6141, which adds the reason upstream.

Client report accounting on native events

The two bridges end up asymmetric, both cases covered by #5634:

Route	callback_error recorded	Native SDK also records	Net
Android	no — the guard lives in BeforeSendWrapper	before_send	counted once, under the pre-revision reason (#5634 item 2)
iOS	yes — the throw goes through the shared DoBeforeSend	before_send	counted twice, reasons disagreeing (#5634 item 1)

The Android side avoids a double count because the guard sits in the bridge's own wrapper; the Cocoa side routes through SentryEventHelper.DoBeforeSend, which records for every caller. Main already double-counts a null return on the iOS path, so this adds the throw case to an existing problem rather than a new one — but it is a behaviour change, hence flagging it here. #5634 item 1 is explicitly marked as needing a decision: suppressing the managed record fixes the count but loses the reason until item 2 ships upstream, so it isn't a fix to slip into this PR.

Scope: Native.EnableBeforeSend is opt-in and defaults to false. Managed events never cross the JNI bridge. SuppressSegfaults installs the same wrapper without calling user code, so that route can't reach the guard.

Notes for review

• e.Demystify() goes with the breadcrumb. It existed only to make the stack trace readable in that breadcrumb, and was the reason both methods carried an [UnconditionalSuppressMessage("Trimming", "IL2026")] and an AotHelper.IsTrimmed guard — all now gone. Ben.Demystifier is still used by DebugStackTrace for StackTraceMode.Enhanced, so nothing is orphaned.
• BeforeSendTransaction now takes spanCount and owns its own reporting for both the null return and the throw, so the caller's drop branch just returns. Previously the span accounting lived at the call site, which meant a throw either duplicated it or logged twice.
• CaptureEvent_BeforeEventThrows_ErrorToEventBreadcrumb and CaptureTransaction_BeforeSendTransactionThrows_ErrorToEventBreadcrumb pinned the old behaviour and are deleted, along with their three Verify snapshots and the now-empty SentryClientTests.verify.cs. Replaced with ordinary assertions covering the drop, the client report, the error log, and the absence of any breadcrumb on the item.
• iOS native crashes also run managed BeforeSend, and SentryEvent.Exception is null there, so a callback that dereferences it throws on that path only — previously reported with a breadcrumb, now dropped. The throwing case had no coverage in the existing ProcessOnBeforeSend tests; it does now. The underlying null-Exception problem is not introduced here and is tracked separately in iOS: SentryEvent.Exception is null for native crashes, so a BeforeSend callback that dereferences it throws #5620.
• Still user-visible for anyone whose callback throws today. They currently get a degraded event; they will now get none — but they also get an error-level diagnostic log naming the callback, and the loss is counted in client reports rather than being invisible.

Verified on device as well as the host: the Android guard by the Run Android API-34/36 Test jobs, and the Cocoa path by ios-tests (both new tests confirmed present and passing in the results artifact, not just inferred from a green job).

Closes #5535

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.92%. Comparing base (cc9ec24) to head (61f72bf).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5610      +/-   ##
==========================================
- Coverage   74.94%   74.92%   -0.03%     
==========================================
  Files         515      515              
  Lines       18994    18973      -21     
  Branches     3698     3693       -5     
==========================================
- Hits        14236    14215      -21     
- Misses       3877     3878       +1     
+ Partials      881      880       -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[ric-oliv]

Looks good! Just two notes my agent flagged:

Android native BeforeSend still fails open. Platforms/Android/Callbacks/BeforeSendCallback.cs catches the exception, logs "Before Send Error" and returns the original native event (return e), so a scrubber that throws partway still lets the event through. It only applies to apps that opt into Native.EnableBeforeSend, and it wasn't in #5535's conformance table, but it's the same gap this PR closes elsewhere. Could we align it here, or track it separately and change Closes #5535 to Part of?

Worth a line in the release note (not a blocker). On iOS, BeforeSend also runs on native crashes (Platforms/Cocoa/SentrySdk.cs:321), where @event.Exception is null. A callback that dereferences it currently still gets the crash reported, with a breadcrumb. After this PR it's silently dropped. Something like "BeforeSend also runs on iOS native crashes, where Exception is null; a throwing callback now drops the event" would help people notice. A ProcessOnBeforeSend test with a throwing callback would also pin down that path.

[jamescrosswell]

Android native BeforeSend still fails open. Platforms/Android/Callbacks/BeforeSendCallback.cs catches the exception, logs "Before Send Error" and returns the original native event (return e), so a scrubber that throws partway still lets the event through. It only applies to apps that opt into Native.EnableBeforeSend, and it wasn't in #5535's conformance table, but it's the same gap this PR closes elsewhere. Could we align it here, or track it separately and change Closes #5535 to Part of?

Ah, that's actually a problem.

We use serialisation to convert the Java object to a managed one, so we can run the managed beforesend callback. We don't want issues with that serialisation to prevent the event from being sent.

However that catches issues in the beforesend and then still sends the original event - meaning any problems in the user's beforesend callback (maybe a callback they wrote to strip PII) would result in all events being sent unredacted... not good.

It's not introduced by this PR but it is the kind of thing this PR is supposed to fix, so we should address it.