Convert DecodingErrors thrown from request handling code to HTTP 400 …

[colindignazio]

Motivation

The library does not gracefully handle when clients send requests with malformed request bodies, parameters, etc. If a client sends a request that for example is missing a required field or has a field that cannot be converted to the correct type, the server should respond with a 400 indicating to the client that they have done something wrong. Instead, the library throws a DecodingError which propagates all the way up resulting in a 500 response which incorrectly tells the client that something went wrong on the server.

Modifications

• Added a layer of handling to each call that is made to decode request objects which wraps DecodingErrors into RuntimeErrors.
• Make ServerError conform to HTTPResponseConvertible
• Modify ErrorHandlingMiddleware to first check if the underlying error conforms to HTTPResponseConvertible and if not use the values from ServerError. This allows the HTTPResponseConvertible values set in a RuntimeError to be honoured after the RuntimeError is transformed into a ServerError.

Result

Since RuntimeError conforms to HTTPResponseConvertible these errors will get converted to 400 responses by the ErrorHandlingMiddleware. This isn't a perfect solution because consumers of the library have to opt-in to the ErrorHandlingMiddleware to avoid returning 500.

Test Plan

• Added unit tests for each modification.
• Verified in my own service that with this change malformed requests get converted into 400 responses.

[czechboy0]

These two will need to be optional and default to nil to avoid an API break.

[colindignazio]

Ah good catch thank you

[czechboy0]

Suggested change

	httpBody: OpenAPIRuntime.HTTPBody?
	httpBody: OpenAPIRuntime.HTTPBody? = nil

[czechboy0]

Technically, adding parameters to an existing public initializer is an API break as well, so can you add a second initializer with all the properties, and call from the existing unchanged one to the new one?

[colindignazio]

Instead of making these new properties nullable, how about setting them with sensible default values from the existing initializer? That way we can leave ServerError conforming to HTTPResponseConvertible and we don't have to deal with unwrapping them down the line.

[colindignazio]

@czechboy0 Thoughts?