Skip to content

Support custom Sentry.runZoneGuarded zone creation#2088

Merged
denrase merged 34 commits into
mainfrom
fix/runzoneguarded
Dec 9, 2024
Merged

Support custom Sentry.runZoneGuarded zone creation#2088
denrase merged 34 commits into
mainfrom
fix/runzoneguarded

Conversation

@buenaflor

@buenaflor buenaflor commented Jun 6, 2024 •
edited by denrase
Loading

Copy link
Copy Markdown
Contributor

📜 Description

If the sentry init is already set up in a zone, don't create another one internally.

Provide a sentry wrapped Sentry.runZonedGuarded function which captures exceptions, adds breadcrumbs and propagate the user's onError on top of it so we can keep the original behaviour.

💡 Motivation and Context

Fixes #1943

💚 How did you test it?

Unit tests.

📝 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
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

@codecov

codecov Bot commented Jun 6, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 95.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.00%. Comparing base (07351da) to head (d23edf9).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
dart/lib/src/sentry_run_zoned_guarded.dart 92.85% 2 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #2088      +/-   ##
==========================================
+ Coverage   86.85%   87.00%   +0.15%     
==========================================
  Files         260      261       +1     
  Lines        9242     9249       +7     
==========================================
+ Hits         8027     8047      +20     
+ Misses       1215     1202      -13

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions

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

Copy link
Copy Markdown
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 423.29 ms 477.35 ms 54.06 ms
Size 6.49 MiB 7.56 MiB 1.07 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
affcf07 446.19 ms 476.32 ms 30.13 ms
2331d89 352.45 ms 417.34 ms 64.89 ms
7fd33d3 452.90 ms 506.75 ms 53.85 ms
f9d18f3 430.94 ms 497.59 ms 66.65 ms
af2d175 279.08 ms 312.37 ms 33.29 ms
3334ac1 303.98 ms 366.65 ms 62.67 ms
c732386 316.84 ms 390.62 ms 73.78 ms
e82709a 361.18 ms 423.50 ms 62.32 ms
5baa201 389.26 ms 462.83 ms 73.57 ms
03e4c9b 410.34 ms 493.20 ms 82.86 ms

App size

Revision Plain With Sentry Diff
affcf07 6.52 MiB 7.59 MiB 1.06 MiB
2331d89 5.94 MiB 6.96 MiB 1.02 MiB
7fd33d3 6.49 MiB 7.56 MiB 1.08 MiB
f9d18f3 6.15 MiB 7.13 MiB 1000.49 KiB
af2d175 5.94 MiB 6.92 MiB 1001.83 KiB
3334ac1 6.06 MiB 7.03 MiB 993.54 KiB
c732386 6.26 MiB 7.20 MiB 958.78 KiB
e82709a 6.34 MiB 7.29 MiB 970.37 KiB
5baa201 6.35 MiB 7.33 MiB 1005.56 KiB
03e4c9b 6.35 MiB 7.42 MiB 1.07 MiB

Previous results on branch: fix/runzoneguarded

Startup times

Revision Plain With Sentry Diff
2822764 455.45 ms 496.37 ms 40.92 ms
3f0f7d9 529.37 ms 585.55 ms 56.18 ms
20261fb 444.80 ms 506.02 ms 61.22 ms
ff275ca 503.47 ms 564.47 ms 61.00 ms
2ef2edf 626.36 ms 693.30 ms 66.94 ms
321b584 415.48 ms 468.86 ms 53.38 ms
b8b75a7 448.68 ms 490.80 ms 42.12 ms
4b1e6c7 468.25 ms 519.58 ms 51.33 ms
4b77b41 478.64 ms 535.36 ms 56.72 ms
6634f0d 459.17 ms 520.98 ms 61.81 ms

App size

Revision Plain With Sentry Diff
2822764 6.49 MiB 7.56 MiB 1.07 MiB
3f0f7d9 6.49 MiB 7.56 MiB 1.07 MiB
20261fb 6.49 MiB 7.56 MiB 1.07 MiB
ff275ca 6.49 MiB 7.56 MiB 1.07 MiB
2ef2edf 6.49 MiB 7.56 MiB 1.07 MiB
321b584 6.49 MiB 7.56 MiB 1.07 MiB
b8b75a7 6.49 MiB 7.56 MiB 1.08 MiB
4b1e6c7 6.49 MiB 7.56 MiB 1.07 MiB
4b77b41 6.49 MiB 7.56 MiB 1.07 MiB
6634f0d 6.49 MiB 7.56 MiB 1.07 MiB

@github-actions

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

Copy link
Copy Markdown
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1260.76 ms 1284.04 ms 23.29 ms
Size 8.38 MiB 9.78 MiB 1.40 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
90a08ea 1260.45 ms 1285.24 ms 24.80 ms
e239c83 1248.40 ms 1269.28 ms 20.89 ms
c9d3212 1231.84 ms 1237.40 ms 5.57 ms
6aab859 1245.14 ms 1247.59 ms 2.45 ms
e0f3acf 1238.21 ms 1263.54 ms 25.34 ms
f3a18f2 1246.86 ms 1268.00 ms 21.14 ms
04bd9e6 1230.78 ms 1250.71 ms 19.94 ms
5aba417 1265.31 ms 1287.90 ms 22.59 ms
64af39c 1243.98 ms 1257.69 ms 13.71 ms
5603ab2 1268.47 ms 1280.73 ms 12.26 ms

App size

Revision Plain With Sentry Diff
90a08ea 8.38 MiB 9.73 MiB 1.36 MiB
e239c83 8.38 MiB 9.74 MiB 1.36 MiB
c9d3212 8.29 MiB 9.39 MiB 1.10 MiB
6aab859 8.29 MiB 9.36 MiB 1.07 MiB
e0f3acf 8.38 MiB 9.78 MiB 1.40 MiB
f3a18f2 8.38 MiB 9.75 MiB 1.37 MiB
04bd9e6 8.33 MiB 9.61 MiB 1.27 MiB
5aba417 8.16 MiB 9.17 MiB 1.01 MiB
64af39c 8.28 MiB 9.34 MiB 1.06 MiB
5603ab2 8.15 MiB 9.12 MiB 990.57 KiB

Previous results on branch: fix/runzoneguarded

Startup times

Revision Plain With Sentry Diff
cc0cedd 1263.18 ms 1289.23 ms 26.04 ms
b8b75a7 1261.64 ms 1273.83 ms 12.19 ms
20261fb 1251.88 ms 1279.57 ms 27.69 ms
2ef2edf 1245.10 ms 1274.41 ms 29.31 ms
6634f0d 1252.96 ms 1288.66 ms 35.70 ms
4b77b41 1249.47 ms 1278.40 ms 28.93 ms
2822764 1244.98 ms 1274.42 ms 29.44 ms
ff275ca 1255.73 ms 1281.53 ms 25.80 ms
321b584 1242.79 ms 1273.15 ms 30.36 ms
4b1e6c7 1250.65 ms 1268.02 ms 17.38 ms

App size

Revision Plain With Sentry Diff
cc0cedd 8.38 MiB 9.78 MiB 1.40 MiB
b8b75a7 8.38 MiB 9.78 MiB 1.40 MiB
20261fb 8.38 MiB 9.78 MiB 1.40 MiB
2ef2edf 8.38 MiB 9.78 MiB 1.40 MiB
6634f0d 8.38 MiB 9.78 MiB 1.40 MiB
4b77b41 8.38 MiB 9.77 MiB 1.39 MiB
2822764 8.38 MiB 9.78 MiB 1.40 MiB
ff275ca 8.38 MiB 9.78 MiB 1.40 MiB
321b584 8.38 MiB 9.78 MiB 1.40 MiB
4b1e6c7 8.38 MiB 9.78 MiB 1.40 MiB

@buenaflor buenaflor changed the title [draft] fix: don't create runZoneGuarded on web if it already exists [draft] fix(web): don't create runZoneGuarded on web if it already exists Jun 6, 2024
@buenaflor buenaflor mentioned this pull request Jun 7, 2024
Closed
buenaflor
buenaflor commented Nov 20, 2024
View reviewed changes

@buenaflor buenaflor left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

first look looks good, prolly also makes sense to test the case when apis are called before sentry is initialized e.g an error happens that triggers runZonedGuarded before SentryFlutter.init

@denrase denrase changed the title [draft] fix(web): don't create runZoneGuarded on web if it already exists Support custom Sentry.runZoneGuarded zone creation Nov 26, 2024
@denrase
denrase marked this pull request as ready for review November 26, 2024 13:59
buenaflor
buenaflor commented Nov 28, 2024
View reviewed changes
Comment thread flutter/lib/src/sentry_flutter.dart
Comment thread dart/lib/src/sentry.dart Outdated
Comment thread CHANGELOG.md Outdated
@buenaflor

buenaflor commented Nov 28, 2024

Copy link
Copy Markdown
Contributor Author

@denrase also pls prepare docs for this

@denrase denrase mentioned this pull request Dec 2, 2024
Merged
3 tasks
buenaflor
buenaflor commented Dec 2, 2024
View reviewed changes
Comment thread dart/test/sentry_test.dart Outdated
@denrase
denrase enabled auto-merge (squash) December 3, 2024 18:38
@denrase
denrase self-requested a review December 3, 2024 18:38
buenaflor
buenaflor commented Dec 5, 2024
View reviewed changes

@buenaflor buenaflor left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@denrase looks good, only 1 minor change

I can't approve cause this I'm the owner of the PR lol

so you gotta do it I guess

Comment thread dart/test/sentry_run_zoned_guarded_test.dart Outdated
denrase
denrase approved these changes Dec 9, 2024
View reviewed changes

@denrase denrase 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.

Approving "myself" because i'm not the PRs creator.

@denrase
denrase merged commit b6bb5b4 into main Dec 9, 2024
@denrase
denrase deleted the fix/runzoneguarded branch December 9, 2024 09:39
@denrase denrase mentioned this pull request Jan 28, 2025
Merged
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@denrase denrase denrase approved these changes
@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich
@stefanosiano stefanosiano Awaiting requested review from stefanosiano
@martinhaintz martinhaintz Awaiting requested review from martinhaintz

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Zone missmatch on flutter web

2 participants

@buenaflor @denrase