Skip to content

fix(sessions): Missing mechanism.handled is not considered crash#3353

Merged
brustolin merged 11 commits into
mainfrom
fix/mechanism-handled-missing-not-crash
Oct 19, 2023
Merged

fix(sessions): Missing mechanism.handled is not considered crash#3353
brustolin merged 11 commits into
mainfrom
fix/mechanism-handled-missing-not-crash

Conversation

@krystofwoldrich

@krystofwoldrich krystofwoldrich commented Oct 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

📜 Description

Some events have mechanism.handled missing in events.exceptions.values and those events should not crash the session.

This happens for example in Flutter.

Java SDK does this already.

https://github.com/getsentry/sentry-java/blob/7ca9895a31aae5edd0efa8fd98a606532f95d731/sentry/src/main/java/io/sentry/SentryEvent.java#L220-L223

💚 How did you test it?

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

@krystofwoldrich krystofwoldrich changed the title fix(sessions): Missing is not considered crash fix(sessions): Missing mechanism.handled is not considered crash Oct 19, 2023
@github-actions

github-actions Bot commented Oct 19, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1231.06 ms 1240.90 ms 9.83 ms
Size 22.85 KiB 410.98 KiB 388.13 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
ddc9b9a 1201.71 ms 1226.70 ms 24.99 ms
7bb0873 1193.70 ms 1222.74 ms 29.04 ms
a9103fe 1221.49 ms 1243.33 ms 21.84 ms
6943de0 1230.02 ms 1235.32 ms 5.30 ms
4a947cf 1230.73 ms 1239.38 ms 8.65 ms
965db8a 1211.61 ms 1226.60 ms 14.99 ms
fc163f5 1224.17 ms 1248.24 ms 24.08 ms
1bbcb9c 1192.51 ms 1231.96 ms 39.45 ms
bef2003 1248.18 ms 1258.86 ms 10.68 ms
67460f4 1244.56 ms 1255.96 ms 11.40 ms

App size

Revision Plain With Sentry Diff
ddc9b9a 20.76 KiB 420.40 KiB 399.65 KiB
7bb0873 22.85 KiB 407.09 KiB 384.24 KiB
a9103fe 20.76 KiB 426.95 KiB 406.19 KiB
6943de0 20.76 KiB 393.33 KiB 372.57 KiB
4a947cf 20.76 KiB 399.69 KiB 378.92 KiB
965db8a 22.84 KiB 403.24 KiB 380.39 KiB
fc163f5 20.76 KiB 436.30 KiB 415.54 KiB
1bbcb9c 20.76 KiB 426.10 KiB 405.34 KiB
bef2003 22.85 KiB 407.73 KiB 384.88 KiB
67460f4 20.76 KiB 426.15 KiB 405.39 KiB

Previous results on branch: fix/mechanism-handled-missing-not-crash

Startup times

Revision Plain With Sentry Diff
b042784 1229.33 ms 1243.17 ms 13.84 ms
7a760cb 1250.71 ms 1262.44 ms 11.73 ms
74d9500 1255.02 ms 1257.88 ms 2.86 ms
dd0b395 1229.31 ms 1246.02 ms 16.71 ms

App size

Revision Plain With Sentry Diff
b042784 22.85 KiB 410.98 KiB 388.13 KiB
7a760cb 22.85 KiB 410.98 KiB 388.13 KiB
74d9500 22.85 KiB 410.98 KiB 388.13 KiB
dd0b395 22.85 KiB 410.98 KiB 388.13 KiB

@codecov

codecov Bot commented Oct 19, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #3353 (e725fa5) into main (209e288) will increase coverage by 0.071%.
The diff coverage is 100.000%.

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3353       +/-   ##
=============================================
+ Coverage   89.182%   89.254%   +0.071%     
=============================================
  Files          500       500               
  Lines        54488     54573       +85     
  Branches     19561     19591       +30     
=============================================
+ Hits         48594     48709      +115     
+ Misses        5028      4890      -138     
- Partials       866       974      +108
Files Coverage Δ
Sources/Sentry/SentryHub.m 98.516% <100.000%> (ø)
Tests/SentryTests/SentryHubTests.swift 98.364% <100.000%> (+0.078%) ⬆️

... and 39 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 209e288...e725fa5. Read the comment docs.

@buenaflor buenaflor mentioned this pull request Oct 19, 2023
Closed
@brustolin
brustolin merged commit 16450b8 into main Oct 19, 2023
@brustolin
brustolin deleted the fix/mechanism-handled-missing-not-crash branch October 19, 2023 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@philipphofmann philipphofmann Awaiting requested review from philipphofmann philipphofmann is a code owner
@armcknight armcknight Awaiting requested review from armcknight
1 more reviewer
@brustolin brustolin brustolin approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@krystofwoldrich @brustolin @getsentry-bot