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

Fix print recursion detection #2624

Merged
merged 6 commits into from
Feb 4, 2025
Merged

Fix print recursion detection #2624

merged 6 commits into from
Feb 4, 2025

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jan 28, 2025

📜 Description

Bug introduced in #2088 refactoring. There the addBreadcrumb call was not awaited, and we probably introduced this as the compiler issues a warning.

Bildschirmfoto 2025-01-28 um 15 13 00

💡 Motivation and Context

Closes #2581
Relates to #2088

💚 How did you test it?

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

🔮 Next steps

@denrase denrase marked this pull request as ready for review January 28, 2025 14:19
Copy link

codecov bot commented Jan 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.10%. Comparing base (0d852d0) to head (2781b11).
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2624   +/-   ##
=======================================
  Coverage   89.10%   89.10%           
=======================================
  Files         263      263           
  Lines        8943     8943           
=======================================
  Hits         7969     7969           
  Misses        974      974           

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

Copy link
Contributor

github-actions bot commented Jan 28, 2025

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 416.90 ms 505.26 ms 88.36 ms
Size 6.46 MiB 7.48 MiB 1.02 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
64e4616 349.82 ms 436.96 ms 87.14 ms
33d0587 308.79 ms 370.86 ms 62.07 ms
3de8b9b 348.55 ms 445.84 ms 97.29 ms
f3a18f2 456.29 ms 504.41 ms 48.12 ms
aa950e9 337.42 ms 400.72 ms 63.30 ms
2e93bab 515.33 ms 558.79 ms 43.46 ms
61e71ec 343.94 ms 410.59 ms 66.66 ms
3e5ee37 317.56 ms 366.84 ms 49.28 ms
5e8d2b3 342.17 ms 375.83 ms 33.66 ms
94757e3 480.21 ms 515.28 ms 35.07 ms

App size

Revision Plain With Sentry Diff
64e4616 6.27 MiB 7.20 MiB 956.52 KiB
33d0587 6.16 MiB 7.14 MiB 1007.47 KiB
3de8b9b 6.27 MiB 7.20 MiB 957.75 KiB
f3a18f2 6.49 MiB 7.57 MiB 1.08 MiB
aa950e9 5.94 MiB 6.96 MiB 1.02 MiB
2e93bab 6.49 MiB 7.55 MiB 1.07 MiB
61e71ec 6.34 MiB 7.28 MiB 966.26 KiB
3e5ee37 5.94 MiB 6.92 MiB 1001.19 KiB
5e8d2b3 6.15 MiB 7.13 MiB 1000.11 KiB
94757e3 6.49 MiB 7.57 MiB 1.08 MiB

Previous results on branch: fix/print-recursion-detection

Startup times

Revision Plain With Sentry Diff
c00bd14 478.42 ms 613.10 ms 134.69 ms
decaa40 440.45 ms 509.69 ms 69.24 ms

App size

Revision Plain With Sentry Diff
c00bd14 6.46 MiB 7.48 MiB 1.02 MiB
decaa40 6.46 MiB 7.48 MiB 1.02 MiB

Copy link
Contributor

github-actions bot commented Jan 30, 2025

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1237.00 ms 1261.50 ms 24.50 ms
Size 8.42 MiB 9.91 MiB 1.49 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
e5b744f 1250.82 ms 1284.46 ms 33.64 ms
0bed04d 1235.57 ms 1251.02 ms 15.45 ms
1c926c2 1246.82 ms 1263.15 ms 16.33 ms
0a23f98 1252.98 ms 1276.76 ms 23.78 ms
9d7e862 1236.88 ms 1258.62 ms 21.74 ms
f056db1 1254.16 ms 1268.38 ms 14.22 ms
1596141 1230.77 ms 1241.90 ms 11.13 ms
5aa047a 1236.57 ms 1241.02 ms 4.45 ms
519423f 1240.80 ms 1260.88 ms 20.08 ms
136c365 1248.12 ms 1277.33 ms 29.20 ms

App size

Revision Plain With Sentry Diff
e5b744f 8.09 MiB 9.07 MiB 1001.19 KiB
0bed04d 8.32 MiB 9.37 MiB 1.05 MiB
1c926c2 8.42 MiB 9.89 MiB 1.47 MiB
0a23f98 8.10 MiB 9.18 MiB 1.08 MiB
9d7e862 8.32 MiB 9.38 MiB 1.05 MiB
f056db1 8.38 MiB 9.73 MiB 1.35 MiB
1596141 8.29 MiB 9.37 MiB 1.08 MiB
5aa047a 8.29 MiB 9.39 MiB 1.10 MiB
519423f 8.10 MiB 9.16 MiB 1.06 MiB
136c365 8.38 MiB 9.75 MiB 1.37 MiB

Previous results on branch: fix/print-recursion-detection

Startup times

Revision Plain With Sentry Diff
c00bd14 1256.06 ms 1265.82 ms 9.76 ms
decaa40 1238.75 ms 1258.67 ms 19.92 ms

App size

Revision Plain With Sentry Diff
c00bd14 8.42 MiB 9.91 MiB 1.49 MiB
decaa40 8.42 MiB 9.91 MiB 1.49 MiB

@denrase denrase requested a review from buenaflor February 3, 2025 13:47
@buenaflor
Copy link
Contributor

@denrase I think the unawaited change is still missing

@denrase
Copy link
Collaborator Author

denrase commented Feb 4, 2025

@buenaflor ah sry, forgot to commit/push

@denrase denrase enabled auto-merge (squash) February 4, 2025 13:04
@@ -2,6 +2,9 @@

## Unreleased

### Fixes

- Fix print recursion detection ([#2624](https://github.com/getsentry/sentry-dart/pull/2624))
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- Fix print recursion detection ([#2624](https://github.com/getsentry/sentry-dart/pull/2624))
- Fix print recursion detection ([#2624](https://github.com/getsentry/sentry-dart/pull/2624))

@denrase denrase merged commit c15867f into main Feb 4, 2025
161 checks passed
@denrase denrase deleted the fix/print-recursion-detection branch February 4, 2025 14:48
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.

Recursion during print() call.Abort adding print() call as Breadcrumb.
2 participants