timostamm / protobuf-ts

Protobuf and RPC for TypeScript
Apache License 2.0
1.1k stars 129 forks source link

Missing headers on RPCError #628

Open SalimZayCKA opened 9 months ago

SalimZayCKA commented 9 months ago

As part of our application we want to send a x-request-id header so that when there is an error or failure, we would be able to help them with tracing.

We receive the x-request-id in the response header on unary grpc calls. However, on RPCError, we do not have access to the x-request-id or any headers.

Upon investigating grpc-web-transport, it only returns the trailers in the meta field:

https://github.com/timostamm/protobuf-ts/blob/bbda0c30ecc8e826c07a41105e93e05dbb9638b2/packages/grpcweb-transport/src/grpc-web-transport.ts#L265

Is this an intended behaviour? We expect to receive headers

Thank you!

timostamm commented 9 months ago

@SalimZayCKA, I think this might be a shortcoming of the gRPC-web transport. Most other gRPC clients merge headers and trailers before providing them as "metadata" in an error.

I think you might be able to include headers with an interceptor. It would be nice to fix this - getting good confidence that a change works correctly and does not introduce regressions is not trivial though, since I don't think we have very good test coverage.

jcready commented 9 months ago

Here's my attempt at the proposed interceptor:

import { RpcError, RpcInterceptor, RpcMetadata } from '@protobuf-ts/runtime-rpc';

const noop = () => {};

function combineRpcMetadata(a: RpcMetadata, b: RpcMetadata): RpcMetadata {
    const out = structuredClone(a);
    for (const [key, value] of Object.entries(b)) {
        const bIsString = typeof value === 'string';
        if (out[key]) {
            out[key] = typeof out[key] === 'string' ? out[key] : [out[key]];
            if (bIsString) {
                out[key].push(value)
            } else {
                out[key].push(...value);
            }
        } else {
            out[key] = bIsString ? value : [...value];
        }
    }
    return out;
}

export const metaInterceptor: RpcInterceptor = {
    interceptUnary(next, method, input, options) {
        const res = next(method, input, options);
        let maybeHeaders: RpcMetadata | undefined;
        res.headers.then((h) => {
            maybeHeaders = h;
        }, noop);
        res.then(noop, (e) => {
            if (e instanceof RpcError && maybeHeaders && e.meta !== maybeHeaders) {
                // Mutate the RpcError's meta field, trailers are appended to headers
                e.meta = combineRpcMetadata(maybeHeaders, e.meta);
            }
        });
        return res;
    },
};
dawsonc623 commented 8 months ago

The latter part of this discussion might shed some light on this: https://github.com/grpc/grpc-web/issues/736

jcready commented 8 months ago

@dawsonc623 that discussion seems a bit unrelated to this issue. This issue is asking for the RpcMetadata received in the response headers to be included in the RpcError (which currently only provides the RpcMetadata from the trailers). The issue you linked to is about the "Rich Error Model" which relies on decoding a google.rpc.Status message from the grpc-status-details-bin value in RpcMetadata.

If you want to get a decoded google.rpc.Status it's pretty simple to create your own function for getting it from RpcMetadata:

import { base64decode } from '@protobuf-ts/runtime';
import { RpcMetadata } from '@protobuf-ts/runtime-rpc';
import { Status } from 'google/rpc/status';

export function richStatusFromRpcMetadata(meta: RpcMetadata): Status | undefined {
    const ref = meta['grpc-status-details-bin'];
    if (!ref) return;
    const b64StatusBin = typeof ref === 'string' ? ref : ref[ref.length - 1];
    return Status.fromBinary(base64decode(b64StatusBin));
}

And assuming you're using the "standard error payloads" you can even get their JSON representation from the google.protobuf.Any values from the google.rpc.Status's details:

import { JsonValue, JsonWriteOptions } from '@protobuf-ts/runtime';
import { Any } from 'google/protobuf/any';
import * as standardErrorDetails from 'google/rpc/error_details';
import { Status } from 'google/rpc/status';

const standardTypeRegistry = Object.values(standardErrorDetails) as unknown as NonNullable<JsonWriteOptions['typeRegistry']>;

export function errorDetailsFromStatus(s: Status, options?: JsonWriteOptions): JsonValue[] {
    const opts: JsonWriteOptions = { typeRegistry: standardTypeRegistry, ...(options ?? null) };
    return s.details.map((any) => Any.toJson(any, opts));
}
dawsonc623 commented 8 months ago

Maybe I am using the library wrong, but in the case of an error I get a rejection with an RPCError that does not seem to contain this metadata. Perhaps I have misunderstood how to get the metadata in the case of a failure?

jcready commented 8 months ago

@dawsonc623 are you sure the service is sending grpc-status-details-bin in its response (headers or trailers)? You could try to use the interceptor I posted above which should combine the headers and trailers into the RpcError's meta property. If you still aren't seeing grpc-status-details-bin in the meta that would indicate that the service isn't sending it.

You could also open the browser network inspector and see if the response headers include it.

dawsonc623 commented 8 months ago

The header in question is in the response in my network inspector. However, neither the headers on the failed call nor the meta on the error include the Grpc-X headers (three of them come back on the response per the network inspector) at all. I do get the Content-Length header of 0 in meta, which I imagine makes sense in the error case. But it seems like not all of that data makes it into the error object, which I (potentially mistakenly) attributed to the limitation in the linked issue (AFAIK grpc-web is used as the transport level in my code since I use GrpcWebFetchTransport).

EDIT Apologies, I mixed up calls and was not testing the code that was using the interceptor (I added it to my sign-up routine but was testing my log-in routine). I will be trying again on the right routine shortly and will report back.

dawsonc623 commented 8 months ago

Follow-up: Even with the interceptor (and calling the right client using it) I get the same result as above (perhaps I did test the correct flow and thought I made a mistake). I also added debug logging to the interceptor and confirmed that data never appears in the interceptor's maybeHeaders.

vdupinbetclic commented 2 months ago

@dawsonc623 Maybe it's a back-end CORS problem (Access-Control-Expose-Headers). Access-Control-Expose-Headers: grpc-status, grpc-message