feat: Use callstack from NSException #5306

itaybre · 2025-05-27T19:24:29Z

📜 Description

Add a new method to collect crashes on _crashOnException and report the stacktrace from the exception instead of the current thread

💡 Motivation and Context

Exception captured on _crashOnException do no collect the proper stacktraces/
Fixes: #5058

💚 How did you test it?

On macOS App

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

TODO:

Update changelog
Add test case

github-actions · 2025-05-27T19:24:53Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 6d8ab6c

codecov · 2025-05-27T19:29:19Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.946%. Comparing base (9c9b0a8) to head (6d8ab6c).
Report is 3 commits behind head on main.

Additional details and impacted files

@@              Coverage Diff              @@
##              main     #5306       +/-   ##
=============================================
- Coverage   85.957%   85.946%   -0.011%     
=============================================
  Files          394       394               
  Lines        34217     34219        +2     
  Branches     14796     14800        +4     
=============================================
- Hits         29412     29410        -2     
+ Misses        4763      4761        -2     
- Partials        42        48        +6

Files with missing lines	Coverage Δ
Sources/Sentry/SentryClient.m	`98.459% <ø> (ø)`
Sources/Sentry/SentrySDK.m	`88.095% <ø> (ø)`
Sources/Sentry/SentryStacktraceBuilder.m	`78.125% <100.000%> (-0.337%)`	⬇️

... and 8 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 9c9b0a8...6d8ab6c. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2025-05-27T19:37:17Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1226.18 ms	1239.35 ms	13.16 ms
Size	23.76 KiB	838.07 KiB	814.31 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`7bc3c0d`	1261.16 ms	1278.38 ms	17.22 ms
`881a955`	1222.94 ms	1246.26 ms	23.32 ms
`c83ffd9`	1235.15 ms	1256.92 ms	21.77 ms
`885c76b`	1227.37 ms	1248.21 ms	20.84 ms
`88aed1e`	1227.83 ms	1245.27 ms	17.43 ms
`c75412c`	1224.90 ms	1254.27 ms	29.37 ms
`326b7eb`	1252.86 ms	1259.56 ms	6.70 ms
`07ea386`	1228.86 ms	1255.31 ms	26.45 ms
`8ef07fb`	1231.45 ms	1243.98 ms	12.53 ms
`8f397a7`	1219.12 ms	1236.67 ms	17.55 ms

App size

Revision	Plain	With Sentry	Diff
`7bc3c0d`	20.76 KiB	427.36 KiB	406.59 KiB
`881a955`	22.85 KiB	407.63 KiB	384.78 KiB
`c83ffd9`	21.58 KiB	573.16 KiB	551.57 KiB
`885c76b`	22.31 KiB	775.27 KiB	752.96 KiB
`88aed1e`	22.30 KiB	829.53 KiB	807.22 KiB
`c75412c`	22.31 KiB	775.27 KiB	752.95 KiB
`326b7eb`	20.76 KiB	432.31 KiB	411.55 KiB
`07ea386`	22.30 KiB	747.52 KiB	725.22 KiB
`8ef07fb`	21.58 KiB	707.42 KiB	685.84 KiB
`8f397a7`	20.76 KiB	420.55 KiB	399.79 KiB

Previous results on branch: itaybre/crash_on_exception

Startup times

Revision	Plain	With Sentry	Diff
`8280f74`	1225.12 ms	1251.00 ms	25.87 ms
`9c37143`	1211.76 ms	1244.86 ms	33.10 ms
`ee78418`	1247.53 ms	1268.06 ms	20.53 ms
`25d4149`	1220.37 ms	1242.23 ms	21.86 ms
`1885f9a`	1226.58 ms	1248.65 ms	22.06 ms
`4b1ac68`	1236.14 ms	1256.84 ms	20.69 ms
`9fc9a23`	1237.35 ms	1264.80 ms	27.45 ms
`2a51821`	1222.06 ms	1234.18 ms	12.12 ms
`f5ba7c1`	1220.90 ms	1245.17 ms	24.28 ms

App size

Revision	Plain	With Sentry	Diff
`8280f74`	23.76 KiB	821.69 KiB	797.93 KiB
`9c37143`	23.76 KiB	837.52 KiB	813.76 KiB
`ee78418`	23.76 KiB	821.47 KiB	797.71 KiB
`25d4149`	23.76 KiB	836.50 KiB	812.74 KiB
`1885f9a`	23.76 KiB	821.48 KiB	797.72 KiB
`4b1ac68`	23.76 KiB	837.51 KiB	813.75 KiB
`9fc9a23`	23.76 KiB	831.33 KiB	807.57 KiB
`2a51821`	23.76 KiB	822.94 KiB	799.18 KiB
`f5ba7c1`	23.76 KiB	837.99 KiB	814.23 KiB

philprime

Good progress, left some comments.

@philipphofmann I would appreciate a review of you as well.

Sources/Sentry/SentryExceptionWrapper.h

Sources/Sentry/SentryExceptionWrapper.m

Sources/Sentry/include/SentryExceptionWrapper.h

Tests/SentryTests/Helper/ExceptionCatcher.h

Tests/SentryTests/Helper/ExceptionCatcher.m

Sources/Sentry/Public/SentrySDK.h

Sources/Sentry/SentryClient.m

Sources/Sentry/SentryExceptionWrapper.m

armcknight

Works and LGTM, just some small suggestions from me.

Sources/Sentry/include/SentryExceptionWrapper.h

Sources/Sentry/SentryExceptionWrapper.m

Sources/Sentry/include/SentryExceptionWrapper.h

maxpoi · 2025-05-30T01:36:16Z

Hi, quick question. Is this only working with SentryApplication? Can "options.enableUncaughtNSExceptionReporting" also benefits from this? Many thanks.

- (void)_crashOnException:(NSException *)exception
{
    [SentrySDK captureException:exception];
    [SentrySDK captureCrashOnException:exception];
    abort();
}

philipphofmann

Thanks for doing this. I added a few comments.

Sources/Sentry/Public/SentrySDK.h

Sources/Sentry/SentryNSExceptionWrapper.m

Sources/Sentry/include/SentryNSExceptionWrapper.h

Sources/Sentry/SentryClient.m

Tests/SentryTests/SentryClientTests.swift

Sources/Sentry/SentryNSExceptionWrapper.m

Sources/Sentry/SentrySDK.m

Sources/Sentry/SentryCrashExceptionApplication.m

Sources/Sentry/SentryClient.m

philipphofmann · 2025-05-30T07:11:30Z

FYI, @itaybre I think after merging this PR we could easily tackle #741 and #3907.

…apper`

…ace has the expected values

…ting and crashing logic + Test for the logic

philipphofmann

Thanks for including most of the feedback. I gave this another round.

Sources/Sentry/include/SentryUseNSExceptionCallstackWrapper.h

Tests/SentryTests/SentryClientTests.swift

Tests/SentryTests/SentryCrashExceptionApplicationTests.swift

Sources/Sentry/SentryUseNSExceptionCallstackWrapper.m

…directly instead of extracting them through a protocol

philipphofmann

Thank you @itaybre. LGTM

…/crash_on_exception

feat: Use callstack from NSException

6d590ff

itaybre added 4 commits May 27, 2025 18:01

Add unit tests

626131e

Don't check userinfo

f7a6e85

Add compiler macros

15db2b2

Update changelog

d3e3551

itaybre marked this pull request as ready for review May 27, 2025 21:21

itaybre requested review from philipphofmann, armcknight, philprime, noahsmartin, trevor-e and NicoHinderling as code owners May 27, 2025 21:21

itaybre added 2 commits May 27, 2025 18:35

Move macro

919eb26

Add missing precompiler macro

d5e6e74

philprime reviewed May 28, 2025

View reviewed changes

itaybre and others added 5 commits May 28, 2025 10:52

Remove headers

2696804

Add additional assertions for thread properties in SentryClientTests

c693de3

Merge branch 'main' into itaybre/crash_on_exception

23c51c9

Fix changelog

ef9ccb6

Merge branch 'main' into itaybre/crash_on_exception

82ea4f6

armcknight approved these changes May 29, 2025

View reviewed changes

Sources/Sentry/include/SentryExceptionWrapper.h Outdated Show resolved Hide resolved

Sources/Sentry/SentryExceptionWrapper.m Outdated Show resolved Hide resolved

Sources/Sentry/include/SentryExceptionWrapper.h Outdated Show resolved Hide resolved

Rename SentryExceptionWrapper to SentryNSExceptionWrapper

34568e7

philipphofmann reviewed May 30, 2025

View reviewed changes

itaybre added 2 commits May 30, 2025 14:11

Move captureCrashOnException to SentrySDK+Private

489381b

Rename SentryNSExceptionWrapper to `SentryUseNSExceptionCallstackWr…

987a906

…apper`

itaybre added 4 commits May 30, 2025 14:48

Improve testCaptureSentryWrappedException test to make sure stacktr…

654a5ca

…ace has the expected values

Add tests for SentryUseNSExceptionCallstackWrapper

a770378

Add SentryCrashExceptionApplicationHelper to handle exception repor…

65b2bfa

…ting and crashing logic + Test for the logic

Exclude tests for non macos

ce02c30

philipphofmann reviewed Jun 2, 2025

View reviewed changes

itaybre and others added 4 commits June 2, 2025 13:14

Improve tests

7f425d5

Refactor SentryUseNSExceptionCallstackWrapper to accept parameters …

28e35aa

…directly instead of extracting them through a protocol

Merge branch 'main' into itaybre/crash_on_exception

15346fb

Improve error message

c733c53

philipphofmann approved these changes Jun 3, 2025

View reviewed changes

itaybre added 7 commits June 3, 2025 11:15

Use Arrance, Act, Assert

b5a6144

Extract duplicated logic

3980028

Merge branch 'main' of github.com:getsentry/sentry-cocoa into itaybre…

fbb31f2

…/crash_on_exception

Update changelog

fdd14c4

Merge branch 'main' of github.com:getsentry/sentry-cocoa into itaybre…

30672df

…/crash_on_exception

Merge branch 'main' of github.com:getsentry/sentry-cocoa into itaybre…

e7df2f5

…/crash_on_exception

Update changelog

6d8ab6c

itaybre merged commit 426a2d3 into main Jun 5, 2025
79 of 81 checks passed

itaybre deleted the itaybre/crash_on_exception branch June 5, 2025 17:21

Uh oh!

feat: Use callstack from NSException #5306

feat: Use callstack from NSException #5306

Uh oh!

Conversation

itaybre commented May 27, 2025

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

TODO:

Uh oh!

github-actions bot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented May 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: itaybre/crash_on_exception

Startup times

App size

Uh oh!

philprime left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

armcknight left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

maxpoi commented May 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

philipphofmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipphofmann commented May 30, 2025

Uh oh!

philipphofmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

philipphofmann left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented May 27, 2025 •

edited

Loading

codecov bot commented May 27, 2025 •

edited

Loading

github-actions bot commented May 27, 2025 •

edited

Loading

maxpoi commented May 30, 2025 •

edited

Loading