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.

Sign up for GitHub

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

doc: Document error handling and Observer callback behavior #11876

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

doc: Document error handling and Observer callback behavior #11876

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68b4f42
Changes
kannanjgithub Feb 1, 2025
916e848
Documenting error transmission and callback behavior.
kannanjgithub Feb 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions stub/src/main/java/io/grpc/stub/ClientCalls.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,13 @@ private ClientCalls() {}
*
* <p>If the provided {@code responseObserver} is an instance of {@link ClientResponseObserver},
* {@code beforeStart()} will be called.
*
* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds like it should only be on server-side. Let's talk more about the client's perspective.

If the server completes the RPC with status code OK, then {@code responseObserver.onCompleted()}
is called at the end of the RPC. Otherwise the status and trailers are passed as a Throwable to
{@code onError()} and can be accessed with {@link Status#fromThrowable} and {@link
Status#trailersFromThrowable}.

* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be
* received by the {@link ClientCall}'s onClose, and UNKNOWN status code otherwise. Its
* description will be encoded to the stream trailer, but the cause (which may contain server
* application's information) will not.
*/
public static <ReqT, RespT> void asyncUnaryCall(
ClientCall<ReqT, RespT> call, ReqT req, StreamObserver<RespT> responseObserver) {
Expand All @@ -84,6 +91,13 @@ public static <ReqT, RespT> void asyncUnaryCall(
*
* <p>If the provided {@code responseObserver} is an instance of {@link ClientResponseObserver},
* {@code beforeStart()} will be called.
*
* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be
* received by the {@link ClientCall}'s onClose, and UNKNOWN status code otherwise. Its
* description will be encoded to the stream trailer, but the cause (which may contain server
* application's information) will not.
*/
public static <ReqT, RespT> void asyncServerStreamingCall(
ClientCall<ReqT, RespT> call, ReqT req, StreamObserver<RespT> responseObserver) {
Expand All @@ -100,6 +114,23 @@ public static <ReqT, RespT> void asyncServerStreamingCall(
* {@code beforeStart()} will be called.
*
* @return request stream observer. It will extend {@link ClientCallStreamObserver}
*
* <h3>Client errors</h3>
* onError called on the request stream observer will result in stream cancellation. The response
* {@link StreamObserver} will be immediately notified of the cancellation with a
* {@link io.grpc.StatusRuntimeException} with the exception passed to onError set as the cause
* and the stream is considered closed. The server's request stream observer will receive an
* onError callback with a {@link io.grpc.StatusRuntimeException} for the cancellation with the
* message 'Client cancelled', and exception cause set to null because the actual exception
* passed by the client to onError is never actually transmitted to the server and the server
* just receives a RST_STREAM frame indicating cancellation by the client.
*
* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be
* received by the {@link ClientCall}'s onClose, and UNKNOWN status code otherwise. Its
* description will be encoded to the stream trailer, but the cause (which may contain server
* application's information) will not.
*/
public static <ReqT, RespT> StreamObserver<ReqT> asyncClientStreamingCall(
ClientCall<ReqT, RespT> call,
Expand All @@ -116,6 +147,23 @@ public static <ReqT, RespT> StreamObserver<ReqT> asyncClientStreamingCall(
* {@code beforeStart()} will be called.
*
* @return request stream observer. It will extend {@link ClientCallStreamObserver}
*
* <h3>Client errors</h3>
* onError called on the request stream observer will result in stream cancellation. The response
* {@link StreamObserver} will be immediately notified of the cancellation with a
* {@link io.grpc.StatusRuntimeException} with the exception passed to onError set as the cause
* and the stream is considered closed. The server's request stream observer will receive an
* onError callback with a {@link io.grpc.StatusRuntimeException} for the cancellation with the
* message 'Client cancelled', and exception cause set to null because the actual exception
* passed by the client to onError is never actually transmitted to the server and the server
* just receives a RST_STREAM frame indicating cancellation by the client.
*
* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be
* received by the {@link ClientCall}'s onClose, and UNKNOWN status code otherwise. Its
* description will be encoded to the stream trailer, but the cause (which may contain server
* application's information) will not.
*/
public static <ReqT, RespT> StreamObserver<ReqT> asyncBidiStreamingCall(
ClientCall<ReqT, RespT> call, StreamObserver<RespT> responseObserver) {
Expand Down
58 changes: 58 additions & 0 deletions stub/src/main/java/io/grpc/stub/ServerCalls.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import io.grpc.ServerCall;
import io.grpc.ServerCallHandler;
import io.grpc.Status;
import io.grpc.StatusException;
import io.grpc.StatusRuntimeException;

/**
* Utility functions for adapting {@link ServerCallHandler}s to application service implementation,
Expand All @@ -45,6 +47,14 @@ private ServerCalls() {
* Creates a {@link ServerCallHandler} for a unary call method of the service.
*
* @param method an adaptor to the actual method on the service implementation.
* <p>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You shouldn't have paragraphs after @param. You shouldn't use </p> (javadoc uses HTML style, not XHTML). Having an h3 in a p doesn't make sense.

* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be sent
* and UNKNOWN status code otherwise. Its description will be encoded to the stream trailer, but
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"stream trailer" is weird and not going to mean much to users. And END_STREAM is pulling in transport details that we shouldn't have in our API (our API does not generally assume the HTTP/2 transport).

Consider something closer to this:

Calling {@code responseObserver}'s {@link StreamObserver#onCompleted} or {@link
StreamObserver#onError} is the end of the RPC. {@code onCompleted()} will close the RPC with
status code OK. {@code onError()} will convert the Throwable to a Status with {@link
Status#fromThrowable} and trailers with {@link Status#trailersFromThrowable}. The
{@link Status#getCause} is not sent to the client, except if done by an interceptor.
Callers generally create a Throwable with {@link Status#asException()},
{@link Status#asException(Metadata)}, {@link Status#asRuntimeException()}, or
{@link Status#asRuntimeException(Metadata)}.

* the cause (which may contain server application's information) will not. After the stream
* trailer with END_STREAM is sent, the server side call is considered to be closed.
* </p>
*/
public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncUnaryCall(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This adapter doesn't look much like the server stub API. This documentation seems like it'd be better on UnaryMethod, as then you could say "responseObserver" and talk about that specific instance.

UnaryMethod<ReqT, RespT> method) {
Expand All @@ -55,6 +65,22 @@ public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncUnaryCall(
* Creates a {@link ServerCallHandler} for a server streaming method of the service.
*
* @param method an adaptor to the actual method on the service implementation.
* <p>
* <h3>Client errors</h3>
* The server's request stream observer will receive an
* onError callback with a {@link io.grpc.StatusRuntimeException} for the cancellation with the
* message 'Client cancelled', and exception cause set to null because the actual exception
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, we don't want to specify what message will be used. The important part is the status is CANCELLED. And in general, it is probably best for users to use Status.fromThrowable() to get the Status, as then they can just assume it is a Status as opposed to instanceof checks, casts, and some fall-back behavior if it isn't the expected type. On server-side that really doesn't matter because the error never has much information, but matters more on client-side. So it may be more "the Throwable, when converted to a status with Status.fromThrowable(), always has the status code CANCELLED."

* passed by the client to onError is never actually transmitted to the server and the server just
* receives a RST_STREAM frame indicating cancellation by the client.
* </p>
* <p>
* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be sent
* and UNKNOWN status code otherwise. Its description will be encoded to the stream trailer, but
* the cause (which may contain server application's information) will not. After the stream
* trailer with END_STREAM is sent, the server side call is considered to be closed.
* </p>
*/
public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncServerStreamingCall(
ServerStreamingMethod<ReqT, RespT> method) {
Expand All @@ -65,6 +91,22 @@ public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncServerStreamingC
* Creates a {@link ServerCallHandler} for a client streaming method of the service.
*
* @param method an adaptor to the actual method on the service implementation.
* <p>
* <h3>Client errors</h3>
* The server's request stream observer will receive an
* onError callback with a {@link io.grpc.StatusRuntimeException} for the cancellation with the
* message 'Client cancelled', and exception cause set to null because the actual exception
* passed by the client to onError is never actually transmitted to the server and the server just
* receives a RST_STREAM frame indicating cancellation by the client.
* </p>
* <p>
* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be sent
* and UNKNOWN status code otherwise. Its description will be encoded to the stream trailer, but
* the cause (which may contain server application's information) will not. After the stream
* trailer with END_STREAM is sent, the server side call is considered to be closed.
* </p>
*/
public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncClientStreamingCall(
ClientStreamingMethod<ReqT, RespT> method) {
Expand All @@ -75,6 +117,22 @@ public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncClientStreamingC
* Creates a {@link ServerCallHandler} for a bidi streaming method of the service.
*
* @param method an adaptor to the actual method on the service implementation.
* <p>
* <h3>Client errors</h3>
* The server's request stream observer will receive an
* onError callback with a {@link io.grpc.StatusRuntimeException} for the cancellation with the
* message 'Client cancelled', and exception cause set to null because the actual exception
* passed by the client to onError is never actually transmitted to the server and the server just
* receives a RST_STREAM frame indicating cancellation by the client.
* </p>
* <p>
* <h3>Server errors</h3>
* If the throwable sent to the server's outbound {@link StreamObserver}'s onError
* is a {@link StatusException} or {@link StatusRuntimeException}, that status code will be sent
* and UNKNOWN status code otherwise. Its description will be encoded to the stream trailer, but
* the cause (which may contain server application's information) will not. After the stream
* trailer with END_STREAM is sent, the server side call is considered to be closed.
* </p>
*/
public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncBidiStreamingCall(
BidiStreamingMethod<ReqT, RespT> method) {
Expand Down
Loading