vmagamedov/grpclib

Access underlying HTTP details on GRPCError

JonatannQm opened this issue · 6 comments

Add HTTP details to the GRPCError exception for handling various errors and flows related to HTTP.

For example, the 30X responses (moved) cannot be handled without access to the HTTP headers to fetch the new location.

gRPC has (almost) nothing to do with HTTP semantics, it is abstracted away, so you shouldn't know about HTTP/2 or any other protocol underneath.

When you're making a gRPC request you should know in advance that endpoint on the server supports HTTP/2 protocol and can pass/handle gRPC requests. There is even no protocol negotiation/upgrade between client and server.

For reference you can check out grpcio API. They also don't share low-level details about HTTP errors.

If in case of unexpected HTTP errors grpclib shows not informative enough error messages then I'm ok to accept PR with better error messages. Currently grpclib treats all unexpected errors (not specified in gRPC protocol spec) as unrecoverable.

If your example is not imaginary and you have to deal with unknown servers, then you may use any HTTP/2-capable client to make first request and detect 30X responses. But I really can't imagine such situation.

My suggestion is not to change the behavior of grpclib regarding unexcepted errors. I understand that gRPC is and should not handle these errors.

In my opinion, adding underlying HTTP information (if available) to the error does not break this statement.

I am only suggesting including the information in the error; as you can see in the PR that I opened (#171), the headers already exist in most of the functions that create the error. It's only a matter of passing it forward.

The example I gave is not imaginary. It's a problem that I have, which is why I suggested this change.

It is currently possible to get headers using private API:

stream = None
try:
    async with greeter.SayHello.open() as stream:
        await stream.send_message(HelloRequest(name='Dr. Strange'), end=True)
        reply = await stream.recv_message()
        print(reply.message)
except GRPCError:
    if stream:
        print(stream._stream.headers)
    raise

Sorry but currently I don't want to introduce new APIs. Only if many users will benefit from them.

Do you think we can make this private API public?

If it's private, API should not be used outside the class.

xloem commented

The full error details on e.g. cloudflare errors are generally in the page body rather than the headers; how does one access the body of the failing response?

grpclib only handles gRPC errors, which are defined here: https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#errors

It doesn't handles HTTP errors like generic http clients. For grpclib they are all the same – unrecoverable transport errors.

Here is quick and dirty way to receive body:

    async with Channel('www.google.com', 443, ssl=True) as channel:
        greeter = GreeterStub(channel)
        async with greeter.SayHello.open() as stream:
            await stream.send_message(HelloRequest(name='Dr. Strange'), end=True)
            try:
                reply = await stream.recv_message()
                print(reply.message)
            except GRPCError:
                print(stream._stream.headers)
                size = int(dict(stream._stream.headers)['content-length'])
                print(await stream._stream.recv_data(size))
                raise  # this is critical to reraise original exceptions inside `.open()` context-manager