search

grpc / grpc-java

The Java gRPC implementation. HTTP/2 based RPC
https://grpc.io/docs/languages/java/
Apache License 2.0
11.36k stars 3.82k forks source link

Client does not report expected error code for 431 HTTP status #11248

Open jhump opened 3 months ago

jhump commented 3 months ago

A well-formed gRPC response always has an HTTP status code of “200 OK”. The gRPC docs specify a table for mapping other HTTP status codes to gRPC error codes: https://github.com/grpc/grpc/blob/master/doc/http-grpc-status-mapping.md.

The client in this repo mostly conforms to this table. This was tested with responses that include a variety of HTTP status codes but no body (and thus no “content-type” header) and no “grpc-status” header. The status codes tested include all values in the table linked above as well as a sampling of other codes (to test the last line of the table, which states that all other HTTP status codes should map to an unknown error code).

The one issue uncovered was for HTTP status 431 (Request Header Fields Too Large). Per the table, this HTTP status should result in an unknown error code. However it instead results in an internal error code.

ejona86 commented 3 months ago

Heh. There's a TODO saying to update the spec. https://github.com/grpc/grpc-java/blob/781b4c4575218278614948902a6873304d29b4e4/core/src/main/java/io/grpc/internal/GrpcUtil.java#L309

It was added in https://github.com/grpc/grpc-java/commit/d5eb248737a584309b21cbb4d1355bac00bed244

I suspect that was because the server-side behavior changed from a RST_STREAM to HEADERS with error. It would be fair to discuss cross-language what code is appropriate and use it instead of unknown.