Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing headers on RPCError #628

Open
SalimZayCKA opened this issue Feb 2, 2024 · 9 comments
Open

Missing headers on RPCError #628

SalimZayCKA opened this issue Feb 2, 2024 · 9 comments

Comments

@SalimZayCKA
Copy link

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:

throw new RpcError(maybeStatus.detail, maybeStatus.code, maybeTrailer);

Is this an intended behaviour? We expect to receive headers

Thank you!

@timostamm
Copy link
Owner

@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
Copy link
Contributor

jcready commented Feb 7, 2024

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
Copy link

The latter part of this discussion might shed some light on this: grpc/grpc-web#736

@jcready
Copy link
Contributor

jcready commented Mar 15, 2024

@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
Copy link

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
Copy link
Contributor

jcready commented Mar 16, 2024

@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
Copy link

dawsonc623 commented Mar 16, 2024

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
Copy link

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
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants