Skip to content

feat!: add support for crash-time attachments via hint - #2112

Open
jpnurmi wants to merge 2 commits into
jpnurmi/feat/hintsfrom
jpnurmi/feat/crash-hints
Open

jpnurmi wants to merge 2 commits into
jpnurmi/feat/hintsfrom
jpnurmi/feat/crash-hints

Conversation

@jpnurmi

@jpnurmi jpnurmi commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

Merge attachments and pass as hint to on_crash, then use the resulting hint attachments in the envelope.

Caution

BREAKING CHANGE: on_crash callbacks now have an additional sentry_hint_t *hint parameter.

Close: #2098

@jpnurmi
jpnurmi added this pull request to stack #2113 September 18, 2026 16:49
@jpnurmi
jpnurmi marked this pull request as draft September 18, 2026 16:49

@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 3 potential issues.

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 d30860a. Configure here.

Comment thread src/backends/sentry_backend_crashpad.cpp
Comment thread src/backends/sentry_backend_native.c
Comment thread src/backends/sentry_backend_inproc.c
@codecov

codecov Bot commented Sep 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 60.41667% with 76 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.35%. Comparing base (ce10a77) to head (d8f1a2a).

Additional details and impacted files
@@                  Coverage Diff                   @@
##           jpnurmi/feat/hints    #2112      +/-   ##
======================================================
+ Coverage               75.20%   75.35%   +0.14%     
======================================================
  Files                     103      104       +1     
  Lines                   27936    28069     +133     
  Branches                 5102     5126      +24     
======================================================
+ Hits                    21008    21150     +142     
+ Misses                   5600     5587      -13     
- Partials                 1328     1332       +4     
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jpnurmi
jpnurmi force-pushed the jpnurmi/feat/crash-hints branch 2 times, most recently from fb6c4ac to 579efa4 Compare September 21, 2026 14:53
@jpnurmi
jpnurmi force-pushed the jpnurmi/feat/crash-hints branch from 579efa4 to 902cae1 Compare September 21, 2026 16:10
@jpnurmi
jpnurmi marked this pull request as ready for review September 21, 2026 16:21
@jpnurmi
jpnurmi force-pushed the jpnurmi/feat/crash-hints branch from d8f1a2a to 6f6faf1 Compare September 21, 2026 19:23
Comment on lines 1240 to +1243
sentry_backend_t *backend, const sentry_options_t *options)
{
native_backend_state_t *state = (native_backend_state_t *)backend->data;
if (!state || !state->event_path) {
if (!state || !state->event_path || sentry__atomic_fetch(&state->crashed)) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The early-return guard sentry__atomic_fetch(&state->crashed) in native_backend_flush_scope prevents scope-level attachments added from an on_crash_func callback from being written, as the crashed state is set before the callback runs.
Severity: HIGH

Suggested Fix

Revert the order of operations in native_backend_flush_scope. The attachment manifest should be written before checking the state->crashed flag. This ensures that attachments added from within an on_crash_func are correctly persisted, restoring the behavior that was explicitly supported in the previous version.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: src/backends/sentry_backend_native.c#L1240-L1243

Potential issue: In `native_backend_flush_scope`, the new early-return guard checks
`sentry__atomic_fetch(&state->crashed)` before writing the attachment manifest. The
`crashed` state is set before the `on_crash_func` is invoked. Consequently, if
`sentry_add_attachment()` is called from within an `on_crash_func` callback, it triggers
`native_backend_flush_scope`, which now returns early. This causes the scope-level
attachment to be silently dropped. The previous implementation intentionally wrote the
manifest before this check to support this exact use case, so this change is a
regression.

Did we get this right? 👍 / 👎 to inform future reviews.

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.

Feature request: hints

1 participant