Skip to content

fix: Set handled to false for fatal app hangs #5514

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jun 27, 2025
Merged

fix: Set handled to false for fatal app hangs #5514

merged 1 commit into from
Jun 27, 2025

Conversation

philipphofmann
Copy link
Member

📜 Description

Fatal app hangs didn't have mechanism.handled = false, which should be the case because it's an unhandled error.

💡 Motivation and Context

A customer brought this up.

💚 How did you test it?

Unit tests and triggering a fatal app hang via simulator: https://sentry-sdks.sentry.io/issues/6675226860/events/e75fd272f1604537b5a8b3d9e6e7b99f/?project=5428557

Screenshot 2025-06-27 at 14 24 50

📝 Checklist

You have to check all boxes before merging:

  • 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.

@philipphofmann
fix: Set handled to false for fatal app hangs
a050ca4 
Fatal app hangs didn't have mechanism.handled = false, which should be
the case because it's an unhandled error.
@philipphofmann
Copy link
Member Author

@sentry review

@seer-by-sentry Seer by Sentry
Copy link

On it! We are reviewing the PR and will provide feedback shortly.

@codecov Codecov
Copy link

codecov bot commented Jun 27, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.249%. Comparing base (acac774) to head (a050ca4).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5514       +/-   ##
=============================================
+ Coverage   86.141%   86.249%   +0.107%     
=============================================
  Files          402       403        +1     
  Lines        34766     34908      +142     
  Branches     15026     15157      +131     
=============================================
+ Hits         29948     30108      +160     
+ Misses        4771      4757       -14     
+ Partials        47        43        -4
Files with missing lines Coverage Δ
Sources/Sentry/SentryANRTrackingIntegration.m 99.382% <100.000%> (+0.003%) ⬆️

... and 27 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acac774...a050ca4. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@seer-by-sentry Seer by Sentry
Copy link

PR Description

This pull request addresses an issue where fatal app hang events were not being correctly categorized as unhandled exceptions within Sentry. The goal is to ensure that these events are properly flagged, allowing for more accurate error tracking and prioritization.

Click to see more

Key Technical Changes

The primary technical change involves setting the handled property of the SentryMechanism associated with the exception to false when capturing a stored app hang event that is determined to be fatal. This is achieved by adding the line exception.mechanism.handled = @(NO); in SentryANRTrackingIntegration.m. Corresponding tests in SentryANRTrackingIntegrationTests.swift have been updated to assert that this property is indeed set to false for fatal app hang events.

Architecture Decisions

The decision to set handled = @(NO) directly within the captureStoredAppHangEvent method reflects the understanding that fatal app hangs, resulting from user or OS watchdog termination, are inherently unrecoverable from the application's perspective. This approach avoids the need for more complex logic to determine the 'handled' status based on other factors.

Dependencies and Interactions

This change primarily affects the SentryANRTrackingIntegration and its interaction with the SentryEvent, SentryException, and SentryMechanism objects. It does not introduce any new external dependencies. The change interacts with the existing ANR tracking logic, specifically the part that processes stored app hang events after a potential crash.

Risk Considerations

The main risk is the potential for unintended consequences if other parts of the Sentry SDK rely on the default handled status of exceptions. However, the tests added in this PR should mitigate this risk by ensuring that the intended behavior is consistently enforced. There is also a small risk that setting handled = @(NO) could interfere with user-defined exception handling logic, but this is unlikely given that fatal app hangs are generally unrecoverable.

Notable Implementation Details

The implementation directly modifies the handled property of the SentryMechanism. The tests use XCTUnwrap to safely access the exception and its mechanism before asserting the value of the handled property. The tests cover the scenario where the app hang is detected and then results in a fatal event on the next launch.

seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 27, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 27, 2025
View reviewed changes
seer-by-sentry[bot]
seer-by-sentry bot reviewed Jun 27, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1233.63 ms 1251.02 ms 17.39 ms
Size 23.75 KiB 866.68 KiB 842.93 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
db9572a 1223.13 ms 1241.60 ms 18.47 ms
db9572a 1212.61 ms 1237.73 ms 25.13 ms
5cfc768 1220.74 ms 1245.06 ms 24.32 ms
35c962f 1207.61 ms 1235.90 ms 28.29 ms
8ea5293 1242.70 ms 1262.25 ms 19.55 ms
8047b99 1226.37 ms 1246.63 ms 20.26 ms
d38165b 1211.41 ms 1242.49 ms 31.08 ms
b13e93a 1236.24 ms 1247.33 ms 11.08 ms
acac774 1217.76 ms 1253.29 ms 35.52 ms
db9572a 1200.27 ms 1234.80 ms 34.53 ms

App size

Revision Plain With Sentry Diff
db9572a 23.75 KiB 858.64 KiB 834.89 KiB
db9572a 23.75 KiB 858.65 KiB 834.90 KiB
5cfc768 23.75 KiB 850.73 KiB 826.98 KiB
35c962f 23.75 KiB 854.77 KiB 831.02 KiB
8ea5293 23.75 KiB 852.24 KiB 828.49 KiB
8047b99 23.75 KiB 855.37 KiB 831.62 KiB
d38165b 23.75 KiB 855.37 KiB 831.62 KiB
b13e93a 23.75 KiB 855.37 KiB 831.62 KiB
acac774 23.75 KiB 866.51 KiB 842.76 KiB
db9572a 23.75 KiB 858.69 KiB 834.93 KiB

@philipphofmann philipphofmann merged commit 581ef4b into main Jun 27, 2025
127 checks passed
@philipphofmann philipphofmann deleted the fix/mark-fatal-app-hangs-unhandled branch June 27, 2025 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@seer-by-sentry seer-by-sentry[bot] seer-by-sentry[bot] left review comments

@itaybre itaybre itaybre approved these changes

@armcknight armcknight Awaiting requested review from armcknight

@philprime philprime Awaiting requested review from philprime philprime is a code owner

@noahsmartin noahsmartin Awaiting requested review from noahsmartin noahsmartin is a code owner

@trevor-e trevor-e Awaiting requested review from trevor-e

@NicoHinderling NicoHinderling Awaiting requested review from NicoHinderling

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@philipphofmann @itaybre