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: Avoid NPE when MDC contains null values #1385

Merged
merged 9 commits into from
Apr 9, 2021

Conversation

wreulicke
Copy link
Contributor

@wreulicke wreulicke commented Apr 8, 2021

📜 Description

Avoid NPE when MDC contains null values
Follow up #1364

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

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

🔮 Next steps

@wreulicke wreulicke changed the title Avoid NPE when MDC contains null values Fix: Avoid NPE when MDC contains null values Apr 8, 2021
@marandaneto
Copy link
Contributor

@wreulicke why is it a follow-up of #1364 ? doesn't #1364 fix the issue at all?

@wreulicke
Copy link
Contributor Author

@marandaneto
To be exact, This fixes the issues similar to #1364.
follow up might not be accurate.

CHANGELOG.md Outdated Show resolved Hide resolved
@marandaneto
Copy link
Contributor

@marandaneto
To be exact, This fixes the issues similar to #1364.
follow up might not be accurate.

I just noticed its pretty much the same fix but for sentry-jul, that was the issue, I thought you were just fixing the same issue for sentry-logback :)

mind writing a unit test? thanks

@codecov-io
Copy link

Codecov Report

Merging #1385 (cbe4ae4) into main (142dd44) will increase coverage by 0.03%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #1385      +/-   ##
============================================
+ Coverage     75.68%   75.72%   +0.03%     
- Complexity     1858     1861       +3     
============================================
  Files           185      185              
  Lines          6363     6368       +5     
  Branches        636      637       +1     
============================================
+ Hits           4816     4822       +6     
  Misses         1258     1258              
+ Partials        289      288       -1     
Impacted Files Coverage Δ Complexity Δ
...jul/src/main/java/io/sentry/jul/SentryHandler.java 75.00% <100.00%> (+1.89%) 34.00 <2.00> (+3.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 142dd44...cbe4ae4. Read the comment docs.

@wreulicke
Copy link
Contributor Author

@marandaneto

I have written the unit test.
Could you check this PR?

Copy link
Contributor

@marandaneto marandaneto left a comment

Choose a reason for hiding this comment

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

LGTM

@marandaneto
Copy link
Contributor

@marandaneto

I have written the unit test.
Could you check this PR?

all good, thanks for the contribution @wreulicke

@marandaneto marandaneto merged commit 9d8b774 into getsentry:main Apr 9, 2021
@wreulicke wreulicke deleted the patch-1 branch August 5, 2021 09:10
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.

3 participants