Add @Nullable in RetryContext to easier detect possible NPE

[szpak]

Both getParent() and getLastThrowable() might return null, as mentioned in javadoc. @Nullable helps an IDE warns developers about potential NPE.

It is a reminiscence of the problem, I've encountered recently, incorrectly assuming that onSuccess will only be executed if there was at least failed attempt before (which resulted in no throwable and NPE).

[szpak]

Btw, not strictly related to the PR, but re-reading javadoc:

	/**
	 * Accessor for the exception object that caused the current retry.
	 * @return the last exception that caused a retry, or possibly null. It will be null
	 * if this is the first attempt, but also if the enclosing policy decides not to
	 * provide it (e.g. because of concerns about memory usage).
	 */
	Throwable getLastThrowable();

I wonder, if "It will be null if this is the first attempt" is fully correct. In onError(), in the first attempt, it should be set. Only in onSuccess() in the first (successful) attempt it will be null. WDYT?

[artembilan]

The logic there is like this:

		T result = retryCallback.doWithRetry(context);
					doOnSuccessInterceptors(retryCallback, context, result);
					return result;
				}
				catch (Throwable e) {

					lastException = e;

					try {
						registerThrowable(retryPolicy, state, context, e);
					}
					catch (Exception ex) {
						throw new TerminatedRetryException("Could not register throwable", ex);
					}
					finally {
						doOnErrorInterceptors(retryCallback, context, e);
					}

So, onSuccess() might be called immediately, or after some number of retries.
The onError() is always called for any failed attempt.

Therefore I think Javadoc is correct: if we call service and it does not fail we immediately go to the onSuccess() and exit from retry logic.
The "first attemp" is exactly that T result = retryCallback.doWithRetry(context); when we just entered to the retry loop.

[szpak]

The "first attemp" is exactly that T result = retryCallback.doWithRetry(context); when we just entered to the retry loop.

After NPE in the tests, I analyzed the code and draw similar conclusions :-). However, I would phrase it rather:

It will be null if this is the first attempt and it finishes successfully

or

It might be null in the first attempt (if it finishes successfully)

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 ;-) ).

[artembilan]

Good. Feel free to update that Javadoc with your first suggestion as part of this PR and I’ll merge ASAP.

[szpak]

Good. Feel free to update that Javadoc with your first suggestion as part of this PR and I’ll merge ASAP.

Done. I also amended the original commit to "fix" the "Unverified" signature warning.

[artembilan]

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

[szpak]

I've just pushed the improved version.

[artembilan]

@szpak ,

thank you for contribution; looking forward for more!