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(java): Ensure potential callback exceptions are caught #2123 #2291

Merged
merged 5 commits into from Oct 14, 2022

Conversation

markushi
Copy link
Member

@markushi markushi commented Oct 12, 2022

📜 Description

Try/Catches callback executions and add logging in case of errors.

💡 Motivation and Context

If certain callbacks provided via the Sentry, Hub or SentryOptions crash during execution, it could crash the whole application. So let's catch 'em all 😄

Related issues:

Callbacks which were already handled:

  • BeforeSendCallback
  • BeforeBreadcrumbCallback

Callbacks:

  • SentryOptions.TracesSamplerCallback
  • SentryOptions.ProfilesSamplerCallback
  • OptionsConfiguration.configure
  • Hub.captureMessage
  • Hub.captureException
  • Hub.captureEvent
  • Hub.withScope
  • Hub.configureScope

💚 How did you test it?

Added tests where required

📝 Checklist

  • [ x ] I reviewed the submitted code
  • [ x ] I added tests to verify the changes
  • [ x ] I updated the docs if needed
  • [ x ] No breaking changes

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Oct 12, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 310.88 ms 339.26 ms 28.38 ms
Size 1.73 MiB 2.29 MiB 580.10 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
7300956 337.57 ms 384.21 ms 46.64 ms
4ca1d7b 328.46 ms 368.22 ms 39.76 ms
54cebc8 300.86 ms 341.43 ms 40.57 ms
c5ccd8a 329.98 ms 365.52 ms 35.54 ms
1e4690d 354.69 ms 387.88 ms 33.19 ms
4dd88fe 306.88 ms 391.58 ms 84.70 ms
4dd88fe 302.12 ms 331.17 ms 29.04 ms
7300956 324.20 ms 353.79 ms 29.58 ms
3d89dea 345.59 ms 364.06 ms 18.47 ms
54cebc8 331.12 ms 385.14 ms 54.02 ms

App size

Revision Plain With Sentry Diff
7300956 1.73 MiB 2.29 MiB 578.69 KiB
4ca1d7b 1.73 MiB 2.29 MiB 579.88 KiB
54cebc8 1.73 MiB 2.29 MiB 579.43 KiB
c5ccd8a 1.74 MiB 2.33 MiB 607.44 KiB
1e4690d 1.74 MiB 2.33 MiB 604.92 KiB
4dd88fe 1.73 MiB 2.29 MiB 579.50 KiB
4dd88fe 1.73 MiB 2.29 MiB 579.50 KiB
7300956 1.73 MiB 2.29 MiB 578.69 KiB
3d89dea 1.74 MiB 2.33 MiB 604.92 KiB
54cebc8 1.73 MiB 2.29 MiB 579.43 KiB

Previous results on branch: fix/capture-callback-exceptions

Startup times

Revision Plain With Sentry Diff
a6facbc 305.74 ms 333.83 ms 28.09 ms
179e40b 308.38 ms 313.78 ms 5.40 ms
558a3bd 277.29 ms 350.02 ms 72.73 ms
2e5138d 276.08 ms 343.17 ms 67.09 ms

App size

Revision Plain With Sentry Diff
a6facbc 1.73 MiB 2.29 MiB 579.94 KiB
179e40b 1.73 MiB 2.29 MiB 579.96 KiB
558a3bd 1.73 MiB 2.29 MiB 579.94 KiB
2e5138d 1.73 MiB 2.29 MiB 579.94 KiB

@codecov-commenter
Copy link

codecov-commenter commented Oct 12, 2022

Codecov Report

Base: 79.96% // Head: 80.10% // Increases project coverage by +0.13% 🎉

Coverage data is based on head (1171870) compared to base (7d87f22).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #2291      +/-   ##
============================================
+ Coverage     79.96%   80.10%   +0.13%     
- Complexity     3425     3430       +5     
============================================
  Files           242      242              
  Lines         12734    12754      +20     
  Branches       1702     1702              
============================================
+ Hits          10183    10216      +33     
+ Misses         1901     1889      -12     
+ Partials        650      649       -1     
Impacted Files Coverage Δ
sentry/src/main/java/io/sentry/Hub.java 75.53% <100.00%> (+1.08%) ⬆️
sentry/src/main/java/io/sentry/Sentry.java 53.17% <100.00%> (+6.79%) ⬆️
sentry/src/main/java/io/sentry/TracesSampler.java 100.00% <100.00%> (ø)
sentry/src/main/java/io/sentry/IHub.java 91.66% <0.00%> (+4.16%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@markushi markushi linked an issue Oct 12, 2022 that may be closed by this pull request
Copy link
Member

@romtsn romtsn left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@markushi markushi merged commit adee765 into main Oct 14, 2022
@markushi markushi deleted the fix/capture-callback-exceptions branch October 14, 2022 06:36
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.

Some callbacks do not have their Exceptions caught yet
4 participants