(java) Correct use of onError on the server

[Lorenzo]

Hi,

I am having problems with error reporting while implementing a java grpc server with the beta API.

Whenever an exception is thrown I call the StreamObserver.onError method which correctly makes the clients raise an Exception as well, but I lose any kind of information on the thrown exception.

I have modified the HelloWorldServer java example in the following way for illustration purposes:

@Override
public void sayHello(Helloworld.HelloRequest req, StreamObserver<Helloworld.HelloReply> responseObserver) {
      responseObserver.onError(new Exception("Goodbye world"));

}

Is there something wrong with the way I am handling errors server side? What is the proper way to handle errors?

For reference, here are the stack traces of both the python and java client, where you can see the Exception message is nowhere to be seen.

Java:

Oct 22, 2015 3:24:54 PM com.example.HelloWorldClient greet

INFO: Will try to greet world ...

Oct 22, 2015 3:24:54 PM com.example.HelloWorldClient greet

WARNING: RPC failed

io.grpc.StatusRuntimeException: UNKNOWN

        at io.grpc.Status.asRuntimeException(Status.java:430)

        at io.grpc.stub.ClientCalls.getUnchecked(ClientCalls.java:156)

        at io.grpc.stub.ClientCalls.blockingUnaryCall(ClientCalls.java:106)

        at com.example.helloworld.GreeterGrpc$GreeterBlockingStub.sayHello(GreeterGrpc.java:109)

        at com.example.HelloWorldClient.greet(HelloWorldClient.java:69)

        at com.example.HelloWorldClient.main(HelloWorldClient.java:89)

Python:

Traceback (most recent call last):

  File "./.bootstrap/_pex/pex.py", line 271, in execute

  File "./.bootstrap/_pex/pex.py", line 320, in execute_entry

  File "./.bootstrap/_pex/pex.py", line 337, in execute_module

  File "/usr/lib/python2.7/runpy.py", line 180, in run_module

    fname, loader, pkg_name)

  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code

    exec code in run_globals

  File "./pex_main.py", line 72, in <module>

  File "/usr/lib/python2.7/runpy.py", line 180, in run_module

    fname, loader, pkg_name)

  File "/usr/lib/python2.7/runpy.py", line 72, in _run_code

    exec code in run_globals

  File "/home/lorenzo/Projects/core/plz-out/bin/experimental/lorenzo/grpc_test/src/main/python/com/example/helloworld_client_bin.pex/experimental/lorenzo/grpc_test/src/main/python/com/example/helloworld_client.py", line 11, in <module>

  File "/home/lorenzo/Projects/core/plz-out/gen/third_party/python/grpc/framework/crust/implementations.py", line 73, in __call__

    protocol_options, metadata, request)

  File "/home/lorenzo/Projects/core/plz-out/gen/third_party/python/grpc/framework/crust/_calls.py", line 109, in blocking_unary_unary

    return next(rendezvous)

  File "/home/lorenzo/Projects/core/plz-out/gen/third_party/python/grpc/framework/crust/_control.py", line 412, in next

    raise self._termination.abortion_error

third_party.python.grpc.framework.interfaces.face.face.RemoteError

[Eric Anderson]

On Thu, Oct 22, 2015 at 7:33 AM, Lorenzo <lor...@thoughtmachine.net> wrote:

Hi,

I am having problems with error reporting while implementing a java grpc server with the beta API.

Whenever an exception is thrown I call the StreamObserver.onError method which correctly makes the clients raise an Exception as well, but I lose any kind of information on the thrown exception.

I have modified the HelloWorldServer java example in the following way for illustration purposes:

@Override
public void sayHello(Helloworld.HelloRequest req, StreamObserver<Helloworld.HelloReply> responseObserver) {
      responseObserver.onError(new Exception("Goodbye world"));

}

Use a StatusException or StatusRuntimeException instead; these communicate clear messages to the remote side. Normal way of doing that is something like Status.INVALID_ARGUMENT.withDescription("Invalid greeting").asException().

[Lorenzo Lucherini]

Oh I see, thank you very much.

While this does work very well with the java client, it doesn't on the python side, where the abortion_error is still raised without a Status nor a description.

[Nathaniel Manista]

On Thu, Oct 22, 2015 at 9:46 AM, Lorenzo Lucherini <lor...@thoughtmachine.net> wrote:

it doesn't on the python side, where the abortion_error is still raised without a Status nor a description.

Do you not see "code" and "details" attributes on the Python exception?

-N

[steven...@ascend.io]

So checking if I have the pattern correct here:

• We get errors back to clients by calling onError.
  
• If we give onError a StatusException or a StatusRuntimeException (i.e., Status*Exception) it will use it.
  
• If we are going to explicitly give one of those, there are helpers/builders to create them.
  
• If the provided Throwable is not a Status*Exception, the Throwable will become the cause of a Status*Exception where the type is determined by the inheritance of the Throwable provided
  
• If a Status*Exception is not provided, the Status of the delivered throwable will be UNKNOWN

I'm trying to figure out what the best practice is for delivering a domain-specific error to a client and how the client should extract it.

I'm not sure what the best status is. AFAICT, UNKNOWN might be best (and I don't set it.) I'm doing a type-match on the cause anyway, so the Status doesn't seem to matter in this case, i.e., there's no ambiguity.

I'm pondering if there should be an explicit application-level Status. Or, alternatively and somewhat heretically, if I should just pass those back in-band, e.g., a Haskell Either-like object passed to onNext. That seems like going too far/out of style/a PITA. I'm just trying to keep application and infrastructure errors completely distinct (maybe a losing battle but ...)

[Justin Rudd]

I went down this route for a bit, and I noticed that I started justifying "This error is reasonably close to this Status", and it just became a headache. So I bit the bullet and started using "one_of" in my responses (i.e. Either) where it made sense. In my mind, I use Status for simple errors, and I use my custom for domain specific (i.e. cycle detected in content graph which I once justified as PRECONDITION_FAILED).

I can't seem to find the issue in the Github repo anymore, but I seem to recall one about making Status extensible.

--
You received this message because you are subscribed to the Google Groups "grpc.io" group.
To unsubscribe from this group and stop receiving emails from it, send an email to grpc-io+u...@googlegroups.com.
To post to this group, send email to grp...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/grpc-io/bac408ac-c848-4af4-bf59-e3b12ac38f28%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Eric Anderson]

On Thu, Apr 21, 2016 at 12:39 PM, Justin Rudd <jus...@mailchimp.com> wrote:

I went down this route for a bit, and I noticed that I started justifying "This error is reasonably close to this Status", and it just became a headache. So I bit the bullet and started using "one_of" in my responses (i.e. Either) where it made sense. In my mind, I use Status for simple errors, and I use my custom for domain specific (i.e. cycle detected in content graph which I once justified as PRECONDITION_FAILED).

The recommended solution for application-specific errors is to respond with a failure Status, but include application-specific Metadata with additional details. This is a bit painful today (like, use a ThreadLocal+interceptor). Issue 681 should make it a lot easier.

I can't seem to find the issue in the Github repo anymore, but I seem to recall one about making Status extensible.

https://github.com/grpc/grpc-java/issues/681

On Thu, Apr 21, 2016 at 2:56 PM <steven...@ascend.io> wrote:

So checking if I have the pattern correct here:

• We get errors back to clients by calling onError.
  
• If we give onError a StatusException or a StatusRuntimeException (i.e., Status*Exception) it will use it.
  
• If we are going to explicitly give one of those, there are helpers/builders to create them.
  

Yes to the above.

• If the provided Throwable is not a Status*Exception, the Throwable will become the cause of a Status*Exception where the type is determined by the inheritance of the Throwable provided
  

Not quite. The Throwable will become the cause of the Status. No Status*Exception will be used here, as internally we deal only with Status.

The type of Status*Exception is not based on the cause, but instead whether you call asException() or asRuntimeException(). I don't think we are checking the cause anywhere to change what type is used.

The important thing to realize in this case is that the Throwable is effectively lost; noone will see it, because grpc won't send it to the client.

• If a Status*Exception is not provided, the Status of the delivered throwable will be UNKNOWN

Yes, except that we will deliver a Status, not actually a Status*Exception. The Status*Exception is an artifact of the stubs; ClientCall and ServerCall deal directly with Status.

I'm trying to figure out what the best practice is for delivering a domain-specific error to a client and how the client should extract it.

I'm not sure what the best status is. AFAICT, UNKNOWN might be best (and I don't set it.) I'm doing a type-match on the cause anyway, so the Status doesn't seem to matter in this case, i.e., there's no ambiguity.

Common errors should use a non-UNKNOWN code and a message. UNKNOWN can be used for random, unpredicted errors.

I'm not sure what you are referring to with "no ambiguity".