Ability to propagate cause when using StatusProto.toStatusException

[jasnross]

Is your feature request related to a problem?

When converting a com.google.rpc.Status to an io.grpc.StatusException I can't find a way to propagate a cause with the resulting StatusException.

Currently StatusProto.toStatusException is defined as:

  public static StatusException toStatusException(com.google.rpc.Status statusProto) {
    return toStatus(statusProto).asException(toMetadata(statusProto));
  }

However, using this method there doesn't seem to be any way of providing a cause.

Describe the solution you'd like

My ideal would be to make StatusProto.toStatus public, which would allow creating the status and calling asException something callers could do themselves. This would also be helpful in scenarios where callers require a Status instead of a StatusException.

  private static Status toStatus(com.google.rpc.Status statusProto) {
    Status status = Status.fromCodeValue(statusProto.getCode());
    checkArgument(status.getCode().value() == statusProto.getCode(), "invalid status code");
    return status.withDescription(statusProto.getMessage());
  }

Describe alternatives you've considered

It feels a little clunky, but I'd be open to having an extra parameter on toStatusException for the cause, or another solution depending on the preferences of grpc-java maintainers.

I've also tried calling initCause on the resulting StatusException, but that isn't possible because the cause is set to null in the asException code, so initCause throws an IllegalArgumentException.

Additional context

[ejona86]

Cause isn't sent over the wire, so the status isn't being used for an RPC?

StatusProto's API was designed to make sure io.grpc.Status and com.google.rpc.Status agree. It'd be very bad for one to say OK and the other to be PERMISSION_DENIED, for example. But it sounds like you are using com.google.rpc.Status within your process for error information. Can you explain more about how your app code is organized to use this?

[jasnross]

Thanks for the reply!

The way our application is designed is that exceptions in our gRPC services are expected to ultimately return StatusException, and eventually that StatusException is translated into a response. Just before that happens typically we log the exception (if it is one of the types we are interested in logging - for example, INTERNAL), and would like to have the cause there for logging purposes.

We recently incorporated com.google.rpc.Status into our responses in order to provide a richer payload of response data (invalid fields with BadRequest, etc.) but prior to that we were just using Status#withCause:

io.grpc.Status.INTERNAL.withDescription("Message to API caller.").withCause(someException).asException()

When we logged this we would get the stack trace from the cause, which can be useful for debugging.

For now we are working around it by building the io.grpc.Status from com.google.rpc.Status ourselves and calling withCause on it, but ideally I was thinking there could be a way to add the exception while still benefiting from the StatusProto conversion code that grpc-java provides.

[ejona86]

I see. Local logging. That's quite fair.

Worst-case adding a overload that also receives a Throwable could work. I don't think we'd have a Metadata-less overload, but maybe Metadata would be permitted to be null instead of required. But we have a few overloads, so it'd probably be good to look around to see if there's something more natural (I'm not seeing anything immediately, but I've only spent 15 seconds looking at it).

[temawi]

@ejona86 What is your thinking - is this an enhancement we should take up?

[ejona86]

I do think we should do something here. There's just the question of the API.

[ejona86]

I think the easiest/best think right now is just to add another overload (like I said in #10900 (comment)). I don't think there's going to be any cleaner API unless we introduce a builder.

Feel free to send a PR.