GRPC interceptors: why is StreamInterceptor needed for unary RPCs?

[uwemaurer]

We use the GRPC interceptor functionality to add authentication information to the metadata of the request.

When I add an UnaryInterceptor to the client it works fine for unary RPCs, however only when I call them with the method which returns a Promise.

If I call the same unary RPC with the methods which takes the callback as parameter, then the interceptor doesn't get executed but it would execute the StreamInterceptor.

I noticed that the generated code for the client has a seperate method in case there is a callback:

 if (callback !== undefined) {
      return this.client_.rpcCall(

otherwise it goes via

return this.client_.unaryCall(

I see in https://github.com/grpc/grpc-web/blob/master/javascript/net/grpc/web/grpcwebclientbase.js

that unaryCall (thenableCall) uses the unaryInterceptors_ but rpcCall uses the streamInterceptors_.

Why is this? I think it would be better to go through the same code path for doing a unary RPC no matter how the caller decided to invoke the RPC.

Otherwise I need to provide two interceptors for unary RPCs if the codebase uses both type of calls (callback and Promise based)

[stanley-cheung]

Why would your codebase be mixing callback-based unary calls and promise-based unary calls? Doesn't sound ideal.

But yes, you need streamInterceptors for both unary calls and server streaming calls for the callback-based clients, and unaryInterceptors for promise-based clients (promise-based clients only support unary calls anyways).

Yes the naming is slightly confusing. One of the goals for the overall interceptors design for this project is to accommodate some of the internal use cases / frameworks we need to integrate with, and also the open source use case here. So there are some challenge there with naming. But on the other hand, we did not intend user of this would be mixing callback-based clients and promise-based clients in the same codebase. Hence, we were expecting that you only need 1 type of interceptors or the other but not both.

Here is an example for StreamInterceptor. And Here is an example for UnaryInterceptor.

Hope this helps.

[ericb-summit]

I would hope that anyone who is reading this let alone working with grpc knows what a unary RPC is and what a streaming RPC is as a basic core concept. Given how fundamentally different they are, it made logical sense to have different interceptors.

But grpc-web's definition of "unary" and "stream" only when dealing with interceptors feels arbitrary and nonsensical. Worse, it isn't captured in the closest thing to docs I could find. Something that is this unexpected should be so obvious it's impossible to miss while perusing docs.

It cannot be I am the first to say: please consider renaming the interceptor fields names in GrpcWebClientBaseOptions to something that makes more sense: say, callbackInterceptors or promiseInterceptors. The change is, I believe trivial, but if you want a PR, I'll be glad to submit it.

(Yes, I've spent better part of a day on this telling myself "this is so simple I must have a mistake in one of my other 900 components")

[sampajano]

@ericb-summit Thanks for the comment :)

I do agree with the naming being confusing, like Stanley has mentioned above.

Although, we generally do not making breaking API renames, hence will keep the naming here.

The blog you mentioned above did try to capture this, though. In 2 places:

1. a callout note:

A StreamInterceptor can be applied to any RPC with a ClientReadableStream return type, whether it’s a unary or a server-streaming RPC.

2. In the "Binding interceptors" section.

But you're right it is still "possible" to miss. :)

You could consider submitting a PR to grpc.io to make this more obvious: (I'm not exactly sure how the review process work but if it's possible to change we'll happy to review it.)

https://github.com/grpc/grpc.io/blob/main/content/en/blog/grpc-web-interceptor.md

---

In any case, we'll keep in mind to add more documentations around Interceptors in this repo. Especially if this troubles more people.

Sorry for the inconveniences :)

[ericb-summit]

Thanks for the reply. I also agree about breaking changes, that is not an ideal scenario. A better idea would be to add new fields to GrpcWebClientBaseOptions. In any case, I will look at the doc above and come up with a pr once I'm at a PC again.

And no need to apologize. Like all Foss projects I am grateful for everyone's contributions. Cheers!