refactor(ios): remove dead SentrySDKWrapper init surface#6059
Merged
Conversation
`SentrySDKWrapper.createOptionsWithDictionary:isSessionReplayEnabled:error:` (and `setupWithDictionary:`, `startWithOptions:`, `enableAutoSessionTracking`, `enableWatchdogTerminationTracking`) became dead when v8 moved init to `RNSentryStart` (#5582 / #4442). The dead methods had a parallel test suite that kept passing, which is exactly how the bugs in #6012 and #6014 shipped: new code was added to `SentrySDKWrapper` because the tests there made it look like the live surface. Delete the dead methods, migrate the still-relevant test coverage to `RNSentryStart`-based tests, drop duplicate tests already pinned on `RNSentryStart`. `SentrySDKWrapper.h` is not in the podspec's `public_header_files`, so no downstream consumers are affected. The kept methods (`configureScope:`, `crash`, `close`, `crashedLastRun`, `debug`, `releaseName`) continue to be used from `RNSentry.mm`. Closes #6015. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7 tasks
📲 Install BuildsAndroid
|
Contributor
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 1213.37 ms | 1218.15 ms | 4.78 ms |
| 4b87b12+dirty | 1212.90 ms | 1222.09 ms | 9.19 ms |
| 890d145+dirty | 1223.59 ms | 1231.37 ms | 7.78 ms |
| 0d9949d+dirty | 1211.38 ms | 1219.67 ms | 8.29 ms |
| 04207c4+dirty | 1191.27 ms | 1189.78 ms | -1.48 ms |
| 3ce5254+dirty | 1219.93 ms | 1221.90 ms | 1.96 ms |
| 4953e94+dirty | 1212.06 ms | 1214.83 ms | 2.77 ms |
| 2c735cc+dirty | 1229.67 ms | 1221.50 ms | -8.17 ms |
| a50b33d+dirty | 1197.74 ms | 1197.17 ms | -0.57 ms |
| df5d108+dirty | 1225.90 ms | 1220.14 ms | -5.76 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 4b87b12+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| 890d145+dirty | 3.38 MiB | 4.77 MiB | 1.38 MiB |
| 0d9949d+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 04207c4+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 3ce5254+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 4953e94+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 2c735cc+dirty | 3.38 MiB | 4.74 MiB | 1.35 MiB |
| a50b33d+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| df5d108+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
Contributor
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 1202.35 ms | 1198.31 ms | -4.04 ms |
| 4b87b12+dirty | 1199.49 ms | 1199.78 ms | 0.29 ms |
| 890d145+dirty | 1212.98 ms | 1220.10 ms | 7.12 ms |
| 0d9949d+dirty | 1203.94 ms | 1202.27 ms | -1.67 ms |
| 04207c4+dirty | 1228.55 ms | 1226.04 ms | -2.51 ms |
| 3ce5254+dirty | 1217.70 ms | 1224.69 ms | 6.99 ms |
| 4953e94+dirty | 1217.41 ms | 1223.53 ms | 6.12 ms |
| 2c735cc+dirty | 1223.33 ms | 1224.38 ms | 1.04 ms |
| a50b33d+dirty | 1207.11 ms | 1212.10 ms | 5.00 ms |
| df5d108+dirty | 1207.34 ms | 1210.50 ms | 3.16 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 7ac3378+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 4b87b12+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| 890d145+dirty | 3.38 MiB | 4.77 MiB | 1.38 MiB |
| 0d9949d+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 04207c4+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 3ce5254+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 4953e94+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 2c735cc+dirty | 3.38 MiB | 4.74 MiB | 1.35 MiB |
| a50b33d+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| df5d108+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
The migration in the previous commit added a `testStart…KeepsBreadcrumbWhenDevServerUrlIsNotPassed…` method that already existed in the `RNSentryStart Tests` block. Compile failed in CI with "Duplicate declaration of method". The earlier `@interface RNSentryStart Tests` already covers this scenario; the migrated copy is redundant. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
…Message Warden flagged the migrated tests using `[SentryException alloc]` and `[SentryMessage alloc]` without `init`. Both classes have `SENTRY_NO_INIT` and require designated initializers (`initWithValue:type:` and `initWithFormatted:`), so the alloc-only pattern is undefined behavior — property assignments happened to work via zero-initialized memory but the test could crash or behave inconsistently and mask real regressions. The pattern was inherited from the original SentrySDKWrapper-based tests this PR migrated; same fix applied across all 12 occurrences. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ysdkwrapper-deadcode # Conflicts: # packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.m # packages/core/ios/SentrySDKWrapper.h # packages/core/ios/SentrySDKWrapper.m
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
2 participants
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
SentrySDKWrapper.createOptionsWithDictionary:isSessionReplayEnabled:error:and friends became unreachable from production when v8 moved init toRNSentryStartin #5582 (sub-PR #4442), but were left in place along with their parallel XCTest suite. Those dead-code tests are the reason two bugs shipped in every 8.x release until we noticed them — #5611's author added profiling parsing toSentrySDKWrapperbecause the tests there made it look like the live surface, and the tests kept passing while the liveRNSentryStartpath silently dropped the option (#6012, #6014).This PR removes the dead surface so the same mistake can't recur:
packages/core/ios/SentrySDKWrapper.{h,m}— delete:+ createOptionsWithDictionary:isSessionReplayEnabled:error:+ setupWithDictionary:isSessionReplayEnabled:error:+ startWithOptions:(only callsite was the deletedsetupWithDictionary:)+ enableAutoSessionTracking(zero callers inpackages/core/)+ enableWatchdogTerminationTracking(zero callers inpackages/core/)getURLFromDSN:helper and unused importsKept (still called 18 times from
RNSentry.mm):configureScope:,crash,close,crashedLastRun,debug,releaseName.packages/core/RNSentryCocoaTester/RNSentryCocoaTesterTests/RNSentryTests.m— 22 SentrySDKWrapper-based tests deleted (each had an existingRNSentryStart-based equivalent: spotlight, native crash handling, auto-perf tracing, breadcrumb filtering, CPP exceptions v2,testRemovesPerformanceProperties, etc.). 15 still-relevant tests migrated toRNSentryStart-based equivalents under the existing#pragma mark - RNSentryStart Testsblock:enableLogsenabled / disabled (2)beforeBreadcrumbkeeps-when-no-devServer edge case (1)ignoreErrorsfamily (5)beforeSendJS-exception / CPP-exception filtering (2) — these now also call[RNSentryStart updateWithReactFinals:]so the full two-layerbeforeSendchain is exercised, which is stronger coverage than the old tests (the wrapped outer filter wasn't in the old SentrySDKWrapper path).enableSessionReplayInUnreliableEnvironmentDefault / WithErrorSampleRate / WithSessionSampleRate / WithSessionSampleRates / Disabled (5) — drops the explicitisSessionReplayEnabled:parameter;RNSentryStart.createOptionsWithDictionary:computes it internally via[RNSentryReplay updateOptions:]on its mutable copy, driven by the samereplays*SampleRatekeys the fixtures already pass.#import "SentrySDKWrapper.h"removed from the test file. ClassRNSentryInitNativeSdkTestsretained (could be renamed in a follow-up since it's no longer init-only, but that's bikeshed). Tests forsetEventOriginTag,fetchNativeStackFramesBy, and user creation that don't touchSentrySDKWrapperare unmodified.Net diff: −582 lines.
💡 Motivation and Context
Closes #6015. Companion to #6012 and #6014, which fixed two real bugs caused by code being added to this dead surface. The cleanup removes the conditions that allowed those bugs to ship undetected — the tests on the dead path will no longer make the wrong file look "tested."
SentrySDKWrapper.his not ins.public_header_filesinpackages/core/RNSentry.podspec(onlyRNSentry.h,RNSentrySDK.h,RNSentryStart.h,RNSentryVersion.h,RNSentryBreadcrumb.h,RNSentryReplay.h,RNSentryReplayBreadcrumbConverter.h,Replay/RNSentryReplayMask.h,Replay/RNSentryReplayUnmask.h,RNSentryTimeToDisplay.hare). The deleted methods are not exposed to downstream Swift/Obj-C consumers, so no API break.💚 How did you test it?
AppDelegate.swift) for any reference to the deleted methods or toSentrySDKWrapper.setupWithDictionary— zero hits.SentryOptions.enableAutoSessionTracking/enableWatchdogTerminationTrackingdocumentation, which is unrelated to the deleted accessor wrappers.testCreateOptionsWithDictionaryRemovesPerformanceProperties↔testStartWithDictionaryRemovesPerformanceProperties): semantically equivalent, both pin the user-visible behavior.RNSentryReplay.updateOptions:via the same dict keys;beforeSendfilter tests now exercise the fullprepareOptions+updateWithReactFinalschain.yarn build,yarn test(211 passing),yarn circularDepCheck,./scripts/clang-format.sh lint,yarn lint(full suite — oxlint, prettier, lerna, android, kotlin, clang, swift) all clean. Cocoa XCTest run is delegated to CI (local has a pre-existing fmt-pod / Xcode 22 compatibility issue unrelated to this change).📝 Checklist
sendDefaultPIIis enabled🔮 Next steps