-
Notifications
You must be signed in to change notification settings - Fork 7.6k
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
1.x: Fix NPE in CompositeException when nested throws on initCause #3620
Conversation
👍 |
@@ -122,6 +122,7 @@ public synchronized Throwable getCause() { | |||
// ignore | |||
// the javadocs say that some Throwables (depending on how they're made) will never | |||
// let me call initCause without blowing up even if it returns null | |||
chain = e; | |||
} | |||
chain = chain.getCause(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this line should be moved into the try
clause. Otherwise, chain
will be assigned twice here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zsxwing done.
👍 @msavitskiy can you please squash commits in your PR into one? |
@artem-zinnatullin I make squash commits. I did the right thing? |
@msavitskiy mm, nope… Basically you should have one commit after squash done right. I'd suggest to read documentation and some examples (maybe even videos) about If it'll be blocker for merge I can squash your PR and resubmit it (you'll still be author of the commit!) |
Move line to try block. For avoid assigned twice.
@artem-zinnatullin Now everything is fine? |
@msavitskiy yes, thanks! |
👍 |
1.x: Fix NPE in CompositeException when nested throws on initCause
possible solution :)