gRPC-Java: exception message

[duarte...@gmail.com]

Hi,

Having a StreamObserver in the server, if we do:

observer.onError(new IllegalStateException("my message"));

we get in the client a StatusRuntimeException with message "UNKNOWN" and code also set to UNKNOWN.

I think would be useful to send back at least the message of the exception. The problem is that unless we pass a StatusException or StatusRuntimeException, the message is lost.

Right now, I achieve that with this workaround:

    } catch (Exception e) {
      response.onError(new StatusException(Status.INTERNAL.withDescription(e.getMessage()).withCause(e)));
    }

In this case, we get in the client a StatusRuntimeException with message "INTERNAL: my message" and code INTERNAL. Note that the code got appended to the message, which is useless because it is made available with getCode().

So I have two suggestions:

1. OnError with unknown exceptions, create an UNKNOWN status, using the exception's message as description (if there is one).
2. When creating an Exception from a Status, If there is a description in the Status do not append the code to the Exception's message. This is redundant information as StatusRuntimeException exposes a getter to obtain the code and it pollutes the original message. It is difficult or impossible to translate all unchecked exceptions to specific codes in a typical java application, resulting in all error messages being "UNKNOWN: ....", unless the client parses it out.

I've made the proposed changes here, in case it's not clear what I mean:

https://github.com/grpc/grpc-java/compare/master...dbmeneses:exception

[Eric Anderson]

On Fri, Apr 22, 2016 at 6:52 AM, <duarte...@gmail.com> wrote:

Having a StreamObserver in the server, if we do:

observer.onError(new IllegalStateException("my message"));

we get in the client a StatusRuntimeException with message "UNKNOWN" and code also set to UNKNOWN.

The message in the exception is UNKNOWN because we can't assume users of the exception are aware of gRPC. That is an artifact of Status*Exception, not what Status is sent to the client. If you look at Status.getDescription(), you will see it is null. I just tried it in the route guide example and got "Status{code=UNKNOWN, description=null, cause=null}". If you are gRPC-aware, interact with Status.getDescription() instead of Throwable.getMessage().

I think would be useful to send back at least the message of the exception. The problem is that unless we pass a StatusException or StatusRuntimeException, the message is lost.

Unfortunately, we don't think it is safe to send back arbitrary exception text, as the client is commonly not trusted.

Right now, I achieve that with this workaround:

    } catch (Exception e) {
      response.onError(new StatusException(Status.INTERNAL.withDescription(e.getMessage()).withCause(e)));
    }

You can create a ServerInterceptor that can copy any cause message to the status.

public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(

      MethodDescriptor<ReqT, RespT> method,

      ServerCall<RespT> call,

      Metadata headers,

      ServerCallHandler<ReqT, RespT> next) {

  return new SimpleForwardingServerCall(

      next.startCall(method, call, headers)) {

    @Override

    public void close(Status status, String message) {

      if (status.getDescription() == null

          && status.getCause() != null) {

        status = status.withDescription(

            status.getCause().getMessage());

      }

      super.close(status, message):

    }

  };

}

[Eric Anderson]

On Fri, Apr 22, 2016 at 7:30 AM, Eric Anderson <ej...@google.com> wrote:

    public void close(Status status, String message) {

Typo: Instead of "String message" that should be "Metadata trailers".

[duarte....@sonarsource.com]

Thanks for the quick response and the insights.

[Smallufo Huang]

Eric Anderson於 2016年4月22日星期五 UTC+8下午10時30分29秒寫道：

You can create a ServerInterceptor that can copy any cause message to the status.

public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(

      MethodDescriptor<ReqT, RespT> method,

      ServerCall<RespT> call,

      Metadata headers,

      ServerCallHandler<ReqT, RespT> next) {

  return new SimpleForwardingServerCall(

      next.startCall(method, call, headers)) {

    @Override

    public void close(Status status, String message) {

      if (status.getDescription() == null

          && status.getCause() != null) {

        status = status.withDescription(

            status.getCause().getMessage());

      }

      super.close(status, message):

    }

  };

}

Hi , it seems the signature changed in 1.0.0
Is there a 1.0.0 version of ServerInterceptor  (that translate the underlaying Exception to String )

private ServerInterceptor serverInterceptor = new ServerInterceptor() {
  @Override

  public <ReqT, RespT> ServerCall.Listener<ReqT> interceptCall(

    ServerCall<ReqT, RespT> serverCall,
    Metadata metadata,
    ServerCallHandler<ReqT, RespT> next) {
    logger.info("intercept {}" , serverCall.getMethodDescriptor().getFullMethodName());

    // how to replace the following line ?
    return next.startCall(serverCall, metadata);
  }
};

How to replace the next.startCall() line ?

Thanks a lot ! 

[Smallufo Huang]

Hi , 

I wonder how to implement a  'global runtime exception interceptor' , 

that intercepts all server runtime exceptions , and auto 

responseObserver.onError(new StatusRuntimeException(Status.INTERNAL.withDescription(e.getMessage())));

so that in the server code, we don't need to try catch RuntimeException , and write the above code in each method call.

Thanks a lot.

Smallufo Huang於 2016年9月30日星期五 UTC+8下午8時21分54秒寫道：