Allow customizing the server's default error response

[stevenj]

Version 1.6.0

Platform Linux x86-64

Description

When hyper gets a header with 0x7f in it, it generates a 400 response with no content type specified and no body.

The problem is, our OpenApi specification for our service defines a different response for 400, which matches the behavior of our services endpoints.

However, OpenAPI 3.0.x only allows you to define multiple response types for the same response code IF and ONLY IF they ALL have a content type. You can only have one response schema per distinct content type.

This breaks our CI. Our automated tests trigger hypers default 400 response. The response does not conform with our OpenAPI spec, so the breakage is 100% correct. It's not possible to define an OpenAPI spec which includes both hypers 400 response, and our designed 400 response.

The upshot is, Hyper has squatted on 400 and we can't re-define it, if we want a valid OpenAPI specification, and a service which 100% reflects it. So our choices if we want to use 400 and have a valid spec is return no information in the error, this is not possible for us. This has meant we can't use 400 at all.

Interestingly, hyper internally comments the response can be any 4xx, but the trait that would be needed to customize the Server struct is not public outside the crate, so it has proven impossible to get Hyper to have different behavior than an empty 400 with no content type.

We have spent a lot of time trying to solve this, but have we missed something and it is possible to redefine what hyper will return for 400?

[seanmonstar]

I can see why someone would need to modify those automatic responses. I'd be fine with adding a mechanism to do so. Care would be required that it not leak too many internal details, so we can refactor things at any time and not worry about breaking this public API.

Would you be interested in exploring that with those constraints?

[stevenj]

Absolutely. Although it's not clear where one could easily add this functionality to hyper.

[samp5]

Hey @seanmonstar I was starting to look at this and am thinking about the following approach:

• Add a new public trait like Http1ErrorResponder with a single method like:

 // Response<()>  here since MessageHead`is not public. Alternatively,  http::Parts could be reexported.
// This would require something like from_response for MessageHead?
fn respond(&self, cause: &Http1ErrorReason) -> Option<crate::Response<()>>;

• Map internal error::Kind to a new exposed enum Http1ErrorReason with corresponding variants (like Kind::Parse(Parse::Method) => Http1ErrorReason::InvalidMethod)
• Let the user plug in their Http1ErrorResponder to the Builder then pass that along to Connection. The custom error handling could happen in Http1Transaction::on_error.

Let me know what you think and I can submit a pull request!

[seanmonstar]

Thanks for taking a look, sounds nice! What does it end up becoming on the Connection type? Adding a generic is tricky and potentially breaking... Would it need to a boxed trait object? Would that mean that it'd need to be Box<dyn Response + Send + Sync>?

[samp5]

Yeah, in my solution hereConnection type contains a trait object. Is that performance cost acceptable since it's on the error path?

For reference, I've just finished working out this approach and will submit a pull request. The pain point seems that Http1ErrorResponder really should only be available for a Server, but needs to integrate with the Http1Transaction::on_error which is implemented for clients and servers.

I'll submit what I have which is sort of an ugly conditional compilation solution to this. Any and all feedback is appreciated!

[samp5]

@seanmonstar Just checking in on this! If things look alright, I can go ahead and write some additional tests and add those to the pull request.