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

Populate MethodInvocation to RetryPolicy (via RetryContext) #229

Closed
ttddyy opened this issue Jan 20, 2021 · 3 comments
Closed

Populate MethodInvocation to RetryPolicy (via RetryContext) #229

ttddyy opened this issue Jan 20, 2021 · 3 comments
Milestone

Comments

@ttddyy
Copy link

ttddyy commented Jan 20, 2021

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.

@dsyer
Copy link
Member

dsyer commented Jun 14, 2021

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

@artembilan
Copy link
Member

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
Copy link
Member

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 artembilan added this to the 2.0.9 milestone Sep 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants