-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Inconsistencies in Bidirectional Streaming Error Handling #7558
Comments
Thanks for your behavior report and easy-to-reproduce example project. We recently had some discussion about how users should handle exceptions on StreamObserver, #7172 (and the merged #7173) is open trying to make such things clearer (note that PR may still have some pending changes). I believe we do have some vague definitions on how errors should be propagated/bounced/handled, they may be inconsistent on the two sides. It's confusing and not well-documented. We will bring up this for discussion and get back to you once we had some progressive outcome. |
Thanks @voidzcy , let me know if I can help in any way 😄 |
Most of the observations described are working as intended, and the way how it works is just the nature of gRPC and HTTP/2 protocol. You can see the gRPC protocol design at gRPC over HTTP2. Client ends RPC by cancellation (sends a RST_STREAM frame) while server ends RPC by finishing the stream (encodes status to stream trailer and mark END_STREAM). If you look at the interfaces for ClientCall and ServerCall (and their Listeners), you will see the difference. From StreamObserver API's perspective, the outbound onError() semantics for client and server are still the same, which terminates the call/stream. Information passed to client's outbound onError() is never transmitted to the server, server side only sees the cancellation. The "client cancelled" description is just the canonical message attached by server implementation. After the RST_STREAM frame is sent, the client side call is considered to be closed without waiting for future server messages. The ClientCall.Listener is immediately notified with CANCELLED status including the original cause. The Throwable passed to server's outbound onError() should be However, ServerCall.Listener's onComplete() does not call anything on server's inbound StreamObserver in this case. (Note for client cancel or half-close, server's inbound StreamObserver is notified correctly by ServerCall.Listener's onCancel() or onHalfClose()). This is something we are trying to discuss. Calling onError() might be a little strange and we need to think about what status code and description to deliver, while calling onCompleted() may make it more confusing for server applications. |
@voidzcy sorry for the delayed response. |
@voidzcy In #8174, I encountered a situation where client's inbound onError was sometimes not called after client's outbound onError call. This observation defies your explanation below (I assume client's inbound onError is a ClientCall.Listener)
Do you happen to know a case where ClientCall.Listener is not called upon client's outbound onError()? |
There is actually a ClientCall.Listener class and that's the reference here (I suspect). So that's different from the ResponseObserver's
|
One thing I can think of: there is an extremely small time window for the race between outbound grpc-java/core/src/main/java/io/grpc/internal/DelayedClientCall.java Lines 205 to 233 in 869b395
This happens when a real call has not been created (e.g., name resolution has not completed) and the outbound |
@voidzcy Thank you for response. In my case, I didn't find "deadline exceeded" in the log. The problem was (mysteriously) resolved by changing the use of CountDownLatch. #8174 (comment) Thank you for your attention! |
The code that @voidzcy linked to doesn't appear to be a problem to me, as when it uses noopCall it also uses CloseListenerRunnable which will call the listener. @suztomo, it sounds like some application code was interacting with the RPC from multiple threads simultaneously. The call object isn't thread-safe, so if that happens all bets are off. |
Oops, my bad. I overlooked that |
From an API review meeting in January:
|
What version of gRPC-Java are you using?
1.33.0
What is your environment?
macOS 10.15 (but also tested on Ubuntu 18.04), JDK version 11
What did you expect to see?
Documentation on when and what onError() callbacks are called when receiving errors from either side (server / client), and a consistent behavior in both.
Also - a consistent error (and documentation) of using a stream after an error has been raised on it.
What did you see instead?
Multiple inconsistencies in Exception classes and whether they've even been thrown.
I performed some tests and summarized the results on my blog post: https://lewin.co.il/inconsistencies-in-grpc-java-error-handling-with-bidirectional-streams
Steps to reproduce the bug
A project reproducing all the above bugs can be found here: https://github.com/GuyLewin/grpc-bidirectional-streaming-error-handling
The text was updated successfully, but these errors were encountered: