Hide implementation details on Client Error handling

[ignasi35]

Generated clients expose implementation details such as io.grpc.StatusException or related model (Status, Code).

We should replace all error handling and status model with akka-grpc specific classes.

[ignasi35]

about status codes. See:

• https://developers.google.com/maps-booking/reference/grpc-api/status_codes and https://github.com/grpc/grpc-java/blob/e60a179772f29554b69b2f8d7678dc18979fe7fc/core/src/main/java/io/grpc/Status.java#L69-L68 (or https://godoc.org/google.golang.org/grpc/codes)
• https://github.com/grpc/grpc/blob/master/doc/statuscodes.md and
• https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md

[ignasi35]

Related to https://github.com/lightbend/akka-grpc/issues/76#issuecomment-372709790

[patriknw]

why is it important to hide io.grpc.*? if using akka-grpc on client or server side grpc-core will anyway be a dependency, or have I misunderstood that part?

[raboof]

I don't think we need to use it - AFAIK the server side only uses io.grpc.Status and that's just for convenience. While it might not be a big deal (as it doesn't seem to have any dependencies), it does contain a bit of infrastructure we aren't actually interested in, so if we can be independent of it at low cost that might be nice.

[patriknw]

fair enough, if it's only the status code we can have our own classes for that mapping. I'd anyway suggest that we try to keep the api very similar to the "standard" grpc-core classes so that developers feel familiar if they already use grpc-core

[patriknw]

Another thing to consider is CallOptions that was found by @2m when trying out some Google cloud api. It's used there for authentication things. Perhaps it could be good to use the CallOptions from grpc-core to simplify integration with such other libraries?

[2m]

CallOptions are used to set credentials which is then used to add authentication headers which are required to use Google Cloud services, for example.

CallOptions.DEFAULT
  .withCallCredentials(grpc-core/io.grpc.CallCredentials)

And it is possible to get grpc-core/io.grpc.CallCredentials from Google Cloud provided auth json files by using "com.google.auth" % "google-auth-library-oauth2-http" % "0.9.0" library. And that one can be added as a dependency by the user of akka-grpc.

So exposing CallOptions is a nice integration point from an auth point of view.

[ignasi35]

I think the problem with CallOptions is that it's an all-or-nothing object that configures a bunch of stuff:

    deadline = other.deadline;
    authority = other.authority;
    credentials = other.credentials;
    executor = other.executor;
    compressorName = other.compressorName;
    customOptions = other.customOptions;
    waitForReady = other.waitForReady;
    maxInboundMessageSize = other.maxInboundMessageSize;
    maxOutboundMessageSize = other.maxOutboundMessageSize;
    streamTracerFactories = other.streamTracerFactories;

which potentially will force us to implement stuff like io.grpc.StreamTracer or io.grpc.Deadline.

The cut point could be CallCredentials instead of CallOptions.

---

After reviewing the code in the Auth @2m's PubSub example I think the Google Auth library depends on:

import io.grpc.Attributes;
import io.grpc.CallCredentials;
import io.grpc.Metadata;
import io.grpc.MethodDescriptor;
import io.grpc.Status;
import io.grpc.StatusException;

I guess if we want to support arbitrary 3rd party libraries like grpc-auth then we're accepting we're married to grpc-core and then the akka impl will need to integrate to that.

[raboof]

Yes, I think that com.google.auth library is a strong indicator it makes sense to have support for some grpc-core concepts built-in.

I agree CallOptions seems to contain a lot of fields that we might be unlikely to implement. Perhaps it would make more sense to use our own (more lean) settings object that accepts the things we do support, such as CallCredentials

[ignasi35]

There are three levels of commitment here:

• Status/StatusRuntimeException/StatusException (*): this is small enough that we could trivially do ourselves. If we don't do it ourselves we expose users to a non-scala-idiomatic API or depending on Metadata
• CallOptions: See my comment above. Total marriage. This could make sense if one of our goals is to make it possible for current users of the netty-based gRPC client to move to the Akka-based client.
• CallCredentials: this one sits in the middle and, while it may add some restrictions on our implementation to ensure 3rd party Auth implementations work correctly, we don't expose so much grpc-core to akka-grpc users.

There are several small quirks on grpc-core that as a scala developer I'd be very annoyed by: how stuff is constructed, mismatch on timeout types, etc...

Alpakka exposes impl details of a connector to the connector user. Akka HTTP is a complete impl of an HTTP server and client. The main concern I have with akka-grpc is keeping a clear list of what's OK and what is not OK to depend on from grpc-core. It'd be good if we had a clear idea so the PR reviews are consistent.

(*) yes, there is both StatusRuntimeException/StatusException

[patriknw]

non-scala-idiomatic API

Status.fromCodeValue isn't too bad. What is the problem? That it's not called apply? Let's be somewhat pragmatic.

[bjaglin]

During development, we just ran into the Status asymmetry between the server (throwing the akka-grpc-wrapped exception) and the client (exposing the grpc-java one), and it does look confusing when the same person produces and consumes a service with akka-grpc.

[raboof]

Replacing GrpcServiceException with the grpc-core StatusException sounds reasonable to me, unless we encounter any unintended consequences when implementing it. Should we make a separate issue out of this, or is that the last thing that needs to happen before we can close this issue?

[johanandren]

Noted this as well, I think we should add support for io.grpc.StatusRuntimeException in the server exception handling, and deprecate the internal lookalike.