spring-retry icon indicating copy to clipboard operation
spring-retry copied to clipboard

Populate MethodInvocation to RetryPolicy (via RetryContext)

Open ttddyy opened this issue 4 years ago • 1 comments

I would like to get MethodInvocation in RetryPolicy(probably via RetryContext) when available(invoked with MethodInvocationRetryCallback); so that, the policy can dynamically determine whether to retry based on the invoked method.

Since 1.3, MethodInvocationRetryCallback exposes MethodInvocation but it is only available to RetryListener. The RetryListener#open could use this MethodInvocation; however, returning false here ends up TerminatedRetryException. So, this cannot be used to determine retry based on the invoked method.

My usecase is that I am writing a retry interceptor(MethodInterceptor) for transaction retry. It determines the retry based on the invoked method(TransactionDefinition applied on the invoked method/class). Currently, in order to get invoked method, I apply ExposeInvocationInterceptor from spring to where spring advises for transactions. Then, my RetryPolicy retrieves the invoked method from ThreadLocal stashed by the ExposeInvocationInterceptor.

If MethodInvocation is available in RetryContext of RetryPolicy, I can directly acquire the invoked method and ExposeInvocationInterceptor is no longer required.

ttddyy avatar Jan 20 '21 00:01 ttddyy

I guess it would be good to look at the change, and see some tests. Anyone feel like sending a PR?

dsyer avatar Jun 14 '21 13:06 dsyer

The ExposeInvocationInterceptor states clearly:

Don't use this interceptor unless this is really necessary. Target objects should not normally know about Spring AOP, as this creates a dependency on Spring API. Target objects should be plain POJOs as far as possible.

I guess same applies here if we would expose a MethodInvocation into a RetryContext. We currently do have this though:

		RetryCallback<Object, Throwable> retryCallback = new MethodInvocationRetryCallback<>(invocation, this.label) {

			@Override
			public Object doWithRetry(RetryContext context) throws Exception {

				context.setAttribute(RetryContext.NAME, this.label);
				context.setAttribute("ARGS", new Args(invocation.getArguments()));

I believe those ARGS are still not enough for your logic. So, probably something like extra:

context.setAttribute("METHOD", invocation.getMethod());

Would satisfy your requirements.

Let me know, and I'll fix it quickly!

artembilan avatar Sep 12 '24 17:09 artembilan

After sleeping on this, I think I'll fix it as I explained. That method attribute would fix your expectations and that would be a good framework opinion how to deal with this objects without exposing Spring AOP API into end-user RetryPolicy.

artembilan avatar Sep 13 '24 13:09 artembilan