fix(ios): Filter ExceptionsManager.reportException duplicates in app-start init#6144
Closed
alwx wants to merge 1 commit into
Closed
fix(ios): Filter ExceptionsManager.reportException duplicates in app-start init#6144alwx wants to merge 1 commit into
alwx wants to merge 1 commit into
Conversation
…start init When the native SDK is initialized early via sentry.options.json (the v8 "Capture App Start Errors" feature, #5582), unhandled JS errors on React Native's New Architecture are wrapped in a C++ exception and captured by the native crash handler. The deduplication added in #5532 / 7.9.0 lived only in RNSentry.mm's prepareOptions (the JS-initiated init path), so the RNSentryStart.updateWithReactFinals path used by the file-based and `autoInitializeNativeSdk: false` flows let the C++ wrapper through and users saw two events per JS error — one JS and one C++. Port the `ExceptionsManager.reportException` value-based filter into `updateWithReactFinals` so both init paths apply the same dedup. Mirror the test coverage in RNSentryStartTests.swift, exercising both `RNSentrySDK.start(configureOptions:)` and the dictionary-based `RNSentryStart.start(options:)` (file-based) paths. Android is unaffected: `updateWithReactDefaults` registers `addIgnoredExceptionForType(JavascriptException.class)` which is shared by both Android init paths. Fixes #6116.
Contributor
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
Contributor
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
### Fixes
- Filter ExceptionsManager.reportException duplicates in app-start init ([#6144](https://github.com/getsentry/sentry-react-native/pull/6144))If none of the above apply, you can opt out of this check by adding |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
📢 Type of change
📜 Description
Port the
ExceptionsManager.reportExceptiondedup filter intoRNSentryStart.updateWithReactFinalsso the file-based / early-native init path filters the same New Architecture C++ wrapper events as the JS-initiated path.Background: there are two iOS
beforeSendfilter sites that both exist to drop JS errors the JS SDK already reported:RNSentry.mm→prepareOptions:— used when JS callsinitNativeSdk(defaultautoInitializeNativeSdk: true). Filters both"Unhandled JS Exception"types and any exception whose value contains"ExceptionsManager.reportException"(the New Architecture C++ wrapper, added in fix(ios): Fix duplicate JS error reporting on iOS with New Architecture #5532 / 7.9.0 to fix iOS Duplicate JS unhandled issues in new architecture #5529).RNSentryStart.m→updateWithReactFinals:— used when the native SDK is started early viasentry.options.json(the v8 "Capture App Start Errors" feature, feat: v8: Capture app start errors before JS #5582) and whenautoInitializeNativeSdk: false. Filtered only"Unhandled JS Exception"types.The C++ wrapper filter from #5532 was never ported into
updateWithReactFinals, so users hitting the file-based init path on RN New Architecture see two events per JS error: the JS one and the C++ wrapper one captured by the native crash handler.This PR adds the same
ExceptionsManager.reportExceptionvalue check inupdateWithReactFinalsso both init paths behave identically.Android is unaffected:
updateWithReactDefaultsregistersaddIgnoredExceptionForType(JavascriptException.class)and is shared by both Android init paths.💡 Motivation and Context
Fixes #6116. The previous fix for this class of duplicate (#5532, fix for #5529) only covered the JS-initiated init path. With v8's "Capture App Start Errors", the native SDK can be started before JS via
sentry.options.json, which goes through a different filter site that didn't have the C++ wrapper check.💚 How did you test it?
testStartIgnoresJsErrorCppExceptionWrapperinRNSentryStartTests.swiftexercises bothRNSentrySDK.start(configureOptions:)and the dictionary-basedRNSentryStart.start(options:)path (which is whatsentry.options.jsonends up calling). Verifies that:"ExceptionsManager.reportException"in its exception value is dropped, andC++ Exception(e.g.std::runtime_error) without that marker is kept.RNSentryTests.m(testStartBeforeSendFiltersOutJSErrorCppException).yarn lint:clang,yarn lint:swift,yarn lint:lernaclean.📝 Checklist
sendDefaultPIIis enabled🔮 Next steps