Skip to content

Fix print recursion detection#2624

Merged
denrase merged 6 commits into
mainfrom
fix/print-recursion-detection
Feb 4, 2025
Merged

Fix print recursion detection#2624
denrase merged 6 commits into
mainfrom
fix/print-recursion-detection

Conversation

@denrase

@denrase denrase commented Jan 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Collaborator

📜 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
@codecov

codecov Bot commented Jan 28, 2025 •
edited
Loading

Copy link
Copy Markdown

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.

@github-actions

github-actions Bot commented Jan 28, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

buenaflor
buenaflor reviewed Jan 28, 2025
View reviewed changes
Comment thread dart/lib/src/sentry_run_zoned_guarded.dart Outdated
@github-actions

github-actions Bot commented Jan 30, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

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

buenaflor commented Feb 3, 2025

Copy link
Copy Markdown
Contributor

@denrase I think the unawaited change is still missing

@denrase

denrase commented Feb 4, 2025

Copy link
Copy Markdown
Collaborator Author

@buenaflor ah sry, forgot to commit/push

@denrase
denrase enabled auto-merge (squash) February 4, 2025 13:04
buenaflor
buenaflor approved these changes Feb 4, 2025
View reviewed changes
Comment thread CHANGELOG.md

### Fixes

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

@buenaflor buenaflor Feb 4, 2025

Copy link
Copy Markdown
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
@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

Reviewers

@buenaflor buenaflor buenaflor approved these changes
@krystofwoldrich krystofwoldrich Awaiting requested review from krystofwoldrich
@stefanosiano stefanosiano Awaiting requested review from stefanosiano

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

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

2 participants

@denrase @buenaflor