doc: Document error handling and Observer callback behavior #11876
Conversation
|
I have deliberately only documented the async versions of the calls, not the blocking and futures versions, for now. Will copy the contents over when approved for the async version. |
| * 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> |
There was a problem hiding this comment.
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.
| * trailer with END_STREAM is sent, the server side call is considered to be closed. | ||
| * </p> | ||
| */ | ||
| public static <ReqT, RespT> ServerCallHandler<ReqT, RespT> asyncUnaryCall( |
There was a problem hiding this comment.
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.
| * <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 |
There was a problem hiding this comment.
"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)}.
| * <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 |
There was a problem hiding this comment.
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."
| * {@code beforeStart()} will be called. | ||
| * | ||
| * <h3>Server errors</h3> | ||
| * If the throwable sent to the server's outbound {@link StreamObserver}'s onError |
There was a problem hiding this comment.
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}.
2 participants
Fixed #7558.