Conversation
|
|
||
| Event shape: | ||
|
|
||
| * `mechanism.type = "assertion"`, uniform so the class is filterable regardless of idiom. |
There was a problem hiding this comment.
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.
| ## Part A: convention and runtime API (all SDKs) | ||
|
|
||
| Public API, with idiom-neutral naming: | ||
|
|
||
| ``` | ||
| captureAssertionViolation(condition, { pragma, message, values }) | ||
| ``` |
There was a problem hiding this comment.
one thing we need to answer here: How does this play with report() (getsentry/sentry-docs#18422), given it aimed to remove capture* 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:
- how would a user-facing call look like with today's
captureExceptioncall? (i.e. what are we abstracting away with this API)- how would this look like with
report()?
- how would this look like with
- Should we consider something like
reportAssertionViolation? (again, this hinges on the continuation ofreport()which I don't know the current state of)
There was a problem hiding this comment.
Good catch 👍 Probably we can change this to report(condition, { pragma, message, values })
| * `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) |
There was a problem hiding this comment.
+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.
This RFC proposes turning 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 with unreadable minified messages.It has two parts: a cross-SDK convention and runtime API (
mechanism.type = "assertion"pluscaptureAssertionViolation()), and an opt-in build-time transform that instruments assertion call sites with no app source changes. A working React Native reference implementation exists in getsentry/sentry-react-native#6592.Rendered RFC