Skip to content
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

[draft] fix(web): don't create runZoneGuarded on web if it already exists #2088

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Jun 6, 2024

📜 Description

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

We can still keep the onError callback 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?

📝 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

🔮 Next steps

Copy link
Contributor

github-actions bot commented Jun 6, 2024

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

- don't create runZoneGuarded on web if it already exists ([#2088](https://github.com/getsentry/sentry-dart/pull/2088))

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 34ea139

Copy link

codecov bot commented Jun 6, 2024

Codecov Report

Attention: Patch coverage is 57.14286% with 3 lines in your changes missing coverage. Please review.

Project coverage is 95.09%. Comparing base (e6b16cd) to head (34ea139).

Files Patch % Lines
flutter/lib/src/sentry_flutter.dart 57.14% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2088      +/-   ##
==========================================
- Coverage   95.25%   95.09%   -0.17%     
==========================================
  Files          54       54              
  Lines        1791     1793       +2     
==========================================
- Hits         1706     1705       -1     
- Misses         85       88       +3     

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

Copy link
Contributor

github-actions bot commented Jun 6, 2024

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 393.15 ms 462.71 ms 69.57 ms
Size 6.35 MiB 7.33 MiB 1005.84 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
bffc2c5 348.00 ms 399.89 ms 51.89 ms
62de927 313.81 ms 358.15 ms 44.34 ms
8cb6557 321.20 ms 370.46 ms 49.26 ms
ca7f531 395.69 ms 497.82 ms 102.12 ms
24b6e60 440.64 ms 557.96 ms 117.32 ms
895becc 326.94 ms 376.02 ms 49.08 ms
be08ed1 361.37 ms 414.84 ms 53.47 ms
613760b 373.42 ms 399.33 ms 25.92 ms
2d74010 400.42 ms 466.50 ms 66.08 ms
68677de 364.41 ms 415.61 ms 51.20 ms

App size

Revision Plain With Sentry Diff
bffc2c5 6.34 MiB 7.28 MiB 966.31 KiB
62de927 6.15 MiB 7.11 MiB 981.78 KiB
8cb6557 6.06 MiB 7.03 MiB 997.01 KiB
ca7f531 6.33 MiB 7.26 MiB 949.75 KiB
24b6e60 6.33 MiB 7.26 MiB 950.14 KiB
895becc 6.06 MiB 7.03 MiB 997.23 KiB
be08ed1 6.33 MiB 7.26 MiB 946.42 KiB
613760b 5.94 MiB 6.92 MiB 1005.98 KiB
2d74010 6.33 MiB 7.26 MiB 943.41 KiB
68677de 6.06 MiB 7.10 MiB 1.04 MiB

Copy link
Contributor

github-actions bot commented Jun 6, 2024

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1245.20 ms 1262.00 ms 16.80 ms
Size 8.33 MiB 9.54 MiB 1.22 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
e82709a 1249.22 ms 1271.35 ms 22.12 ms
0a23f98 1252.98 ms 1276.76 ms 23.78 ms
6325c3b 1266.52 ms 1291.06 ms 24.54 ms
6078ddc 1207.84 ms 1224.10 ms 16.27 ms
24f71aa 1267.47 ms 1272.00 ms 4.53 ms
1c6eb5b 1277.85 ms 1285.71 ms 7.86 ms
8609bd8 1267.16 ms 1291.39 ms 24.22 ms
0a82a1e 1233.56 ms 1244.56 ms 11.00 ms
2d74010 1264.45 ms 1268.42 ms 3.97 ms
72dfc83 1262.50 ms 1289.75 ms 27.25 ms

App size

Revision Plain With Sentry Diff
e82709a 8.33 MiB 9.40 MiB 1.07 MiB
0a23f98 8.10 MiB 9.18 MiB 1.08 MiB
6325c3b 8.16 MiB 9.17 MiB 1.01 MiB
6078ddc 8.33 MiB 9.40 MiB 1.07 MiB
24f71aa 8.10 MiB 9.16 MiB 1.07 MiB
1c6eb5b 8.15 MiB 9.12 MiB 986.27 KiB
8609bd8 8.28 MiB 9.34 MiB 1.06 MiB
0a82a1e 8.29 MiB 9.37 MiB 1.08 MiB
2d74010 8.32 MiB 9.38 MiB 1.05 MiB
72dfc83 8.15 MiB 9.12 MiB 987.30 KiB

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Zone missmatch on flutter web
1 participant