-
-
Notifications
You must be signed in to change notification settings - Fork 357
feat: v8: Capture app start errors before JS #5582
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v8
Are you sure you want to change the base?
Conversation
…ructures (#4445) * Extract Android SDK Init * Update tests * Adds changelog * Fix lint issues * Rename RNSentryStart instance for clarity * Converts RNSentryStart to utility class * Update CHANGELOG.md --------- Co-authored-by: Krystof Woldrich <[email protected]>
* Convert json object to writable map * Make class/methods package-private(default)
- Resolved merge conflicts across iOS, Android, and JavaScript/TypeScript code - Updated iOS implementation to use RNSentryStart API - Fixed Android compilation errors for Sentry Android SDK v7 compatibility - Removed deprecated API calls (setEnableTracing, getPackages, getIntegrations) - Updated Gradle build scripts and resolved conflict markers - All builds verified: iOS, Android, and Expo sample apps compile successfully
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. This PR will not appear in the changelog. 🤖 This preview updates automatically when you update the PR. |
|
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4fa5b4+dirty | 382.09 ms | 398.28 ms | 16.19 ms |
| 12fba4a+dirty | 456.57 ms | 462.90 ms | 6.32 ms |
| fa0d109+dirty | 413.71 ms | 434.22 ms | 20.51 ms |
| d6aa223+dirty | 436.98 ms | 466.42 ms | 29.44 ms |
| 206e87e+dirty | 416.94 ms | 440.98 ms | 24.04 ms |
| bc8a1ed+dirty | 396.10 ms | 426.80 ms | 30.69 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4fa5b4+dirty | 43.94 MiB | 48.91 MiB | 4.97 MiB |
| 12fba4a+dirty | 43.94 MiB | 49.22 MiB | 5.29 MiB |
| fa0d109+dirty | 43.94 MiB | 49.22 MiB | 5.29 MiB |
| d6aa223+dirty | 43.94 MiB | 49.38 MiB | 5.44 MiB |
| 206e87e+dirty | 43.94 MiB | 49.22 MiB | 5.29 MiB |
| bc8a1ed+dirty | 43.94 MiB | 48.91 MiB | 4.97 MiB |
Previous results on branch: antonis/capture-app-start-errors-v8
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4df5a0+dirty | 364.57 ms | 389.29 ms | 24.71 ms |
| 88812b1+dirty | 425.90 ms | 435.25 ms | 9.35 ms |
| 82de722+dirty | 398.52 ms | 422.70 ms | 24.18 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4df5a0+dirty | 43.94 MiB | 49.22 MiB | 5.29 MiB |
| 88812b1+dirty | 43.94 MiB | 49.22 MiB | 5.29 MiB |
| 82de722+dirty | 43.94 MiB | 49.22 MiB | 5.29 MiB |
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4fa5b4+dirty | 449.55 ms | 481.50 ms | 31.95 ms |
| 12fba4a+dirty | 483.60 ms | 514.49 ms | 30.89 ms |
| fa0d109+dirty | 429.60 ms | 452.50 ms | 22.90 ms |
| d6aa223+dirty | 543.40 ms | 564.24 ms | 20.84 ms |
| 206e87e+dirty | 464.80 ms | 504.68 ms | 39.88 ms |
| bc8a1ed+dirty | 442.18 ms | 476.27 ms | 34.08 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4fa5b4+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| 12fba4a+dirty | 43.75 MiB | 48.40 MiB | 4.64 MiB |
| fa0d109+dirty | 43.75 MiB | 48.40 MiB | 4.64 MiB |
| d6aa223+dirty | 43.75 MiB | 48.55 MiB | 4.80 MiB |
| 206e87e+dirty | 43.75 MiB | 48.40 MiB | 4.64 MiB |
| bc8a1ed+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
Previous results on branch: antonis/capture-app-start-errors-v8
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4df5a0+dirty | 405.80 ms | 412.56 ms | 6.77 ms |
| 88812b1+dirty | 428.04 ms | 462.22 ms | 34.18 ms |
| 82de722+dirty | 425.13 ms | 457.02 ms | 31.89 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| b4df5a0+dirty | 43.75 MiB | 48.40 MiB | 4.65 MiB |
| 88812b1+dirty | 43.75 MiB | 48.40 MiB | 4.64 MiB |
| 82de722+dirty | 43.75 MiB | 48.40 MiB | 4.64 MiB |
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 206e87e+dirty | 1197.12 ms | 1204.25 ms | 7.13 ms |
| b4fa5b4+dirty | 1213.59 ms | 1211.26 ms | -2.33 ms |
| d6aa223+dirty | 1216.76 ms | 1213.40 ms | -3.37 ms |
| 12fba4a+dirty | 1209.43 ms | 1217.08 ms | 7.65 ms |
| bc8a1ed+dirty | 1198.66 ms | 1200.60 ms | 1.94 ms |
| fa0d109+dirty | 1206.81 ms | 1205.38 ms | -1.43 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 206e87e+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| b4fa5b4+dirty | 3.44 MiB | 4.66 MiB | 1.22 MiB |
| d6aa223+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| 12fba4a+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| bc8a1ed+dirty | 3.44 MiB | 4.66 MiB | 1.22 MiB |
| fa0d109+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
Previous results on branch: antonis/capture-app-start-errors-v8
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 88812b1+dirty | 1210.98 ms | 1215.22 ms | 4.24 ms |
| 82de722+dirty | 1210.87 ms | 1217.00 ms | 6.13 ms |
| b4df5a0+dirty | 1217.06 ms | 1221.61 ms | 4.55 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 88812b1+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| 82de722+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| b4df5a0+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 206e87e+dirty | 1184.11 ms | 1183.19 ms | -0.92 ms |
| b4fa5b4+dirty | 1203.83 ms | 1207.13 ms | 3.30 ms |
| d6aa223+dirty | 1192.33 ms | 1208.17 ms | 15.84 ms |
| 12fba4a+dirty | 1214.20 ms | 1223.30 ms | 9.09 ms |
| bc8a1ed+dirty | 1194.70 ms | 1201.18 ms | 6.48 ms |
| fa0d109+dirty | 1216.02 ms | 1220.67 ms | 4.65 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 206e87e+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| b4fa5b4+dirty | 3.44 MiB | 4.66 MiB | 1.22 MiB |
| d6aa223+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| 12fba4a+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| bc8a1ed+dirty | 3.44 MiB | 4.66 MiB | 1.22 MiB |
| fa0d109+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
Previous results on branch: antonis/capture-app-start-errors-v8
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 88812b1+dirty | 1216.63 ms | 1215.55 ms | -1.08 ms |
| 82de722+dirty | 1213.09 ms | 1208.90 ms | -4.19 ms |
| b4df5a0+dirty | 1213.04 ms | 1218.59 ms | 5.55 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 88812b1+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| 82de722+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
| b4df5a0+dirty | 3.38 MiB | 4.67 MiB | 1.29 MiB |
|
@sentry review |
|
No noticeable impact on iOS, the app size increase by adding sentry on java is interesting though, but no extra impact from this PR. |
That's a good catch @lucas-zimerman 🤔 I wonder if this has to do with the Gradle Plugin bump #5578 on the v8 base branch #5501 |
| [ | ||
| "@sentry/react-native/expo", | ||
| { | ||
| "useNativeInit": true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Q: Do all sentry options work here? we should document this feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only added option in the plugin. I've started a documentation PR in getsentry/sentry-docs#16170
Looking at it again I think the |
lucas-zimerman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a complete PR review and also loads of tokens, I can really say, LGTM!
Co-authored-by: LucasZF <[email protected]>
# Conflicts: # CHANGELOG.md
|
@sentry review |
* fix(android): Fix ConcurrentModificationException when disabling native crash handling When enableNativeCrashHandling is set to false, the code was iterating over the integrations list with a for-each loop while calling remove() directly, which causes a ConcurrentModificationException at runtime. Fixed by using Java 8's removeIf() method which safely handles iteration and removal in a single operation. This is more concise and follows modern Java best practices. Added unit tests to verify the fix and ensure integrations are properly removed without throwing exceptions. Co-Authored-By: Claude Sonnet 4.5 <[email protected]> * Lint fix --------- Co-authored-by: Claude Sonnet 4.5 <[email protected]>
packages/core/android/src/main/java/io/sentry/react/RNSentryStart.java
Outdated
Show resolved
Hide resolved
…carUrl The code attempted to read defaultSidecarUrl without checking if the key exists in the options map. This caused a NoSuchKeyException crash during startup when spotlight was set to true in sentry.options.json without providing defaultSidecarUrl. Added key existence check to match iOS implementation behavior and prevent the crash while maintaining backward compatibility. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
📢 Type of change
📜 Description
Follow up to #4472 (v6) > #5470 (v7) targeting v8 #5501
Capture App Start errors and crashes by initializing Sentry from
sentry.options.json(#4472)Create
sentry.options.jsonin the React Native project root and set options the same as you currently have inSentry.initin JS.{ "dsn": "https://[email protected]/value", }Initialize Sentry on the native layers by newly provided native methods.
Add RNSentrySDK APIs support to @sentry/react-native/expo plugin (#4633)
useNativeInitoption to automatically initialize Sentry natively before JavaScript loads, enabling capture of app start errors{ "expo": { "plugins": [ [ "@sentry/react-native/expo", { "useNativeInit": true } ] ] } }Changes
optionsFileinto the JS bundle during Metro bundle process (#4476)startWithConfigureOptionsfor Apple platforms (#4444)initwith optionalOptionsConfiguration<SentryAndroidOptions>for Android (#4451)sentry.options.jsonfor Apple platforms (#4447)sentry.options.jsonfor Android (#4451)Sentry.initoptions in JS (#4510)Internal
Note
The feature the feature should be 100% backwards compatible and opt-in. Users who don't create sentry.options.json or call RNSentrySDK.start()/init() will experience zero changes to their existing Sentry implementation. The traditional JS-only initialization path remains fully functional and is the default behavior.
📝 Documentation PR: getsentry/sentry-docs#16170
💡 Motivation and Context
Fixes #3608
Previously, React Native apps couldn't capture errors that occurred during app startup before JavaScript initialized, because Sentry was only initialized from JavaScript code (Sentry.init()). This meant crashes during:
were never captured.
💚 How did you test it?
CI, Manually
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
#skip-changelog