-
Notifications
You must be signed in to change notification settings - Fork 434
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
Error interceptor does not work as expected when using R2DBC repositories #342
Comments
Does this answer your question ? |
Looking at this line, if you throw exception from |
Anyway, if you prefer to work with |
I tried too, in order to have reactive API in handlers. I got the same issue.
Good point, I've replaced with this ugly code without any luck:
Thank you ! I withdraw the @transactional and It works... I just wonder how to deal with transactions with such a limitation. |
Suggest you to ask this question spring team, they should probably have the If |
Another problem in your test :
|
I did it with @transactional, put some more logs
and I tested with both
and
I have the exact same behavior and the same logs:
And in both cases (return Flux.error or throw it) the streamObserver.onError is correctly called (see line 2023-03-30T17:01:29.400+02:00 ERROR 74371 --- [tor-tcp-epoll-1] reactor.Mono.MapFuseable.1 : | onError(java.lang.IllegalStateException: test) printed by |
Some takeaways:
Actually, to correctly handle the exception I have to do this
But I expected the GRpcExceptionHandlerInterceptor to do this automatically. I did the same experience with your concurrent starter and It worked as expected (that is I can handle all exceptions with a global interceptor), but I like your stack too ! |
I did some tests, Spring Tx interceptor has LowestPrecedence and GRpcExceptionHandlerInterceptor has HighestPrecedence.
Always the same outcome. I tried to get some debug logs with Note: If you look at the log of my LogGrpcInterceptor and add |
Well the answer is : their interceptor has precedence over Spring Tx interceptor out of the box. |
Line 22 in 1ca3036
|
Have a closer look at this blog post :
Looks you have no choice but extract the transactional method into separate service that returns I have doubts that order of tx interceptor matters here because I'm processing |
This is already what I do, don't I ?
No, that's why there is probably an issue when using this starter with r2dbc. I tested various config of precedence without any effect, there is a smell here... |
Can you please try to add |
If you put break point here Line 34 in 1ca3036
|
I did, same outcome, I stopped in each break point you specified before Spring Tx manager wrote any logs in the console. You can give it a try yourself with the project I packaged, it should run rather easily on any laptop. For the moment I have to consider that this starter is not compatible with a fully reactive stack, whereas this one is. |
@vincentditlevinz , I was able to reproduce the issue (not related to |
Hello @jvmlet.
|
Hmm, from the stack trace it seems that transactional some how switches threads and the context is lost... I'll continue with investigation, please keep the repo alive... |
@vincentditlevinz , I've added test that reproduces the behavior of your project, test passes after the fix, please upgrade to the latest |
Hello @jvmlet. |
Thanks @vincentditlevinz for helping with the resolution. |
@vincentditlevinz , |
Hello,
I tried to make
LogNet/grpc-spring-boot-starter
work with R2DBC Spring data repositories. Here is a minimal project that helps to reproduce the issue.Issue:
When an exception is thrown, the error interceptor (GRpcExceptionHandlerInterceptor) provided by the starter does not catch it and thus does not provide the correct error code to the client (see the readme file of my project for more details).
This is due to the way I map R2DBC results (I believe this is the right way to leverage R2DBC reactor api, isn't it ?):
Note that the interceptor "works" if I use:
With the following helper which unfortunately uses toIterable()
but I believe this is not the right way if I want to "really be reactive".
The text was updated successfully, but these errors were encountered: