Skip to content

fix: Capture instance and class method [NSApplication _crashOnException] exceptions#7510

Merged
denrase merged 16 commits into
mainfrom
repro/cocoa-881-crashonexception-instead-of-st
Mar 10, 2026
Merged

fix: Capture instance and class method [NSApplication _crashOnException] exceptions#7510
denrase merged 16 commits into
mainfrom
repro/cocoa-881-crashonexception-instead-of-st

Conversation

@denrase

@denrase denrase commented Feb 24, 2026

Copy link
Copy Markdown
Collaborator

📜 Description

Fix macOS NSException crash reports showing _crashOnException: in the stacktrace instead of the actual exception callstack.

  • Replace async captureCrashOnException: with synchronous uncaughtExceptionHandler so the crash report is written before abort() terminates the process
  • Swizzle both class and instance _crashOnException: to cover different macOS versions
  • Deduplicate capture: reportException: internally calls _crashOnException: (when NSApplicationCrashOnExceptions is YES, which Sentry sets), so both paths now go through SentryNSExceptionCaptureHelper which ensures a single capture
  • Remove no unused code: captureCrashOnException:, SentryUseNSExceptionCallstackWrapper

Fixes both the subclass approach (SentryCrashExceptionApplication) and the swizzle approach (enableUncaughtNSExceptionReporting).

Closes #6620

💚 How did you test it?

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

@linear

linear Bot commented Feb 24, 2026

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Feb 24, 2026

Copy link
Copy Markdown
Contributor

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


New Features ✨

  • (SPM) Add package traits for UI framework opt-out and macOS CLI sample by philprime in #7578
  • Use full flamegraph for metrickit app hangs by noahsmartin in #7185

Bug Fixes 🐛

  • Capture instance and class method [NSApplication \_crashOnException] exceptions by denrase in #7510
  • Capture transactions during launch profiling window by philipphofmann in #7602

Internal Changes 🔧

Deps

  • Bump fastlane-plugin-sentry from 2.1.1 to 2.2.0 by dependabot in #7626
  • Bump mikepenz/action-junit-report from 6.3.0 to 6.3.1 by dependabot in #7630
  • Bump actions/setup-node from 6.2.0 to 6.3.0 by dependabot in #7627
  • Bump getsentry/craft/.github/workflows/changelog-preview.yml from 2.23.0 to 2.23.2 by dependabot in #7628
  • Bump getsentry/craft from 2.23.0 to 2.23.2 by dependabot in #7629
  • Bump ruby/setup-ruby from 1.288.0 to 1.290.0 by dependabot in #7631

Samples

  • Restructure iOS-SwiftUI-Widgets sample by philprime in #7658
  • Restructure iOS15-SwiftUI sample by philprime in #7657
  • Restructure watchOS-Swift sample by philprime in #7614
  • Restructure visionOS-Swift sample by philprime in #7616
  • Restructure visionOS-SwiftUI-SPM sample by philprime in #7615

Other

  • (ci) Adds a Nightly Job with unit tests for less frequent devices by itaybre in #7632
  • (test-samples) Restructure SwiftUITestSample and SwiftUICrashTest by philprime in #7612
  • Fix watchOS tests and add them to nightly job by itaybre in #7633
  • Add crash when SentryInitializeForGettingSubclassesNotCalled is… by noahsmartin in #7637
  • Fix flaky HangTracker deallocated test by noahsmartin in #7639

🤖 This preview updates automatically when you update the PR.

Comment thread Sources/Sentry/SentryCrashExceptionApplicationHelper.m
Comment thread Sources/Sentry/SentrySDKInternal.m
@denrase denrase marked this pull request as ready for review February 24, 2026 13:04
Copilot AI review requested due to automatic review settings February 24, 2026 13:04
Comment thread Sources/Sentry/SentryCrashExceptionApplication.m
@denrase

denrase commented Feb 24, 2026

Copy link
Copy Markdown
Collaborator Author

If calling the class function +[NSApplication _crashOnException:] is indeed the way to go for the runtime from here on out, only the swizzle approach will work correctly, as we can/should not override this class function in our custom subclass.

@denrase denrase added the ready-to-merge Use this label to trigger all PR workflows label Feb 24, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes macOS NSException crash reports that were showing _crashOnException: in the stacktrace instead of the actual exception callstack. The root cause was twofold: (1) the async capture path never completed before abort() terminated the process, and (2) newer macOS versions call the class method instead of the instance method, so the original swizzle/override didn't work.

Changes:

  • Replace async captureCrashOnException: path with synchronous uncaughtExceptionHandler to capture crash reports before process termination
  • Swizzle both class and instance methods of _crashOnException: to support different macOS versions
  • Add instance method override in SentryCrashExceptionApplication for the subclass approach

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
Tests/SentryTests/SentryCrashExceptionApplicationTests.swift Removed obsolete test for deleted async crash method
Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift Added tests for both class and instance method swizzling
Sources/Swift/Integrations/SentryCrash/SentryCrashIntegration.swift Enabled swizzling of _crashOnException: method
Sources/Sentry/include/SentryUncaughtNSExceptions.h Added public method declaration for new swizzling
Sources/Sentry/include/SentrySDK+Private.h Removed obsolete async capture method declaration
Sources/Sentry/include/SentryCrashExceptionApplicationHelper.h Removed obsolete _crashOnException: method declaration
Sources/Sentry/SentryUncaughtNSExceptions.m Implemented dual swizzling for class and instance methods
Sources/Sentry/SentrySDKInternal.m Removed implementation of obsolete async capture method
Sources/Sentry/SentryCrashExceptionApplicationHelper.m Removed implementation of obsolete method and unused imports
Sources/Sentry/SentryCrashExceptionApplication.m Added instance method override calling synchronous handler
Samples/SentrySampleShared/SentrySampleShared/SentrySDKWrapper.swift Enabled uncaught exception reporting for testing
CHANGELOG.md Added entry for the fix

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Sources/Sentry/SentryCrashExceptionApplication.m
@codecov

codecov Bot commented Feb 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@316a9d6). Learn more about missing BASE report.
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff            @@
##             main     #7510   +/-   ##
========================================
  Coverage        ?   85.350%           
========================================
  Files           ?       483           
  Lines           ?     28785           
  Branches        ?     12505           
========================================
  Hits            ?     24568           
  Misses          ?      4170           
  Partials        ?        47           
Files with missing lines Coverage Δ
Sources/Sentry/SentryClient.m 98.905% <ø> (ø)
Sources/Sentry/SentrySDKInternal.m 84.037% <ø> (ø)
...egrations/SentryCrash/SentryCrashIntegration.swift 96.721% <ø> (ø)

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 316a9d6...108fa16. Read the comment docs.

@itaybre itaybre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this, LGTM

Comment thread Sources/Sentry/SentryCrashExceptionApplication.m
Comment thread Sources/Sentry/SentryCrashExceptionApplicationHelper.m
Comment thread Sources/Sentry/SentrySDKInternal.m

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

Autofix Details

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Class method swizzle stacks on repeated SDK starts
    • Made swizzleNSApplicationCrashOnException idempotent with dispatch_once so repeated SDK starts no longer stack class-method swizzles, and added a regression test covering repeated calls.

Create PR

Or push these changes by commenting:

@cursor push ca91921bf6
Preview (ca91921bf6)
diff --git a/Sources/Sentry/SentryUncaughtNSExceptions.m b/Sources/Sentry/SentryUncaughtNSExceptions.m
--- a/Sources/Sentry/SentryUncaughtNSExceptions.m
+++ b/Sources/Sentry/SentryUncaughtNSExceptions.m
@@ -40,34 +40,37 @@
 #    if SENTRY_TEST || SENTRY_TEST_CI
 #        pragma clang diagnostic ignored "-Wunused-variable"
 #    endif
-    // AppKit calls _crashOnException: when an exception is caught during CATransaction flush
-    // or view layout, bypassing reportException: entirely. Depending on macOS version, AppKit
-    // may call the class method +[NSApplication _crashOnException:] or the instance method
-    // -[NSApp _crashOnException:], so we swizzle both.
-    SEL selector = NSSelectorFromString(@"_crashOnException:");
+    static dispatch_once_t onceToken;
+    dispatch_once(&onceToken, ^{
+        // AppKit calls _crashOnException: when an exception is caught during CATransaction flush
+        // or view layout, bypassing reportException: entirely. Depending on macOS version, AppKit
+        // may call the class method +[NSApplication _crashOnException:] or the instance method
+        // -[NSApp _crashOnException:], so we swizzle both.
+        SEL selector = NSSelectorFromString(@"_crashOnException:");
 
-    SentrySwizzleClassMethod(NSApplication, selector, SentrySWReturnType(void),
-        SentrySWArguments(NSException * exception), SentrySWReplacement({
-            [SentryUncaughtNSExceptions capture:exception];
+        SentrySwizzleClassMethod(NSApplication, selector, SentrySWReturnType(void),
+            SentrySWArguments(NSException * exception), SentrySWReplacement({
+                [SentryUncaughtNSExceptions capture:exception];
 #    if SENTRY_TEST || SENTRY_TEST_CI
-            // Don't call the original in tests as it would abort() the process.
-            swizzleInfo.originalCalled = YES;
+                // Don't call the original in tests as it would abort() the process.
+                swizzleInfo.originalCalled = YES;
 #    else
-            return SentrySWCallOriginal(exception);
+                return SentrySWCallOriginal(exception);
 #    endif
-        }));
+            }));
 
-    SentrySwizzleInstanceMethod(NSApplication, selector, SentrySWReturnType(void),
-        SentrySWArguments(NSException * exception), SentrySWReplacement({
-            [SentryUncaughtNSExceptions capture:exception];
+        SentrySwizzleInstanceMethod(NSApplication, selector, SentrySWReturnType(void),
+            SentrySWArguments(NSException * exception), SentrySWReplacement({
+                [SentryUncaughtNSExceptions capture:exception];
 #    if SENTRY_TEST || SENTRY_TEST_CI
-            // Don't call the original in tests as it would abort() the process.
-            swizzleInfo.originalCalled = YES;
+                // Don't call the original in tests as it would abort() the process.
+                swizzleInfo.originalCalled = YES;
 #    else
-            return SentrySWCallOriginal(exception);
+                return SentrySWCallOriginal(exception);
 #    endif
-        }),
-        SentrySwizzleModeOncePerClassAndSuperclasses, (void *)selector);
+            }),
+            SentrySwizzleModeOncePerClassAndSuperclasses, (void *)selector);
+    });
 #    pragma clang diagnostic pop
 }
 

diff --git a/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift b/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift
--- a/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift
+++ b/Tests/SentryTests/Integrations/SentryCrash/SentryUncaughtNSExceptionsTests.swift
@@ -51,7 +51,31 @@
 
         XCTAssertTrue(wasUncaughtExceptionHandlerCalled)
     }
+    
+    func testSwizzleNSApplicationCrashOnException_whenCalledMultipleTimes_shouldCaptureClassMethodOnce() throws {
+        let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter
 
+        defer {
+            crashReporter.uncaughtExceptionHandler = nil
+            wasUncaughtExceptionHandlerCalled = false
+            uncaughtExceptionHandlerCallCount = 0
+        }
+
+        wasUncaughtExceptionHandlerCalled = false
+        uncaughtExceptionHandlerCallCount = 0
+        crashReporter.uncaughtExceptionHandler = uncaughtExceptionHandler
+
+        SentryUncaughtNSExceptions.swizzleNSApplicationCrashOnException()
+        SentryUncaughtNSExceptions.swizzleNSApplicationCrashOnException()
+
+        // Call the class method +[NSApplication _crashOnException:].
+        // In tests, SentrySWCallOriginal is skipped so this won't abort().
+        NSApplication.perform(NSSelectorFromString("_crashOnException:"), with: uncaughtInternalInconsistencyException)
+
+        XCTAssertTrue(wasUncaughtExceptionHandlerCalled)
+        XCTAssertEqual(1, uncaughtExceptionHandlerCallCount)
+    }
+
     func testSwizzleNSApplicationCrashOnException_instanceMethod() throws {
         let crashReporter = SentryDependencyContainer.sharedInstance().crashReporter
 
@@ -108,10 +132,12 @@
 // We need to declare this on the file level because otherwise we get the error:
 // A C function pointer cannot be formed from a closure that captures context.
 var wasUncaughtExceptionHandlerCalled = false
+var uncaughtExceptionHandlerCallCount = 0
 func uncaughtExceptionHandler(exception: NSException) {
     XCTAssertEqual(uncaughtInternalInconsistencyException.name, exception.name)
     XCTAssertEqual(uncaughtInternalInconsistencyException.reason, exception.reason)
     wasUncaughtExceptionHandlerCalled = true
+    uncaughtExceptionHandlerCallCount += 1
 }
 
 let uncaughtInternalInconsistencyException = NSException(name: .internalInconsistencyException, reason: "reason", userInfo: nil)
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread Sources/Sentry/SentryUncaughtNSExceptions.m Outdated
@github-actions

github-actions Bot commented Mar 2, 2026

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1208.04 ms 1242.38 ms 34.33 ms
Size 24.14 KiB 1.12 MiB 1.10 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
795e128 1216.68 ms 1245.83 ms 29.15 ms
0e4a9dc 1222.92 ms 1255.78 ms 32.86 ms
7e30a5e 1214.57 ms 1247.78 ms 33.21 ms
79e2bb8 1216.37 ms 1242.42 ms 26.05 ms
64e79c5 1217.59 ms 1243.98 ms 26.39 ms
6108e4c 1212.31 ms 1251.80 ms 39.49 ms
84bc307 1211.52 ms 1237.09 ms 25.56 ms
fee8669 1220.50 ms 1231.84 ms 11.34 ms
80963bd 1194.76 ms 1212.13 ms 17.38 ms
e03f459 1222.56 ms 1255.94 ms 33.37 ms

App size

Revision Plain With Sentry Diff
795e128 24.14 KiB 1.10 MiB 1.08 MiB
0e4a9dc 24.14 KiB 1.11 MiB 1.09 MiB
7e30a5e 24.14 KiB 1.10 MiB 1.08 MiB
79e2bb8 24.14 KiB 1.04 MiB 1.02 MiB
64e79c5 24.14 KiB 1.11 MiB 1.09 MiB
6108e4c 24.14 KiB 1.11 MiB 1.09 MiB
84bc307 24.14 KiB 1.08 MiB 1.06 MiB
fee8669 24.14 KiB 1.10 MiB 1.08 MiB
80963bd 24.14 KiB 1.11 MiB 1.09 MiB
e03f459 24.14 KiB 1.11 MiB 1.09 MiB

Previous results on branch: repro/cocoa-881-crashonexception-instead-of-st

Startup times

Revision Plain With Sentry Diff
fc934bc 1197.49 ms 1213.40 ms 15.91 ms
ac652a2 1222.86 ms 1245.84 ms 22.98 ms
181ff60 1232.25 ms 1256.15 ms 23.90 ms

App size

Revision Plain With Sentry Diff
fc934bc 24.14 KiB 1.12 MiB 1.10 MiB
ac652a2 24.14 KiB 1.12 MiB 1.10 MiB
181ff60 24.14 KiB 1.12 MiB 1.10 MiB

Comment thread Sources/Sentry/SentryNSExceptionCaptureHelper.m
Comment thread Sources/Sentry/SentryNSExceptionCaptureHelper.m
@denrase denrase requested a review from itaybre March 2, 2026 15:55
@denrase

denrase commented Mar 2, 2026

Copy link
Copy Markdown
Collaborator Author

@itaybre I have added your feedback as well as the ones from the agents (de-duplication). Would you care to re-review this? I checked again with our sample app and AFAIK we do the correct thing now, you can double-check here: https://denrase.sentry.io/issues/7287964162/?project=4508007036551168&query=is%3Aunresolved&referrer=issue-stream thank you! 🙇

# Conflicts:
#	Tests/SentryTests/SentryClientTests.swift
Comment thread Sources/Sentry/SentryCrashExceptionApplication.m

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

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Unused import of SentryCrashExceptionApplication.h
    • Removed the unused SentryCrashExceptionApplication.h import from SentryNSExceptionCaptureHelper.m to eliminate the unnecessary dependency.

Create PR

Or push these changes by commenting:

@cursor push bc30c717f6
Preview (bc30c717f6)
diff --git a/Sources/Sentry/SentryNSExceptionCaptureHelper.m b/Sources/Sentry/SentryNSExceptionCaptureHelper.m
--- a/Sources/Sentry/SentryNSExceptionCaptureHelper.m
+++ b/Sources/Sentry/SentryNSExceptionCaptureHelper.m
@@ -3,7 +3,6 @@
 #if TARGET_OS_OSX && !SENTRY_NO_UI_FRAMEWORK
 
 #    import "SentryCrash.h"
-#    import "SentryCrashExceptionApplication.h"
 #    import "SentryNSExceptionCaptureHelper.h"
 #    import "SentrySwift.h"
This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.

Comment thread Sources/Sentry/SentryNSExceptionCaptureHelper.m Outdated
@sentry

sentry Bot commented Mar 10, 2026

Copy link
Copy Markdown

Sentry Build Distribution

App Version Configuration
App 9.6.0 (1) Release

@sentry

sentry Bot commented Mar 10, 2026

Copy link
Copy Markdown

Sentry Build Distribution

App Version Configuration
App 9.6.0 (1) Release

Comment thread Sources/Sentry/SentryNSExceptionCaptureHelper.m

@itaybre itaybre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Almost LGTM, just a small comment + changelog entry

Comment thread CHANGELOG.md Outdated
@denrase denrase enabled auto-merge (squash) March 10, 2026 14:05
@sentry

sentry Bot commented Mar 10, 2026

Copy link
Copy Markdown

Sentry Build Distribution

App Version Configuration
App 9.6.0 (1) Release

@denrase denrase merged commit d6d7fcf into main Mar 10, 2026
208 checks passed
@denrase denrase deleted the repro/cocoa-881-crashonexception-instead-of-st branch March 10, 2026 15:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge Use this label to trigger all PR workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Stacktrace shows +[SentryCrashExceptionApplicationHelper _crashOnException:] instead of actual stacktrace

3 participants