-
-
Notifications
You must be signed in to change notification settings - Fork 7
rfc(feature): Capture assertion violations as non-fatal error events #165
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
d331ea5
3b1e2af
a8097ff
b14babe
0127b4f
cf97d3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,124 @@ | ||
| - Start Date: 2026-09-25 | ||
| - RFC Type: feature | ||
| - RFC PR: https://github.com/getsentry/rfcs/pull/165 | ||
| - RFC Status: draft | ||
| - RFC Author: @antonis | ||
| - RFC Approver: <TBD, Mobile/SDK TSC> | ||
|
|
||
| # Summary | ||
|
|
||
| Turn assertion violations (`invariant`, `assert`, `precondition`, `Debug.Assert`, `console.assert`, and similar) into grouped, non-fatal Sentry error events, instead of letting them be stripped from release builds or crash as ordinary errors without the condition and values. | ||
|
|
||
| The proposal has two parts: | ||
|
|
||
| * **Part A: convention and runtime API** for all SDKs. A uniform `mechanism.type = "assertion"`, a `mechanism.data` schema, grouping rules, PII rules, and a public `captureAssertionViolation()` API. | ||
| * **Part B: build-time instrumentation** for select SDKs. An opt-in build transform that rewrites assertion call sites so violations survive release stripping and carry the condition, runtime values, and a stack anchored at the site, with no app source changes. | ||
|
|
||
| A working React Native reference implementation (Metro/Babel) exists in getsentry/sentry-react-native#6592. | ||
|
|
||
| # Motivation | ||
|
|
||
| A React Native app crashes in a release build. The stack trace ends inside an `invariant` call, and the event looks like any other error: it does not carry the condition that failed or the runtime values, and nothing marks it as a violated invariant. Other checks, like `console.assert`, fail without reaching Sentry at all. | ||
|
|
||
| Developers write assertions to encode invariants, then the toolchain deletes that intent before it reaches production, which is where it matters most: | ||
|
|
||
| * JS/RN: checks behind `__DEV__` are dead-code-eliminated from release bundles, and `console.assert` failures only reach the device log. | ||
| * Android/Java: `assert` is disabled unless the JVM runs with `-ea`, so every `assert` is silent in production. | ||
| * Swift/Cocoa: `assert` and `assertionFailure` are removed under `-O`. | ||
| * .NET: `Debug.Assert` is removed in Release builds. | ||
| * Python: `assert` is stripped with `-O`. | ||
| * Dart/Flutter: `assert` is stripped in profile and release. | ||
|
|
||
| So a large class of correctness checks produces zero production signal. This feature recovers it as grouped non-fatal events carrying the failed condition, the values involved, and a precise stack. | ||
|
|
||
| This matters more as more code is generated than reviewed: assertions are a durable record of intended behavior, and a violation that carries the condition and values is structured input for automated repair. | ||
|
|
||
| # Supporting Data | ||
|
|
||
| Measured against React Native 0.87.1 and a sample app's dependency tree: | ||
|
|
||
| * The `react-native` framework ships ~186 `invariant()` call sites (179 in `Libraries/`) and 103 `__DEV__` guards. `@react-native/virtualized-lists` adds 26 more invariants. | ||
| * Across a full app dependency tree: 317 `invariant()` and 174 `console.assert()` call sites in 16 packages (excluding build-only tooling like metro and babel, which never ships to the device). | ||
|
|
||
| Two distinct signals fall out of this: | ||
|
|
||
| * **Resurrection.** `__DEV__`-guarded checks are stripped from release and `console.assert` failures only reach the device log, so by default neither reaches Sentry today. | ||
| * **Readability.** An `invariant` that fires in release still crashes, but as an ordinary error without the condition or runtime values. This feature adds them. | ||
|
|
||
| These are source call sites, not runtime firings; most never fire. The point is that the ones that do fire in production currently produce nothing useful. | ||
|
|
||
| # Decision | ||
|
|
||
| All violations are captured as **non-fatal (handled) error events** in the issues pipeline, not Logs. An assertion is a correctness violation ("this must never happen"), which is issue-grade signal that deserves grouping, regression detection, alerting, and assignment. Severity is expressed with `level`: hard preconditions use `error`, report-only assertions use `warning`. | ||
|
|
||
| # Design | ||
|
|
||
| ## Part A: convention and runtime API (all SDKs) | ||
|
|
||
| Public API, with idiom-neutral naming: | ||
|
|
||
| ``` | ||
| captureAssertionViolation(condition, { pragma, message, values }) | ||
| ``` | ||
|
|
||
| The concept and base name are shared across SDKs; each applies its own language casing (for example `capture_assertion_violation` in Python), the same convention as `captureException`. | ||
|
|
||
| Event shape: | ||
|
|
||
| * `mechanism.type = "assertion"`, uniform so the class is filterable regardless of idiom. | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mechanism types can/should be liberal but clearly identify the call site that captured the exception, as spec'd here: https://develop.sentry.dev/sdk/telemetry/errors/#mechanism-type-naming Meaning, we shouldn't define one specific string here but rather make sure the integration that captures an assertion error sends an identifyable type string, as layed out in the develop spec. |
||
| * `mechanism.data.pragma` records the specific idiom, so flavors stay separable. | ||
| * `mechanism.handled` is `true` for every violation. A hard precondition still aborts after reporting, and that abort must not be reported a second time. We set this ourselves and do not block on any pending mechanism-types work. | ||
| * Grouping key is pragma plus call site, so a noisy site collapses into one issue. | ||
|
|
||
| ## Part B: build-time instrumentation (select SDKs) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 on this being opt-in and dependencies requiring opt in via an allow list. There's value here, but as you mentioned also a perf tradeoff. Something we should clearly communicate to users. |
||
|
|
||
| An opt-in build transform rewrites assertion call sites to report before their original behavior (throw or no-op). Requirements: | ||
|
|
||
| * Off by default, enabled through the SDK's existing build integration. | ||
| * First-party code by default, dependencies via an explicit allowlist. | ||
| * Lazy evaluation of message and values, so no cost on the passing path and no side effects on the reporting path. | ||
| * Original semantics preserved: hard preconditions still abort after reporting, report-only stays report-only. | ||
| * Reports in release builds only by default, configurable. Dev builds already surface assertions loudly, so reporting there is opt-in. | ||
|
|
||
| ## Which SDKs implement Part B | ||
|
|
||
| An SDK implements Part B only if both hold: | ||
|
|
||
| 1. The ecosystem has an idiomatic assertion mechanism that is stripped or disabled in release. | ||
| 2. The SDK already owns a build-instrumentation pathway, so marginal cost is low. | ||
|
|
||
| By this test the strong fits are RN (pilot, done), Android, and .NET. Part A ships first and independently. See [Appendix A](#appendix-a-part-b-sdk-fit) for the per-SDK assessment. | ||
|
|
||
| # Open problems | ||
|
|
||
| * **Volume.** Since everything is an error event, this is the critical one. The SDK throttles per call site on the client: the first occurrence is always captured, repeats are rate-limited, so a hot-path assertion never serializes or sends thousands of events. Call-site grouping collapses what does send into one issue. Server-side quotas remain the backstop. We prefer throttling over random sampling so a rare but important violation is never dropped. | ||
| * **PII.** Capturing runtime values is high risk. Default: primitives inline only, object and array snapshots behind `sendDefaultPii`, plus a redaction hook. Inherits [RFC 0062](https://github.com/getsentry/rfcs/blob/main/text/0062-controlling-pii-and-credentials-in-sd-ks.md) and [RFC 0038](https://github.com/getsentry/rfcs/blob/main/text/0038-scrubbing-sensitive-data.md). | ||
| * **Performance.** Part B re-adds cost that stripping removed. Off by default, ideally release-configurable, zero cost on the passing path. | ||
|
|
||
| # Alternatives | ||
|
|
||
| * **Runtime API only (Part A, drop Part B).** Universal but loses the headline value (no source changes, survives stripping). Kept as the guaranteed floor. | ||
| * **Route report-only assertions to Logs.** Rejected: it discards the grouping, alerting, and regression tracking that make assertion signal useful, and doubles the per-SDK surface. Revisit only if volume controls prove insufficient. | ||
| * **Do nothing.** Stripped assertions stay dark in production. | ||
|
|
||
| # Unresolved questions | ||
|
|
||
| * Final `mechanism.data` schema and the exact value-capture type policy, pending security and PII review (see Open problems). | ||
|
|
||
| # Prior art | ||
|
|
||
| * [RFC 0062](https://github.com/getsentry/rfcs/blob/main/text/0062-controlling-pii-and-credentials-in-sd-ks.md), [RFC 0038](https://github.com/getsentry/rfcs/blob/main/text/0038-scrubbing-sensitive-data.md): PII and data scrubbing. | ||
| * [RFC 0148](https://github.com/getsentry/rfcs/blob/main/text/0148-logs-for-crashes.md): logs for crashes, which informed the events-vs-logs decision. | ||
|
|
||
| # Appendix A: Part B SDK fit | ||
|
|
||
| | SDK | Fit | Why | | ||
| |---|---|---| | ||
| | Android (Java/Kotlin) | Strongest | `assert` off without `-ea`; gradle plugin already does bytecode weaving | | ||
| | React Native | Strong | idioms stripped by DCE; Metro/Babel (reference impl) | | ||
| | .NET / MAUI / Unity | Strong | `Debug.Assert` removed in Release; Roslyn/IL tooling | | ||
| | Cocoa/Swift | Medium | idiomatic but only Swift macros, opt-in not existing sites | | ||
| | Browser/Node JS | Medium | bundler DCE; Node `assert` already throws | | ||
| | Flutter/Dart | Low | `assert` stripped in release; Dart build hooks only produce native assets, no source transform | | ||
| | Python | Low | `assert` stripped, but AST/import hooks are invasive | | ||
| | Go | N/A | no assert idiom | | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one thing we need to answer here: How does this play with
report()(getsentry/sentry-docs#18422), given it aimed to removecapture*calls.I can see why a more specialized call makes sense here and I wouldn't fully dismiss it, but try to answer these things first:
captureExceptioncall? (i.e. what are we abstracting away with this API)report()?reportAssertionViolation? (again, this hinges on the continuation ofreport()which I don't know the current state of)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch 👍 Probably we can change this to
report(condition, { pragma, message, values })