gRPC Runtime throws Scala NotImplementedError when handling trailers-only response

[jlawrienyt]

Thanks for your help improving the project!

Getting Help

Github issues are for bug reports and feature requests. For questions about
Linkerd, how to use it, or debugging assistance, start by
asking a question in the forums or join us on
Slack.

Full details at CONTRIBUTING.md.

Filing a Linkerd issue

Issue Type:

• Bug report
• Feature request

What happened:

When receiving a trailers-only gRPC response with a Non-OK gRPC status for a unary to unary or stream to unary RPC, a failed Future is returned containing a Scala NotImplementedError.

What you expected to happen:

I expected to receive a failed Future containing a Non-Ok GrpcStatus instance.

How to reproduce it (as minimally and precisely as possible):

Connect to a gRPC service that can reliably produce a Non-OK, trailers-only (i.e. a single Headers frame with EOS set to true) response; observe the Scala NotImplementedError.

Anything else we need to know?:

I've tracked down the issue to an interaction between the H2 Netty4StreamTransport, where it sets an empty Stream when it receives as its first Frame in the H2 Stream a headers with EOS set to true; and the gRPC Codec which always attempts to read from the stream without first checking whether or not it is empty. The NotImplementedError is present the read method implementation for empty Streams.

As a temporary workaround, I'm intercepting h2.Response instances before they are handled by the gRPC Codec and converting them from Stream.empty to a Stream with a single Trailers Frame created from Response.headers.

Environment:

• linkerd/namerd version, config files: I have tested up to the latest Linkerd version using just the grpc-runtime and finagle-h2 dependencies. Note I'm observing this in an application that does not use the Linkerd service mesh, rather is a Finagle application using the grpc-runtime for connecting directly to a gRPC service. I looked through the code and didn't find anywhere outside of grpc-runtime where this is being handled so I think it would affect any proxying of gRPC services as well, but can't be sure.
• Platform, version, and config files (Kubernetes, DC/OS, etc): N/A
• Cloud provider or hardware configuration: N/A

[cpretzer]

thanks @jlawrienyt for this detailed description. We'll start looking in to this

[adleong]

Thanks @jlawrienyt! Nice find, and good work digging into the code. I think you assessment is totally right. The way I would think about fixing this would be to make a change in the acceptUnary method of the client dispatcher: https://github.com/linkerd/linkerd/blob/master/grpc/runtime/src/main/scala/io/buoyant/grpc/runtime/ClientDispatcher.scala#L48

Here we have the HTTP response and we can check if it's a trailers-only message (i.e. the stream is empty). If so, we can return the gRPC status as an exception.

[adleong]

@jlawrienyt is this a fix you'd be interested in submitting? Happy to provide further guidance if that would be helpful.

[jlawrienyt]

is this a fix you'd be interested in submitting? Happy to provide further guidance if that would be helpful.

@adleong yes, I should have something up by end of day.

[adleong]

Fixed by #2379