Return 400 Bad Request to REST downstreams if the request is invalid #135
Comments
|
For the first one (request should have no body), that indeed can and should be fixed. The second one is less clear thanks to the gRPC docs/specs for error-handling. If vanguard did not have to transcode the request (i.e. the request was in JSON, but the server also supported JSON), then no translation would happen and it would be the downstream handler/server that parses the request and fails. In this scenario, the downstream handler would return an "internal" error which vanguard then translates to a 500 status code. Here are the relevant parts of the docs/specs:
Unfortunately, the specs don't allow the RPC library to generate an "invalid argument" error when trying to parse a request, which is the gRPC code that would map to a "400 Bad Request" (link to spec). So vanguard currently treats this situation as a "500" for consistency -- the client should ideally observe the same behavior as if vanguard weren't necessary/in use. We could possibly make an exception specifically for mapping RPC errors for REST clients. Since REST error semantics differ from gRPC and ConnectRPC semantics, we might accommodate this in Vanguard's transcoding and special-case some errors to return the more conventional REST error code (like "400" in this particular case). |
When transcoding REST to a gRPC upstream, by default, all bad requests return a
500instead a400. The final error and status code can be rewritten in an HTTP middleware, but the error messages do not share a common prefix, so that I need to pattern-match all possible errors. For example:passing a body to an endpoint that does not expect a body (no body set in the
google.api.httpoption) returns: request should have no body; instead got %d bytespassing invalid JSON to an endpoint that expects a body returns either
proto: unexpected EOForgrpc: error unmarshalling request: proto: unexpected EOF, depending on the HTTP method (I think)There might be other cases that I missed. Ideally, these error messages would all share a common prefix (i.e.
vanguard: invalid request) so that some middleware can catch them and return a 400 ; or Vanguard would return a 400 to downstream for all decoding errors of the request by default.The text was updated successfully, but these errors were encountered: