DecodingError should return 4xx instead of 500

[gayathrisairam]

Motivation

If there is an error in decoding a request, a 500 is thrown currently. This should be a 4xx status code.

Proposed solution

Conform DecodingError to HTTPResponseConvertible protocol.

Alternatives considered

No response

Additional information

No response

[kdubb]

It should not. Bad Request is for requests that were unprocessed due to the request itself being malformed.

From MDN:

The HTTP 400 Bad Request client error response status code indicates that the server would not process the request due to something the server considered to be a client error. The reason for a 400 response is typically due to malformed request syntax, invalid request message framing, or deceptive request routing.

A response decoding error could be due to a client or server error and may have been processed. It doesn't fit with 400 on multiple levels.

These errors should not be given a status code at all as they do not directly relate to HTTP. If it must be done, pick an unused range (e.g., 6XX or 9XX) and map custom status codes to that range.

[kdubb]

I read this as 400 and realize that it does say 4XX. In any case, I stand by my general thesis that these are not HTTP errors and shouldn't be reported as such.

[czechboy0]

Hi @kdubb - this issue is about server request decoding (on the server). Is it possible you assumed it's about the client decoding a response?

[kdubb]

🤦‍♂️ Yes. It was most definitely about client decoding errors (I went through this discussion with a code generator I built).

Having a bit of experience with this, and more context, I would say that 500 is probably the correct code for server decoding if the requests are validated prior to decoding. If the decoding is essentially validating the request then 400 starts to look more correct but it has issues. Decoding as validation blurs the line and can lead to (as seems to be suggested here) incorrect responses because you're doing two things (decoding and validating) but mapping errors to a single type (decoding or validation).

Coincidentally, I'm currently working on a JSON Schema driven SerDes package that is a single pass validation and decoding library that can report the difference. Basically, if it doesn't validate it's a 4XX error. If it validates but fails during translation to the server's expected type it's a decoding error (aka 5XX).

Side note... The SerDes package work is actually why I started looking into this code generator and switching away from our in-house one. I really like the code structure (even if it's a bit verbose) and it allows us to preserve all the features we need (JSON/CBOR/YAML agnostic). So, you might see a bit of activity from me as I get into converting a fairly large API over as there seems to be a few things to we'll have to figure out (the one large file problem, etc.).

Anyway great job so far!