Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

fix: Sentry.init register integrations after creating main Hub instead of in the main Hub ctor #427

Merged
merged 4 commits into from
May 20, 2020
Merged

Conversation

marandaneto
Copy link
Contributor

@marandaneto marandaneto commented May 20, 2020

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

fix: Sentry.init register integrations after creating main Hub instead of in the main Hub ctor

💡 Motivation and Context

when integrations are registered on Hub ctor and async integrations are fired,
it might and actually happened that integrations called captureSomething
and the Hub was still NoOp.
Registering integrations Sentry.init(...) makes sure that the Hub is already created.

💚 How did you test it?

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • All tests passing

🔮 Next steps

Find a proper way of registering global and only once integrations.

@marandaneto marandaneto requested a review from bruno-garcia May 20, 2020 13:08
Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

happy to merge and ship.
Would be definitely important to have tests for this not to regress

@marandaneto marandaneto changed the title async integrations and noop hub Sentry.init register integrations after creating main Hub instead of on the Hub ctor. May 20, 2020
@marandaneto marandaneto changed the title Sentry.init register integrations after creating main Hub instead of on the Hub ctor. fix: Sentry.init register integrations after creating main Hub instead of in the main Hub ctor May 20, 2020
@marandaneto marandaneto marked this pull request as ready for review May 20, 2020 13:58
@codecov-commenter
Copy link

codecov-commenter commented May 20, 2020

Codecov Report

Merging #427 into master will decrease coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #427      +/-   ##
============================================
- Coverage     60.04%   59.96%   -0.09%     
+ Complexity      813      812       -1     
============================================
  Files            93       93              
  Lines          3742     3742              
  Branches        363      363              
============================================
- Hits           2247     2244       -3     
- Misses         1339     1343       +4     
+ Partials        156      155       -1     
Impacted Files Coverage Δ Complexity Δ
sentry-core/src/main/java/io/sentry/core/Hub.java 65.06% <ø> (-0.32%) 59.00 <0.00> (-1.00)
...ntry-core/src/main/java/io/sentry/core/Sentry.java 32.32% <100.00%> (+2.11%) 8.00 <0.00> (+1.00)
...ntry/core/UncaughtExceptionHandlerIntegration.java 71.05% <0.00%> (-3.95%) 9.00% <0.00%> (-1.00%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ce5093...952e9c7. Read the comment docs.

@marandaneto marandaneto merged commit 69dc554 into getsentry:master May 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants