Skip to content

fix: catch errors thrown during handleBeginFrame and handleDrawFrame#2446

Merged
buenaflor merged 5 commits into
mainfrom
fix/catch-errors-in-frame-methods
Nov 28, 2024
Merged

fix: catch errors thrown during handleBeginFrame and handleDrawFrame#2446
buenaflor merged 5 commits into
mainfrom
fix/catch-errors-in-frame-methods

Conversation

@buenaflor

@buenaflor buenaflor commented Nov 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

📜 Description

Catch errors thrown during handleBeginFrame and handleDrawFrame

💡 Motivation and Context

If these aren't caught, it could potentially disrupt the frame processing and thus freeze the UI -> not good

💚 How did you test it?

Unit test, manual

📝 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

@codecov

codecov Bot commented Nov 25, 2024 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 63.31%. Comparing base (05933ac) to head (fec2346).
Report is 1 commits behind head on main.

❗ There is a different number of reports uploaded between BASE (05933ac) and HEAD (fec2346). Click for more details.

HEAD has 8 uploads less than BASE
Flag BASE (05933ac) HEAD (fec2346)
9 1
Additional details and impacted files 
@@             Coverage Diff             @@
##             main    #2446       +/-   ##
===========================================
- Coverage   86.84%   63.31%   -23.54%     
===========================================
  Files         260        4      -256     
  Lines        9239      169     -9070     
===========================================
- Hits         8024      107     -7917     
+ Misses       1215       62     -1153

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

@github-actions

github-actions Bot commented Nov 25, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1255.85 ms 1274.62 ms 18.77 ms
Size 8.38 MiB 9.78 MiB 1.40 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
bd1b990 1209.43 ms 1226.96 ms 17.53 ms
c73ab67 1267.73 ms 1279.36 ms 11.63 ms
08a7b4f 1277.10 ms 1303.37 ms 26.27 ms
6325c3b 1266.52 ms 1291.06 ms 24.54 ms
bd37365 1236.22 ms 1243.27 ms 7.04 ms
f770c4c 1204.61 ms 1222.82 ms 18.20 ms
547db82 1248.23 ms 1260.90 ms 12.67 ms
732a7b4 1242.17 ms 1266.96 ms 24.79 ms
bec58b2 1252.35 ms 1272.19 ms 19.84 ms
051e97a 1245.94 ms 1249.51 ms 3.57 ms

App size

Revision Plain With Sentry Diff
bd1b990 8.32 MiB 9.38 MiB 1.06 MiB
c73ab67 8.29 MiB 9.36 MiB 1.07 MiB
08a7b4f 8.16 MiB 9.16 MiB 1.01 MiB
6325c3b 8.16 MiB 9.17 MiB 1.01 MiB
bd37365 8.28 MiB 9.34 MiB 1.06 MiB
f770c4c 8.32 MiB 9.38 MiB 1.06 MiB
547db82 8.38 MiB 9.74 MiB 1.36 MiB
732a7b4 8.32 MiB 9.38 MiB 1.06 MiB
bec58b2 8.38 MiB 9.75 MiB 1.37 MiB
051e97a 8.28 MiB 9.34 MiB 1.06 MiB

Previous results on branch: fix/catch-errors-in-frame-methods

Startup times

Revision Plain With Sentry Diff
0c2aa79 1251.79 ms 1274.73 ms 22.94 ms

App size

Revision Plain With Sentry Diff
0c2aa79 8.38 MiB 9.78 MiB 1.40 MiB

denrase
denrase approved these changes Nov 26, 2024
View reviewed changes

@denrase denrase left a comment •
edited
Loading

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.

LGTM. We do have the option to re-trow during tests. Dunno if it makes sense to add this here as well.

if (_options.automatedTestMode) {
  rethrow;
}

@github-actions

github-actions Bot commented Nov 26, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 517.53 ms 611.42 ms 93.89 ms
Size 6.49 MiB 7.56 MiB 1.08 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
f9d18f3 430.94 ms 497.59 ms 66.65 ms
f96ca24 316.08 ms 388.22 ms 72.14 ms
cdf7172 348.54 ms 390.81 ms 42.27 ms
6aab859 306.27 ms 377.92 ms 71.65 ms
5aab4c5 326.12 ms 378.82 ms 52.70 ms
1d47eb7 282.26 ms 342.52 ms 60.26 ms
b930b94 484.42 ms 525.51 ms 41.09 ms
2a0edba 376.12 ms 454.00 ms 77.88 ms
136c365 454.54 ms 505.58 ms 51.04 ms
81bb164 443.79 ms 485.39 ms 41.60 ms

App size

Revision Plain With Sentry Diff
f9d18f3 6.15 MiB 7.13 MiB 1000.49 KiB
f96ca24 6.06 MiB 7.10 MiB 1.04 MiB
cdf7172 5.94 MiB 6.95 MiB 1.01 MiB
6aab859 6.15 MiB 7.13 MiB 999.97 KiB
5aab4c5 6.06 MiB 7.10 MiB 1.04 MiB
1d47eb7 6.16 MiB 7.14 MiB 1010.29 KiB
b930b94 6.49 MiB 7.57 MiB 1.08 MiB
2a0edba 6.33 MiB 7.30 MiB 992.46 KiB
136c365 6.49 MiB 7.57 MiB 1.08 MiB
81bb164 6.49 MiB 7.56 MiB 1.07 MiB

Previous results on branch: fix/catch-errors-in-frame-methods

Startup times

Revision Plain With Sentry Diff
0c2aa79 447.67 ms 504.20 ms 56.53 ms

App size

Revision Plain With Sentry Diff
0c2aa79 6.49 MiB 7.56 MiB 1.07 MiB

@buenaflor
buenaflor merged commit acc307e into main Nov 28, 2024
@buenaflor
buenaflor deleted the fix/catch-errors-in-frame-methods branch November 28, 2024 20:37
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.

2 participants

@buenaflor @denrase