Mistake in grpc error handling
JustCryingCat opened this issue · 14 comments
I use nice-grpc-web in combination with the traefik gateway server and services written in go using the grpc library.
The problem I have is that the library doesn't take any notice of the errors in the headers and won't even let the user handle them.
Violation of logic, in my opinion, in these parts of the code:
You call the user handleTrailer with grpc-status and grpc-message cut out, even though you extract them with the line above and don't pass them anywhere, even though they contain information about the error.
You deliberately do not call the user function onHeader, which does not allow the user to check errors in the header and does not handle them itself.
As I understand you put the main emphasis in error handling on trailers, but trailers is not critical information in most protocols, much more important is handling headers
What do you mean by "errors in the headers"? Are you talking about some kind of a user-defined errors? Can you share an example?
gRPC errors are reported do the user by throwing exceptions.
Sorry for the inaccuracy. I meant http headers. Official google grpc client libraries handle errors based on headers (if you trust grpc-go source code). Your code handles errors based on payload data.
This grpc error will not be handled, because the information about it is written only in http headers
And the library will show this message
Your library is very cool and it would be cool if it could handle errors from http headers
I think the problem is that the gateway converts the HTTP/2 response to HTTP/1.1 and at the same time moves the Header Frame to Http Headers to support HTTP/1.1. I tested with nginx and traefik
Sorry for the inaccuracy. I meant http headers. Official google grpc client libraries handle errors based on headers (if you trust grpc-go source code). Your code handles errors based on payload data.
This grpc error will not be handled, because the information about it is written only in http headers
And the library will show this message
Your library is very cool and it would be cool if it could handle errors from http headers
This exception appeared because I had middleware which inserted 0x80 into payload to copy Trailers which were already converted by the gateway converter http/2->http/1 and didn't exist as such.
Without this middleware we get an empty payload and the following exception:
Your library, as I said before, relies only on payload data. Please add support for error handling from Http Headers
Usually gRPC protocol sends status code in the trailer, not the header. Consider for example a streaming request: it may throw an error after sending multiple responses, and the client will correctly receive the error data.
There are cases though where the server sends status code in the header (called Trailers-Only
in gRPC Protocol), for example when the error is generated by the gRPC library before even reaching method implementation.
nice-grpc-web
handles these cases. From FetchTransport
:
nice-grpc/packages/nice-grpc-web/src/client/transports/fetch.ts
Lines 61 to 64 in e424e91
to decodeResponse
:
nice-grpc/packages/nice-grpc-web/src/client/decodeResponse.ts
Lines 36 to 37 in e424e91
to makeCall
which detects the Trailers-Only
case:
nice-grpc/packages/nice-grpc-web/src/client/makeCall.ts
Lines 93 to 99 in e424e91
The possible situation where this could be buggy is if in your setup the client receives both the Trailers-Only
(headers with grpc-status
) and the normal trailer, but empty. In this case, the CallOptions.onTrailer
callback would be called twice. Can you confirm if that's the case?
Can you please also share your traefik config? So that we could add it to our test suite.
Yes, I'm sorry, I was wrong. The grpc server does put all the headers in the trailer. But for some reason trailers remains empty when the response comes from the server. Even the middleware grpcweb (which is developed by traefik) knows that there must be something there and inserts the 0x80 symbol in the payload.
These are my traefik configuration files.
traefik.yaml
entryPoints:
web:
address: :8081
providers:
file:
directory: /etc/traefik/
experimental:
plugins:
corspreflight:
modulename: github.com/Medzoner/traefik-plugin-cors-preflight
version: v1.0.6
api: {}
provider.yaml
http:
middlewares:
corspreflight-m:
plugin:
corspreflight:
code: 200
method: OPTIONS
cors:
headers:
accessControlAllowOriginList: "*"
accessControlAllowHeaders: "*"
grpcweb:
grpcWeb:
allowOrigins:
- "*"
routers:
local-route:
rule: "Host(`localhost`)"
middlewares:
- cors
- corspreflight-m
- grpcweb
service: myservice
services:
myservice:
loadBalancer:
servers:
- url: h2c://host.docker.internal:8080
The header frame goes into the response headers because http2.Transport from the standard golang library that traefik uses for h2c request inserts it into the response headers when processing the response. Middleware grpc-web obviously does not provide this and inserts 0x80 symbol into payload indicating trailers, so your library tries to read trailers and parseTrailer function does not find necessary grpc-message and grpc-status fields in it and throws an exception. The solution is to disable grpc-web middleware and put expose headers in cors.
cors:
headers:
accessControlAllowOriginList: "*"
accessControlAllowHeaders: "*"
accessControlExposeHeaders:
- "grpc-message"
- "grpc-status"
customRequestHeaders:
content-type: "application/grpc"
I think that 0x80 will get into payload by mistake and it is worth writing to traefik developers about the conflict between grpc-web middleware and http2.Transport from the standard golang library
Also your library processes headers first, but throws them into "empty", because it sees 0x80 symbol in payload when decodeResponse is processed further and starts parseTrailer which generates exception. Perhaps this point you should consider in your library for the future and generate an error if grpc-status != 0 at the moment of primary input to handleTrailers
Also, adding an exception generation on initial processing of response headers in parseTrailers would help to describe errors like 415 in more detail, because the server sends a message which explains the error in detail
The user could receive not only the 415 error message, but also read the grpc-message
Thanks for sharing your config. I have added traefik to our test suite in #346.
I have also reproduced the issue with Trailers-Only response. The fix is in progress.
Published the fix in 📦 nice-grpc-web@3.2.4
, please try it out.
Feel free to reopen if the problem remains.