Skip to content

feat(callbacks): [Callback Errors Optional Addon] Add opt-in strict callback mode - #6173

Draft
adinauer wants to merge 51 commits into
mainfrom
feat/strict-callback-mode
Draft

adinauer wants to merge 51 commits into
mainfrom
feat/strict-callback-mode

Conversation

@adinauer

@adinauer adinauer commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

PR Stack (Callback Errors)


📜 Description

Add opt-in strictCallbackMode, disabled by default, so user callback failures propagate instead of being isolated by the SDK.

  • Preserve exceptions and errors as distinct callback markers with the original failure as their cause.
  • Centralize marker propagation in ExceptionUtils.maybeRethrow() before logging or discard accounting, including enclosing SDK catches and async completion paths. Leave the broader fatal-error policy unchanged.
  • Silently exclude events containing callback markers, including nested/cyclic causes and ExceptionMechanismException, before capture callbacks, processors, logging, or discard accounting. Apply the same exclusion at JUL, Logback, Log4j2, and Timber entry points.
  • Support SDK options, strict-callback-mode external configuration, io.sentry.strict-callback-mode Android metadata, and sentry.strict-callback-mode Spring Boot binding.

Async failures follow the host API's normal behavior: returned futures/completion stages retain the failure; the SDK does not forcibly terminate the process.

💡 Motivation and Context

Offer explicit callback-failure propagation without changing default behavior. Exclude the callback-failure event rather than bypassing the failed filter: sending it without customer filtering could expose sensitive data or re-enter the failing callback.

This PR builds only on #6167; review its diff against fix/callback-error-handling-custom-callbacks, not main.

💚 How did you test it?

  • 4,831 tests passed across core and affected integration/configuration suites, including default-off behavior, callback-marker propagation, cyclic/nested causes, capture exclusion, cleanup, and Spring/Android configuration.
  • Four rebuilt console-sample probes covered exception/error markers both directly and nested in CompletionException: exit code 1, one callback invocation, zero discards, and zero HTTP requests.
  • ./gradlew spotlessApply apiDump, apiCheck, and spotlessCheck passed.

📝 Checklist

  • I added GH Issue ID & Linear ID
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.
  • Public API changes reviewed by another Mobile SDK team member or implemented according to the develop docs spec.

🔮 Next steps

Address remaining SDK team review feedback on marker logging during uncaught-exception capture, feedback dismiss-listener cleanup, and proxy-only Log4j2 events. Broader fatal-error propagation remains deferred to the next major.

⚠️ Merge this PR using a merge commit (not squash). Only the collection branch is squash-merged into main.

adinauer and others added 26 commits September 22, 2026 06:19
Add the callback_error discard reason and use it when a beforeSend callback throws. Preserve before_send for callbacks that intentionally return null and retain item, span, and byte accounting.

Refs #6081
Co-Authored-By: Claude <noreply@anthropic.com>
Stop processing and drop telemetry when an event processor throws. Record callback_error outcomes for every supported category instead of sending potentially partially processed data.

Refs #6081
Co-Authored-By: Claude <noreply@anthropic.com>
Drop breadcrumbs when beforeBreadcrumb throws instead of storing the breadcrumb with exception details. Keep observer state unchanged and release the callback reentrancy guard for later additions.

Refs #6081
Co-Authored-By: Claude <noreply@anthropic.com>
Add an internal marker to SDK event processors across core and integrations.
Keep customer processors and the backfilling marker independent so later
callback handling can distinguish ownership regardless of registration path.

Leave exception handling unchanged in this preparatory change.

Refs #6081
Co-Authored-By: Claude <noreply@anthropic.com>
Make the preceding marker PR available to the processor failure policy.
Preserve the existing stack commits and leave failure behavior unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
Carry the inserted SDK processor marker forward through the adjacent stack
branch without rewriting existing history or changing breadcrumb behavior.

Co-Authored-By: Claude <noreply@anthropic.com>
Use the internal processor marker to continue processing after SDK-owned
processor failures without recording callback_error losses. Keep customer
processor failures fail-closed across events, transactions, replays,
feedback, logs, and metrics.

Cover scope and options registration, continued callbacks and delivery,
logging without discard notifications, intentional drops, and span loss
accounting. Clarify the customer-only failure policy in the changelog.

Refs #6081
Co-Authored-By: Claude <noreply@anthropic.com>
Merge #6142 forward into #6143 so SDK-owned processor failures preserve\ntelemetry throughout the stack. Keep the breadcrumb-only diff unchanged\nand retain both changelog entries without rewriting existing history.
Inherit the parent sampling decision when tracesSampler throws. Without a
parent decision, leave the trace unsampled instead of applying the static
tracesSampleRate, which can override the failed sampling policy.

Keep normal null-result fallback, profiling callbacks, and catch types
unchanged. Cover parent inheritance, backfilling, and static-rate bypass.

Refs #6081

Co-Authored-By: Claude <noreply@anthropic.com>
Link the tracesSampler failure fallback entry to #6163 in the callback
error handling stack.
Disable profiling when profilesSampler throws instead of applying the
static profilesSampleRate or inheriting a parent profiling decision.
Preserve trace sampling metadata and normal null-result fallback.

Cover static-rate bypass, parent inheritance, and failures in both
samplers without changing existing catch types.

Refs #6081

Co-Authored-By: Claude <noreply@anthropic.com>
Link the profilesSampler failure handling entry to #6164 in the callback
error handling stack.
Skip replay capture when beforeErrorSampling throws instead of capturing
replay after a failed user filter. Continue sending the error event and
retain existing exception logging and catch types.

Cover handled errors, crashes, and successful replay capture after a
previous callback failure. Verify error delivery and failure logging.

Refs #6081
Co-Authored-By: Claude <noreply@anthropic.com>
Link the beforeErrorSampling failure entry to PR #6165.

Co-Authored-By: Claude <noreply@anthropic.com>
Record callback_error/profile when beforeSendTransaction or a customer
transaction processor throws and an attached profile is dropped. Notify
OnDiscardCallback through the same client-report accounting path.

Cover scope and options processors, absent profiles, and SDK processor
failures that retain profiles. Leave catch types, intentional drops,
and profile-file cleanup unchanged.

Refs #6081
Co-Authored-By: Claude <noreply@anthropic.com>
Link the transaction profile loss accounting entry to PR #6166.

Co-Authored-By: Claude <noreply@anthropic.com>
Skip Android screenshot and view hierarchy capture when their callbacks
throw, while retaining the error event. Drop spans on beforeSpan failures
in OkHttp, OpenFeign, GraphQL, Ktor, and Apollo without disrupting requests
or replacing the original request error.

Preserve normal callback results and existing catch types. Finish failed
spans and retain request cleanup and breadcrumbs.

Add regression coverage for callback failures, partial mutations, original
request errors, asynchronous GraphQL results, and subsequent Android
captures. Correct callback wiring in the Ktor and screenshot test fixtures.

Verify 18 regression cases fail before the fix and pass afterward; all 454
tests in the affected suites pass, along with formatting and API checks.

Refs #6081

Co-Authored-By: Claude <noreply@anthropic.com>
Link the Android capture and integration beforeSpan callback fixes to
Callback Errors 9 (#6167).

Co-Authored-By: Claude <noreply@anthropic.com>
Replace parent-decision fallback after tracesSampler errors with an
unsampled decision and callback_error reports for the transaction and
root span. Preserve deliberate null-result fallback and existing catches.

Report callback errors directly in the sampler and retain existing
sample_rate and backpressure accounting, accepting duplicate loss reports
instead of adding discard-reason propagation. Cover core and OpenTelemetry
paths with regression tests and update the changelog.

Refs #6163
Co-Authored-By: Claude <noreply@anthropic.com>
Merge the updated tracesSampler failure behavior from #6163 into #6164.
Preserve profile-sampler isolation and update the combined-failure test to
expect tracing and profiling to be disabled, even with a sampled parent.

Refs #6163
Refs #6164
Co-Authored-By: Claude <noreply@anthropic.com>
Merge #6164 forward into #6165 without rewriting stack history.
Retain replay sampling changes and the updated trace-sampler changelog.

Refs #6165
Co-Authored-By: Claude <noreply@anthropic.com>
Merge #6165 forward into #6166 without rewriting stack history.
Preserve attached-profile loss accounting alongside the sampler fixes.

Refs #6166
Co-Authored-By: Claude <noreply@anthropic.com>
Merge #6166 forward into #6167 without rewriting stack history.
Preserve integration callback guards alongside the updated sampler behavior.

Validate the combined core and OpenTelemetry suites, formatting, and API checks.

Refs #6167
Co-Authored-By: Claude <noreply@anthropic.com>
Record callback_error/span when a throwing beforeSpan callback drops a
sampled span in OkHttp, OpenFeign, GraphQL, Ktor, and Apollo. Use the
pre-callback sampling decision so partial callback mutations do not hide
losses, and leave unsampled spans and intentional null drops uncounted.

Add regression coverage for discard notifications, sampling states, and
intentional drops while preserving request and span completion behavior.

Refs #6167
Co-Authored-By: Claude <noreply@anthropic.com>
Propagate callback failures with exception and error markers while keeping
strict callback mode disabled by default. Support SDK options, external configuration,
Android manifest metadata, and Spring Boot property binding.

Silently exclude marked failures, including nested causes, from capture and
logging integration processing. Avoid re-entering failed filters or sending
potentially sensitive telemetry without their filtering.

Preserve failure propagation across enclosing SDK catches and async completion
stages, with regression coverage for configuration, cleanup, and exclusion.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 598c1dd

@sentry

sentry Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

📲 Install Builds

Android

🔗 App Name App ID Version Configuration
SDK Size io.sentry.tests.size 8.59.0 (1) release

⚙️ sentry-android Build Distribution Settings

}

/** Callback failures must be excluded before invoking any processors or other callbacks. */
public static boolean isCallbackException(final @Nullable Throwable throwable) {

@romtsn romtsn Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my clanker found a couple of perf optimizations here:

isCallbackException runs on every captureEvent / captureException and allocates an IdentityHashMap per call, even though markers can only exist when strictCallbackMode is on.

Two cheap fixes: short-circuit on options.isStrictCallbackMode() before walking, and replace the visited set with a depth bound like findRootCause does. The second one also matters for maybeRethrow, which runs inside catch blocks where allocating during an OOM is unsafe.

@romtsn romtsn left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some things to address with this one

Record attached profiles as lost when transaction event processors or
beforeSendTransaction intentionally discard the transaction. Keep client
reports and discard callbacks aligned with dropped profile payloads.

Co-Authored-By: Claude <noreply@anthropic.com>
@adinauer adinauer changed the title feat(callbacks): [Callback Errors 10] Add opt-in strict callback mode feat(callbacks): [Callback Errors Optional Addon] Add opt-in strict callback mode Oct 8, 2026
adinauer and others added 15 commits October 8, 2026 10:11
…iscard-reason

fix(core): [Callback Errors 1] Report callback failures
Record callback_error/replay when beforeErrorSampling throws while the replay
controller has an active replay ID. Do not report a loss when no replay is active.

Refs GH-6081
Co-Authored-By: Claude <noreply@anthropic.com>
Bring the stack onto the latest main changes, including CI resource and test-duration fixes.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and callback error base changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>
Propagate the latest main and preceding callback error changes through the stacked pull request.

Co-Authored-By: Claude <noreply@anthropic.com>

# Conflicts:
#	CHANGELOG.md
#	sentry/src/main/java/io/sentry/util/ExceptionUtils.java
…rocessor-marker

ref(core): [Callback Errors 2] Mark SDK-owned event processors
…vent-processors

fix(core): [Callback Errors 3] Drop failed processor data
…ustom-callbacks

fix(integrations): [Callback Errors 9] Guard custom callbacks
Base automatically changed from fix/callback-error-handling-custom-callbacks to fix/callback-error-handling-profile-accounting October 8, 2026 10:53
…readcrumbs

fix(core): [Callback Errors 4] Drop failed breadcrumbs
…races-sampler

fix(core): [Callback Errors 5] Handle tracesSampler failures
…rofiles-sampler

fix(core): [Callback Errors 6] Handle profilesSampler failures
…eplay-sampling

fix(core): [Callback Errors 7] Handle replay sampling failures
…rofile-accounting

fix(core): [Callback Errors 8] Report dropped profiles
Base automatically changed from fix/callback-error-handling-profile-accounting to fix/callback-error-handling October 8, 2026 10:56
adinauer and others added 2 commits October 9, 2026 06:50
Use Kotlin's error helper in callback failure tests so Detekt accepts the
intentional IllegalStateException across Apollo 2, 3, and 4.

Co-Authored-By: Claude <noreply@anthropic.com>
Base automatically changed from fix/callback-error-handling to main October 13, 2026 14:57

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants