Add @Nullable in RetryContext to easier detect possible NPE#457
Conversation
|
Btw, not strictly related to the PR, but re-reading javadoc: I wonder, if "It will be null if this is the first attempt" is fully correct. In |
|
The logic there is like this: So, Therefore I think Javadoc is correct: if we call service and it does not fail we immediately go to the |
After NPE in the tests, I analyzed the code and draw similar conclusions :-). However, I would phrase it rather:
or
It emphasizes that only on success, it will be null (in the first failed attempt it should not be null - unless "the enclosing policy decides not to provide it"). Nevertheless, it's just wording and the current javadoc is pretty readable (if you read it first ;-) ). |
|
Good. Feel free to update that Javadoc with your first suggestion as part of this PR and I’ll merge ASAP. |
Both getParent() and getLastThrowable() might return null, as mentioned in javadoc. @nullable helps an IDE warns developers about potential NPE.
fcfe975 to
67996be
Compare
Done. I also amended the original commit to "fix" the "Unverified" signature warning. |
artembilan
left a comment
There was a problem hiding this comment.
Error: Failed to execute goal io.spring.javaformat:spring-javaformat-maven-plugin:0.0.41:validate (default) on project spring-retry: Formatting violations found in the following files:
Error: * /home/runner/work/spring-retry/spring-retry/src/main/java/org/springframework/retry/RetryContext.java
Please, consider to run this Maven goal locally: spring-javaformat:apply.
And then push the changes.
Also add your name to the @author list of the modified class.
Thanks
|
I've just pushed the improved version. |
|
@szpak , thank you for contribution; looking forward for more! |
Both
getParent()andgetLastThrowable()might return null, as mentioned in javadoc.@Nullablehelps an IDE warns developers about potential NPE.It is a reminiscence of the problem, I've encountered recently, incorrectly assuming that
onSuccesswill only be executed if there was at least failed attempt before (which resulted in no throwable and NPE).