grpc-ecosystem / grpc-spring

Spring Boot starter module for gRPC framework.
https://grpc-ecosystem.github.io/grpc-spring/
Apache License 2.0
3.48k stars 816 forks source link

StreamObserver.onError() call on different instance when client gets SIGKILL (CANCELLED: client cancelled) #870

Closed Philipp-p closed 1 year ago

Philipp-p commented 1 year ago

The context

I'm working on failure scenarios for an application. Therefore I tested what is happening if a client receives SIGKILL during an ongoing bidirectional stream.

The question

I have an ongoing bidirectional stream between my client and my server. I then terminate my client with SIGKILL. This leads to a io.grpc.StatusRuntimeException: CANCELLED: client cancelled and a call to my onError() implementation of the StreamObserver on the server-side. My issue is that the call is not executed on the same StreamObserver instance as onNext() earlier. This leads to the problem that some attributes in my StreamObserver implementation are null during the onError() call instead of the values I set them to earlier. This does not happen when for example an exception is thrown during the onNext() execution. In this case the onError() is executed on the same instance. My question here is this intended behaviour or not? Also if this is not intended, should I raise this issue with the grpc-java project or was I correct opening the issue here?

Stacktraces and logs

To make my issue more clear if I log the instance (with System.out.print("SOME STRING" + this)) my methods are called during the bidirectional stream would look like something like this:

onNext() call instance: ClassName@AAAA
onNext() call instance: ClassName@AAAA

[the clients gets SIGKILL in the meatime]

onError() call instance: ClassName@BBBB

The application's environment

Which versions do you use?

Additional information

Any input or help is highly appreciated!

ST-DDT commented 1 year ago

That sounds strange to me. I assume you create one observer for each bidi-request, so there shouldn't be any unaccounted for. Also this error seams unrelated to spring/this library, so you should probably check grpc java for help. But please post a link to your question here, so I can watch that thread and maybe expand my knowledge for future questions.

Sorry, that I'm unable to help. Have a nice weekend.

Philipp-p commented 1 year ago

Thanks for the fast answer as always! I'm using the @Scope(scopeName = "grpcRequest", proxyMode = ScopedProxyMode.TARGET_CLASS) notation above my class, so I assume there should be only one per call as per the documentation. I'll post the link once I make the issue, probably on Monday, when I'm back at work.

Have a nice weekend as well!

ST-DDT commented 1 year ago

You use a scoped StreamObserver bean? Then I guess I can understand the problem. The request context gets destroyed by the cancellation. I'm not sure how to delay the bean destruction (since it is bound to the grpc context cancellation).

Could you please debug that to check whether my assumption is correct?

Philipp-p commented 1 year ago

Sorry for not getting back to you sooner. I just debugged the issue in a more basic way as you suggested and it seems you are correct. I tested this by using the Javax annotation @PreDestroy in my StreamObserver. The bean gets destroyed right before the onError() call and the application logging the StatusRuntimeException: CANCELLED: client cancelled. This will probably allow me to circumvent the issue by using the @PreDestroy annotated method. However if I should debug the issue in the way you have suggested, just let me know. My question remains if I should raise the issue with the grpc-java project? For my needs the workaround probably will solve my problems, but this of course feels rather unclean and a quick and dirty fix from my end.

ST-DDT commented 1 year ago

My question remains if I should raise the issue with the grpc-java project?

Yes, IMO the on error callback belongs to the request scope and thus the grpc context should still be valid at that point. This should be a grpc-java bug. Please post a link here so I can follow the issue.

Philipp-p commented 1 year ago

I opened the issue grpc/grpc-java#9996

Philipp-p commented 1 year ago

As with the issue over at grpc-java. I think for my needs everything is resolved. Thanks again for your help and support with the other issue!