Skip to content

fix(sessions): mechanism.handled:false should crash current session#3900

Merged
krystofwoldrich merged 4 commits into
mainfrom
kw/fix-sessions
Jul 3, 2024
Merged

fix(sessions): mechanism.handled:false should crash current session#3900
krystofwoldrich merged 4 commits into
mainfrom
kw/fix-sessions

Conversation

@krystofwoldrich

@krystofwoldrich krystofwoldrich commented Jun 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

📢 Type of change

  • Bugfix

📜 Description

This PR updates the session handling, so native SDK can correctly handle crashing, ending and creating a new session.

Session will be crashed and no new session is started if { handled: false, type: 'onerror' }. Type onerror means the error is from the RN Global Handle => the native application is going to crash.

Session will be crashed and new session is started if { handled: false, type: '!onerror' }. Other types genetic... and others mean the exception is not from RN Global Handled (can be user set) and the native application is not going to crash.

Session will be crashed as no new session is started for native crashed.

🛑 Blocked by

💡 Motivation and Context

💚 How did you test it?

sample app

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled
  • All tests passing
  • No breaking changes

@krystofwoldrich krystofwoldrich mentioned this pull request Jun 18, 2024
Merged
7 tasks
@github-actions

github-actions Bot commented Jun 18, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

iOS (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1216.63 ms 1219.75 ms 3.12 ms
Size 2.36 MiB 3.04 MiB 698.27 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
148f924+dirty 1214.76 ms 1215.73 ms 0.97 ms
31fcca2+dirty 1209.17 ms 1216.21 ms 7.04 ms
3ffcddd+dirty 1244.47 ms 1264.14 ms 19.67 ms
27ef4ee+dirty 1293.52 ms 1296.08 ms 2.56 ms
e5c9b8b+dirty 1258.57 ms 1267.32 ms 8.75 ms
e2b64fe+dirty 1232.22 ms 1255.20 ms 22.98 ms
1d86dd6+dirty 1249.71 ms 1279.16 ms 29.45 ms
c398f67+dirty 1219.67 ms 1225.66 ms 5.99 ms
575f9da+dirty 1266.22 ms 1274.84 ms 8.62 ms
2534337+dirty 1225.08 ms 1230.26 ms 5.17 ms

App size

Revision Plain With Sentry Diff
148f924+dirty 2.36 MiB 3.04 MiB 696.25 KiB
31fcca2+dirty 2.36 MiB 2.90 MiB 552.95 KiB
3ffcddd+dirty 2.36 MiB 2.84 MiB 489.60 KiB
27ef4ee+dirty 2.36 MiB 2.85 MiB 500.03 KiB
e5c9b8b+dirty 2.36 MiB 2.87 MiB 520.43 KiB
e2b64fe+dirty 2.36 MiB 2.85 MiB 495.80 KiB
1d86dd6+dirty 2.36 MiB 2.89 MiB 535.43 KiB
c398f67+dirty 2.36 MiB 3.04 MiB 696.27 KiB
575f9da+dirty 2.36 MiB 2.87 MiB 520.20 KiB
2534337+dirty 2.36 MiB 2.88 MiB 525.47 KiB

@lucas-zimerman

lucas-zimerman commented Jun 26, 2024

Copy link
Copy Markdown
Collaborator

It is missing the changelog, but other than that it looks good to me.
Not approving it for the moment because there are still blocked issues required for this PR that are still open.

@github-actions

github-actions Bot commented Jun 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

iOS (new) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1225.71 ms 1220.87 ms -4.84 ms
Size 2.92 MiB 3.61 MiB 705.15 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
148f924+dirty 1220.72 ms 1221.30 ms 0.58 ms
31fcca2+dirty 1222.04 ms 1226.51 ms 4.47 ms
3ffcddd+dirty 1272.22 ms 1273.98 ms 1.76 ms
27ef4ee+dirty 1236.41 ms 1244.90 ms 8.49 ms
e5c9b8b+dirty 1276.90 ms 1280.92 ms 4.02 ms
e2b64fe+dirty 1285.78 ms 1297.56 ms 11.78 ms
1d86dd6+dirty 1289.25 ms 1293.36 ms 4.11 ms
c398f67+dirty 1227.31 ms 1230.00 ms 2.69 ms
575f9da+dirty 1272.00 ms 1284.38 ms 12.38 ms
2534337+dirty 1220.87 ms 1221.47 ms 0.60 ms

App size

Revision Plain With Sentry Diff
148f924+dirty 2.92 MiB 3.60 MiB 701.88 KiB
31fcca2+dirty 2.92 MiB 3.46 MiB 557.31 KiB
3ffcddd+dirty 2.92 MiB 3.40 MiB 494.39 KiB
27ef4ee+dirty 2.92 MiB 3.41 MiB 503.72 KiB
e5c9b8b+dirty 2.92 MiB 3.43 MiB 524.50 KiB
e2b64fe+dirty 2.92 MiB 3.41 MiB 499.97 KiB
1d86dd6+dirty 2.92 MiB 3.44 MiB 538.27 KiB
c398f67+dirty 2.92 MiB 3.60 MiB 701.89 KiB
575f9da+dirty 2.92 MiB 3.43 MiB 524.26 KiB
2534337+dirty 2.92 MiB 3.43 MiB 529.76 KiB

@krystofwoldrich krystofwoldrich mentioned this pull request Jul 2, 2024
Merged
@github-actions

github-actions Bot commented Jul 3, 2024

Copy link
Copy Markdown
Contributor

Android (legacy) Performance metrics 🚀

  Plain With Sentry Diff
Startup time 478.88 ms 532.66 ms 53.78 ms
Size 17.73 MiB 19.95 MiB 2.21 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
70e6261 482.65 ms 495.70 ms 13.05 ms
86d6d2c+dirty 332.90 ms 352.45 ms 19.55 ms
5571a20 410.55 ms 441.06 ms 30.51 ms
1d86dd6 405.14 ms 411.06 ms 5.92 ms
d0bf494+dirty 375.37 ms 395.14 ms 19.77 ms
148f924 492.65 ms 500.28 ms 7.63 ms
27ef4ee 317.40 ms 321.70 ms 4.30 ms
5bb8d5f 431.21 ms 459.40 ms 28.19 ms
2534337 394.15 ms 415.12 ms 20.97 ms
80b2ce3 385.02 ms 387.36 ms 2.34 ms

App size

Revision Plain With Sentry Diff
70e6261 17.73 MiB 19.94 MiB 2.21 MiB
86d6d2c+dirty 17.73 MiB 20.04 MiB 2.31 MiB
5571a20 17.73 MiB 19.93 MiB 2.19 MiB
1d86dd6 17.73 MiB 19.86 MiB 2.12 MiB
d0bf494+dirty 17.73 MiB 19.75 MiB 2.02 MiB
148f924 17.73 MiB 19.94 MiB 2.21 MiB
27ef4ee 17.73 MiB 19.82 MiB 2.08 MiB
5bb8d5f 17.73 MiB 19.93 MiB 2.20 MiB
2534337 17.73 MiB 19.84 MiB 2.11 MiB
80b2ce3 17.73 MiB 19.75 MiB 2.02 MiB

lucas-zimerman
lucas-zimerman approved these changes Jul 3, 2024
View reviewed changes

@lucas-zimerman lucas-zimerman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGMT!

@krystofwoldrich
krystofwoldrich merged commit 5290b95 into main Jul 3, 2024
@krystofwoldrich
krystofwoldrich deleted the kw/fix-sessions branch July 3, 2024 15:19
@github-actions

github-actions Bot commented Jul 9, 2024

Copy link
Copy Markdown
Contributor
Fails
🚫 Please consider adding a changelog entry for the next release.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- `mechanism.handled:false` should crash current session ([#3900](https://github.com/getsentry/sentry-react-native/pull/3900))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against 7fe38fd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucas-zimerman lucas-zimerman lucas-zimerman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

iOS doesn't update session when handled:false set in JS

2 participants

@krystofwoldrich @lucas-zimerman