Fix(V3): Duplicated session / Fix incompatibility with Cocoa V9#1088
Fix(V3): Duplicated session / Fix incompatibility with Cocoa V9#1088lucas-zimerman merged 12 commits intomainfrom
Conversation
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. Bug Fixes 🐛
Build / dependencies / internal 🔧
🤖 This preview updates automatically when you update the PR. |
lucas-zimerman
changed the title
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if let sessionTrackingIntervalMillis = dict["sessionTrackingIntervalMillis"] as? Int { | ||
| options.sessionTrackingIntervalMillis = UInt(sessionTrackingIntervalMillis) | ||
| } | ||
|
|
||
| if let maxBreadcrumbs = dict["maxBreadcrumbs"] as? Int { | ||
| options.maxBreadcrumbs = UInt(maxBreadcrumbs) | ||
| } |
There was a problem hiding this comment.
Bug: A negative value for sessionTrackingIntervalMillis or maxBreadcrumbs from JavaScript will cause a fatal crash on iOS due to an unsafe Int to UInt cast.
Severity: HIGH
Suggested Fix
Before casting sessionTrackingIntervalMillis and maxBreadcrumbs to UInt, validate that the Int values are non-negative. If a negative value is received, it should be ignored or handled gracefully instead of attempting the cast. For example: if let value = dict["maxBreadcrumbs"] as? Int, value >= 0 { options.maxBreadcrumbs = UInt(value) }.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: ios/Sources/SentryCapacitorPlugin/SentryCapacitorPlugin.swift#L132-L138
Potential issue: The `createOptions` method manually parses
`sessionTrackingIntervalMillis` and `maxBreadcrumbs` from a dictionary. It casts these
values from `Int` to `UInt` using a direct initializer `UInt()`. In Swift, attempting to
initialize a `UInt` with a negative `Int` value triggers a fatal runtime error, which
will crash the application. The JavaScript layer, which provides these options, does not
perform any validation to prevent negative numbers from being passed. This means a
negative value for either of these options will lead to an unavoidable crash on the iOS
platform.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
I'd rather limit this on the JavaScript side.
The PR is not changing the behavior of sessionTrackingIntervalMillis and maxBreadcrumbs so lets leave it as is and fix it on a separated PR.
2 participants
Dublicated Session
Due to not replacing the
defaultIntegration, we have a side effect that the sibling SDK will add their own integrations, inserting theBrowserSessionautomatically. This is wrong since both native and the JS implementation has auto session implementation.The correct fix was applied by adding the capacitor integrations into
defaultIntegrations, overwriting the sibling SDK integrations and thus, only having a single session being created.A minor refactor was also implemented where I renamed
finalOptiontosharedOptionsince it matches what it is actually being used, as a shared data instead of the final option being used.Before:
After:
Cocoa Fix
It just appeared now that
SentryOptionsInternalis not available on SPM, so I just opted for just insert the logic locally .