grpc / grpc-web

gRPC for Web Clients
https://grpc.io
Apache License 2.0
8.68k stars 765 forks source link

StreamInterceptor type doesn't support Promise intercept #988

Open someone1 opened 4 years ago

someone1 commented 4 years ago

The following won't compile but should/does work:

import {
 StreamInterceptor,
} from 'grpc-web';

const streamAuthInterceptor: StreamInterceptor<unknown, unknown> = {
  async intercept(request, invoker) {
    await authRequest(request);
    return invoker(request);
  },
};

removing the StreamInterceptor type from the variable lets it compile and it behaves as expected (adding the appropriate metadata to the request)

stanley-cheung commented 4 years ago

StreamInterceptor is not supposed to be used for the promise-based client.

This is a full example of using streamInterceptors when you are using the normal callback-based client (i.e. xxxxxClient): https://github.com/grpc/grpc-web/blob/master/packages/grpc-web/test/tsc-tests/client03.ts

This is a full example of using unaryInterceptors when you are using the promise-base client (i.e. xxxPromiseClient): https://github.com/grpc/grpc-web/blob/master/packages/grpc-web/test/tsc-tests/client04.ts

someone1 commented 4 years ago

Thanks for the reply - I guess my issue is that I grab my access tokens from a library (auth0-spa) that returns a Promise<string> so ideally the support found in the unary interceptor would also be available in the stream interceptor.

From the examples, it's not clear/obvious how I could add the authorization metadata to the stream request when relying on promise-based sources without hacking together some shim.

Is there a technical limitation/reason why supporting a promise-based interceptor for stream clients wouldn't work?

someone1 commented 4 years ago

This could be related to #942 - my service has unary calls and server stream calls (no bidi calls or client stream calls). Unlike the examples you linked to, I don't have a ServicePromiseClient and a ServiceClient - only a ServiceClient.

Setting up my client, I have to specify both the unaryInterceptors option and the streamInterceptors options (btw, the client options type could probably be updated to include these!) in order for the server-side stream calls to get the appropiate metadata:

const authRequest = async (request: Request<unknown, unknown>) => {
  const metadata = request.getMetadata();
  const accessToken = await AuthModule.accessToken();
  metadata.Authorization = `Bearer ${accessToken}`;
  metadata['x-api-key'] = AuthModule.apiKey;
};

const unaryAuthInterceptor: UnaryInterceptor<unknown, unknown> = {
  async intercept(request, invoker) {
    await authRequest(request);
    return invoker(request);
  },
};

const options = {
  unaryInterceptors: [unaryAuthInterceptor],
  // Oddly enough, I have to pass the UnaryInterceptor typed interceptor to the stream interceptors option
  streamInterceptors: [unaryAuthInterceptor], 
};

export default new InteropClient(process.env.VUE_APP_WEB_API_HOST, null, options);

Perhaps this is a naming issue more than anything else? Maybe its not stream vs unary interceptors but rather promise vs callback interceptors?

someone1 commented 4 years ago

I can confirm that I did need to implement a non-promise version of the interceptor for the streamInterceptors options - reusing the unary interceptor option created Promise returns on all stream calls which was not what the types were expecting.

Again, since the token expiration/refresh is handled from a 3rd party that relies on Promises, it'd be ideal that the interceptor supported Promise-based functions.

ThaDaVos commented 3 years ago

So, we're half an year further and it's still the case... would love a fix for this - I've got a test service and it only generates a single client and the stream interceptor still does not support promises, a pity as my way of acquiring a token is also by async calls...

Proto File:

syntax = "proto3";

message TestRequest {
   string Test = 1;
}
message TestResponse {
   string Test = 1;
}
service TestService {
   rpc Test (TestRequest) returns (TestResponse);
}
ThaDaVos commented 3 years ago

A note on the single client: When using Typescript, the Promise and Callback client are combined into one - each method has a callback and promise version

kurnal-volt commented 3 years ago

any updates on this? why hasn't this been addressed yet as it seems to be a good use case I am running into also

calclavia commented 8 months ago

Would be great if interceptors could be async