Non-standard use of 412 HTTP Status Code

[cjcormack]

Currently a gRPC FAILED_PRECONDITION error gets translated to a HTTP status code of 412 Precondition Failed . However, this is semantically incorrect.

The RFC for the HTTP status code (https://tools.ietf.org/html/rfc7232#section-4.2):

The 412 (Precondition Failed) status code indicates that one or more conditions given in the request header fields evaluated to false when tested on the server. This response code allows the client to place preconditions on the current resource state (its current representations and metadata) and, thus, prevent the request method from being applied if the target resource is in an unexpected state.

This does not match what the gRPC FAILED_PRECONDITION semantics (https://github.com/grpc/grpc/blob/master/doc/statuscodes.md):

The operation was rejected because the system is not in a state required for the operation's execution. For example, the directory to be deleted is non-empty, an rmdir operation is applied to a non-directory, etc.

The gRPC documentation referenced above also suggests a 400 Bad request being the closest HTTP mapping (I suspect this is where the value came from before it was changed by #657.

Having run into this this morning, I ran into a Stack Overflow response that confirmed my feelings https://stackoverflow.com/questions/10730852/http-status-code-400-vs-412.

While changing the response back to 400 Bad request would be the "correct" thing to do, I can see how that would be a breaking change for some people. I also miss the separation between the two different types of errors (there's also the possibility of a 422 response, but the new wording for 400 possibly changes my opinion—https://stackoverflow.com/questions/16133923/400-vs-422-response-to-post-of-data).

[johanbrandhorst]

Yep this is wrong. We should use the one defined in code.proto. It is a backwards-incompatible change, as you say, but I'm tempted to say that it was a bug and so a bug fix should be issued and a patch release made.

Would you like to submit this change? Any more thoughts on breaking users here @achew22 @tmc?

[johanbrandhorst]

Offending line: https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/errors.go#L40

[johanbrandhorst]

This is the code.proto source of truth we should use: https://github.com/googleapis/googleapis/blob/master/google/rpc/code.proto#L127.

[cjcormack]

Thanks for your quick response. I hadn't found the code.proto reference, that is pretty definitive!

I am happy to make the change later.

[achew22]

This is just a bug. Send in a patch and I'll merge it