Skip to content

feat: Allow users to control Mechanism.Handled for captured exceptions - #5449

Merged
jamescrosswell merged 9 commits into
getsentry:mainfrom
vladbrincoveanu:feat/mechanism-handled-control
Aug 6, 2026
Merged

feat: Allow users to control Mechanism.Handled for captured exceptions#5449
jamescrosswell merged 9 commits into
getsentry:mainfrom
vladbrincoveanu:feat/mechanism-handled-control

Conversation

@vladbrincoveanu

@vladbrincoveanu vladbrincoveanu commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Closes #3383

Implements the approach agreed in this comment: SDK users get explicit control over Mechanism.Handled, and manually captured exceptions default to handled: true.

API changes

New overloads for explicit control:

  • SentrySdk.CaptureException(Exception exception, bool handled, bool terminal = false)
  • SentrySdk.CaptureException(Exception exception, bool handled, bool terminal, Action<Scope> configureScope)
  • SentryClientExtensions.CaptureException(this ISentryClient client, Exception ex, bool handled, bool terminal = false)
  • HubExtensions.CaptureException(this IHub hub, Exception ex, bool handled, bool terminal, Action<Scope> configureScope)

The existing signatures are kept, so this is source- and binary-compatible. handled is a required parameter rather than optional: an optional one would be permanently shadowed by the existing exact-match overloads, and replacing those signatures would be binary-breaking.

Behavior

  • Explicit wins. Passing handled records that value, overriding any flag previously set on the exception (including via SetSentryMechanism).
  • The existing overloads are untouched. They write nothing to Exception.Data, so there is no implicit precedence between a preset flag and a plain CaptureException(ex) call.
  • The default moved into MainExceptionProcessor. Any exception no integration claimed is now marked handled: true, replacing the previous "thrown ⇒ true, never thrown ⇒ null" heuristic. This is what fixes the symptom Allow SDK users to control Mechanism.Handled for captured exceptions #3383 opens with: SentrySdk.CaptureException(new Exception()) reported Handled -- and now reports Handled: true, per the protocol docs ("exceptions captured using capture_exception are handled=true"), which are cited at the decision site. Thrown-and-caught exceptions reported Handled: true before and still do.
  • CaptureExceptionInternal (the integration capture path) defaults to handled: false when no flag is already set, since integrations capture exceptions that user code did not handle. All four call sites set the flag via SetSentryMechanism first (WinUI forwards the platform's value), so the default is a defensive fallback only. Terminal is deliberately left unset there, so a missing SetSentryMechanism call still reads as a crash.
  • Terminal is now controllable too, per the discussion below. It only applies when handled: false; passing handled: true removes any terminal flag left over from an earlier SetSentryMechanism call, mirroring that method's null-removes semantics. The default terminal: false means a manual unhandled capture marks the session as unhandled without ending it as crashed and leaves the active transaction running — a manual capture should not dent the crash-free-sessions rate when nothing actually crashed. terminal: true ends the session as crashed and aborts the active transaction. This is a deliberate divergence from SetSentryMechanism(type, handled: false), which leaves Terminal unset and therefore still counts as a crash. The WinUI integration is left as a follow-up rather than expanding this PR's scope.

Testing

Unit tests cover explicit true/false on every capture path, explicit values overriding preset flags, the untouched plain overloads, and the integration-path default. Terminal coverage asserts the session outcome for each combination (ReportError, MarkSessionAsUnhandled, EndSession(Crashed)) and that only terminal captures abort the active transaction. Verify snapshots updated: API approvals for all four target frameworks, the aggregate-exception mechanism snapshot, and the Sentry.DiagnosticSource / Sentry.EntityFramework integration snapshots, where constructed-and-captured exceptions now report Handled: true.

@codecov

codecov Bot commented Jul 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.74%. Comparing base (6516897) to head (6b50143).
⚠️ Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5449      +/-   ##
==========================================
+ Coverage   74.62%   74.74%   +0.11%     
==========================================
  Files         512      513       +1     
  Lines       18672    18744      +72     
  Branches     3657     3666       +9     
==========================================
+ Hits        13934    14010      +76     
+ Misses       3865     3862       -3     
+ Partials      873      872       -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.

@vladbrincoveanu
vladbrincoveanu force-pushed the feat/mechanism-handled-control branch 2 times, most recently from 57b519e to ce1f8c9 Compare July 24, 2026 11:29
@vladbrincoveanu
vladbrincoveanu marked this pull request as ready for review July 25, 2026 04:21
@github-actions github-actions Bot added the risk: medium PR risk score: medium label Jul 25, 2026
Comment thread src/Sentry/Internal/MainExceptionProcessor.cs Outdated
@vladbrincoveanu

vladbrincoveanu commented Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

We have a warden failing check.
Could you please have a look @jamescrosswell ?
Warden cannot review this fork pull request because GitHub withholds repository secrets from fork workflows
Thanks.

vladbrincoveanu added a commit to vladbrincoveanu/sentry-dotnet that referenced this pull request Jul 25, 2026
The Handled-inheritance change in this branch made inner exceptions inherit
Handled=false from a non-terminal unhandled outer (e.g. an unobserved-task
AggregateException marked terminal:false). Terminal was not inherited, so after
the processor strips the outer Data keys, HasUnhandledTerminalException matched
the inner (Handled:false, Terminal:null) and IsFromTerminalException() wrongly
returned true.

Propagate Terminal down the chain symmetric to Handled so the chain stays
internally consistent. Terminal is an SDK-internal, non-serialized flag consumed
only by the Has*TerminalException scans, so this has no wire/protocol impact.
Explicit Terminal flags on any exception still win.

Regression flagged by Cursor Bugbot on PR getsentry#5449.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Comment thread src/Sentry/SentryClientExtensions.cs
@vladbrincoveanu
vladbrincoveanu force-pushed the feat/mechanism-handled-control branch 2 times, most recently from 6345138 to 801db87 Compare July 25, 2026 06:07
@vladbrincoveanu
vladbrincoveanu marked this pull request as draft July 25, 2026 19:14
getsentry#3383)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vladbrincoveanu
vladbrincoveanu force-pushed the feat/mechanism-handled-control branch from 061f5d3 to 0540a78 Compare July 26, 2026 04:48
vladbrincoveanu and others added 4 commits July 26, 2026 08:33
Adds SentrySdk-level coverage for the handled parameter, including that an
explicit argument overrides a flag previously set via SetSentryMechanism.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The non-handled CaptureException overloads no longer default or preserve a
handled flag - they behave exactly as before this feature. Only the new
overloads taking an explicit `handled` argument write Mechanism.HandledKey,
and CaptureExceptionInternal keeps forcing handled: false for integrations.

Tests and the DiagnosticSource/EntityFramework snapshots are updated to match.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
MainExceptionProcessor now marks any exception no integration claimed as
handled, instead of inferring it from the presence of a stack trace and
leaving never-thrown exceptions as null. This fixes the symptom getsentry#3383 opens
with - SentrySdk.CaptureException(new Exception()) reported "Handled --" -
and matches the relay protocol docs, which are now cited at the decision site.

Doing it in the processor keeps the capture overloads free of any implicit
precedence: the plain overloads still write nothing to Exception.Data, and an
explicit handled argument (or SetSentryMechanism) always wins.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The preceding two sentences restated what the quoted Relay docs already
say, so keep only the authoritative reference.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@vladbrincoveanu
vladbrincoveanu marked this pull request as ready for review July 26, 2026 12:15

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1d662b6. Configure here.

Comment thread src/Sentry/HubExtensions.cs
Comment thread src/Sentry/HubExtensions.cs Outdated
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread src/Sentry/Internal/MainExceptionProcessor.cs
…Internal

WinUIUnhandledExceptionIntegration forwards the platform's Handled value via
SetSentryMechanism before capturing; stamping false unconditionally clobbered
it. Only default to unhandled when no flag was declared.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@jamescrosswell jamescrosswell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@vladbrincoveanu thank you very much for the PR... it does raise some tricky problems though - see my inline comments below.

// https://getsentry.github.io/relay/relay_event_schema/protocol/struct.Mechanism.html#structfield.handled
// "Exceptions captured using capture_exception (called from user code) are handled=true as the user
// explicitly captured the exception (and therefore kind of handled it)."
mechanism.Handled = true;

@jamescrosswell jamescrosswell Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This would be a change in behaviour. I think it probably makes sense... we can probably do it without making a major release - arguably it's a bug fix rather than an arbitrary change.

@dingsdax do you have any steer?

Comment thread src/Sentry/HubExtensions.cs Outdated
Comment thread src/Sentry/HubExtensions.cs Outdated
Comment thread src/Sentry/SentrySdk.cs Outdated
Comment thread src/Sentry/SentryClientExtensions.cs
vladbrincoveanu and others added 2 commits July 31, 2026 13:53
Addresses review feedback on getsentry#5449:
- reorder the scope-callback overloads to (ex, handled, configureScope)
  so a long lambda stays the last argument
- skip the Exception.Data mutation when the hub is disabled, matching
  SentryClientExtensions.CaptureException
- note that the CaptureExceptionInternal fallback is defensive only

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…apshots

The 2-arg (handled, configureScope) overload on HubExtensions/SentrySdk was
introduced earlier in this branch and superseded by the (handled, terminal,
configureScope) overload; both are unreleased so removing it needs no
back-compat shim. Callers now pass terminal explicitly. Also brings the
net8.0/net9.0/net4.8 API-approval snapshots in line with net10.0, which had
already dropped the overload.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@github-actions github-actions Bot added risk: high PR risk score: high and removed risk: medium PR risk score: medium labels Aug 4, 2026

@jamescrosswell jamescrosswell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@vladbrincoveanu huge thank you for the PR - it's a meaningful improvement to the SDK 🙏🏻

@jamescrosswell
jamescrosswell merged commit 1733cbd into getsentry:main Aug 6, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: high PR risk score: high

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Allow SDK users to control Mechanism.Handled for captured exceptions

2 participants