Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A better Server Error experience #1128

Closed
seanmonstar opened this issue Apr 11, 2017 · 0 comments
Closed

A better Server Error experience #1128

seanmonstar opened this issue Apr 11, 2017 · 0 comments
Labels
A-error Area: error handling
Milestone

Comments

@seanmonstar
Copy link
Member

seanmonstar commented Apr 11, 2017

There are 4 error cases on the server side:

  • When receiving/parsing the headers of a new incoming Request
  • When receiving/parsing the body of an established Request
  • When a Service's Future resolves to an Err, meaning that it just could not respond to the Request
  • When a user-supplied Stream has an error while generating more of the outgoing body

The current design requires that all 4 of these cases be covered by hyper::Error, but that type is not satisfactory for the semantics of each case. This design is currently required because of tokio-proto's ServerProto only allowing a single Error type. Relevant tokio issue: tokio-rs/tokio-proto#157

Another quirk with the current type is that users must return a hyper::Error when they encounter an error during their Service.call. This causes them to wonder which variant of the current hyper::Error they should use, when the real anwer is: none! They should be returning (if it was a server error) a response with a 5XX status code.

They are also asked to return a hyper::Error in the outgoing Stream, but that is also incorrect. No hyper error occurred! However, they cannot alter to sending a 500 response anymore, so the only valid thing to do is send an error type with the semantics of "something blew up, we cannot respond more, just abort the socket".

So, there are 2 error types that would better suit these 4 cases:

  • Error: The hyper::Error type that comes only from within hyper, and represents when there was an error trying to read and parse incoming HTTP data.
  • Disconnect: A type that comes only from the user, given to hyper, and represents that the connection should be terminated immediately. Other names could be Fatal, Close, Abort...

This separation would hopefully give more clarity as to what error a user should be returning, and that in the Service::Future type, they shouldn't returning an error at all, since the error name should be ominous enough to point at returning a response instead.

There is of course still desire for users to have streams that return error types of std::io::Error, such as streaming from a File, or even hyper::Error, when streaming from a proxied client request, so there should exist From implementations for those types.


New proposal: #1431

@seanmonstar seanmonstar modified the milestone: 0.12 Jun 21, 2017
seanmonstar added a commit that referenced this issue Apr 6, 2018
**The `Error` is now an opaque struct**, which allows for more variants to
be added freely, and the internal representation to change without being
breaking changes.

For inspecting an `Error`, there are several `is_*` methods to check for
certain classes of errors, such as `Error::is_parse()`. The `cause` can
also be inspected, like before. This likely seems like a downgrade, but
more inspection can be added as needed!

The `Error` now knows about more states, which gives much more context
around when a certain error occurs. This is also expressed in the
description and `fmt` messages.

**Most places where a user would provide an error to hyper can now pass
any error type** (`E: Into<Box<std::error::Error>>`). This error is passed
back in relevant places, and can be useful for logging. This should make
it much clearer about what error a user should provide to hyper: any it
feels is relevant!

Closes #1128
Closes #1130
Closes #1431
Closes #1338

BREAKING CHANGE: `Error` is no longer an enum to pattern match over, or
  to construct. Code will need to be updated accordingly.

  For body streams or `Service`s, inference might be unable to determine
  what error type you mean to return. Starting in Rust 1.26, you could
  just label that as `!` if you never return an error.
seanmonstar added a commit that referenced this issue Apr 7, 2018
**The `Error` is now an opaque struct**, which allows for more variants to
be added freely, and the internal representation to change without being
breaking changes.

For inspecting an `Error`, there are several `is_*` methods to check for
certain classes of errors, such as `Error::is_parse()`. The `cause` can
also be inspected, like before. This likely seems like a downgrade, but
more inspection can be added as needed!

The `Error` now knows about more states, which gives much more context
around when a certain error occurs. This is also expressed in the
description and `fmt` messages.

**Most places where a user would provide an error to hyper can now pass
any error type** (`E: Into<Box<std::error::Error>>`). This error is passed
back in relevant places, and can be useful for logging. This should make
it much clearer about what error a user should provide to hyper: any it
feels is relevant!

Closes #1128
Closes #1130
Closes #1431
Closes #1338

BREAKING CHANGE: `Error` is no longer an enum to pattern match over, or
  to construct. Code will need to be updated accordingly.

  For body streams or `Service`s, inference might be unable to determine
  what error type you mean to return. Starting in Rust 1.26, you could
  just label that as `!` if you never return an error.
seanmonstar added a commit that referenced this issue Apr 7, 2018
**The `Error` is now an opaque struct**, which allows for more variants to
be added freely, and the internal representation to change without being
breaking changes.

For inspecting an `Error`, there are several `is_*` methods to check for
certain classes of errors, such as `Error::is_parse()`. The `cause` can
also be inspected, like before. This likely seems like a downgrade, but
more inspection can be added as needed!

The `Error` now knows about more states, which gives much more context
around when a certain error occurs. This is also expressed in the
description and `fmt` messages.

**Most places where a user would provide an error to hyper can now pass
any error type** (`E: Into<Box<std::error::Error>>`). This error is passed
back in relevant places, and can be useful for logging. This should make
it much clearer about what error a user should provide to hyper: any it
feels is relevant!

Closes #1128
Closes #1130
Closes #1431
Closes #1338

BREAKING CHANGE: `Error` is no longer an enum to pattern match over, or
  to construct. Code will need to be updated accordingly.

  For body streams or `Service`s, inference might be unable to determine
  what error type you mean to return. Starting in Rust 1.26, you could
  just label that as `!` if you never return an error.
seanmonstar added a commit that referenced this issue Apr 7, 2018
**The `Error` is now an opaque struct**, which allows for more variants to
be added freely, and the internal representation to change without being
breaking changes.

For inspecting an `Error`, there are several `is_*` methods to check for
certain classes of errors, such as `Error::is_parse()`. The `cause` can
also be inspected, like before. This likely seems like a downgrade, but
more inspection can be added as needed!

The `Error` now knows about more states, which gives much more context
around when a certain error occurs. This is also expressed in the
description and `fmt` messages.

**Most places where a user would provide an error to hyper can now pass
any error type** (`E: Into<Box<std::error::Error>>`). This error is passed
back in relevant places, and can be useful for logging. This should make
it much clearer about what error a user should provide to hyper: any it
feels is relevant!

Closes #1128
Closes #1130
Closes #1431
Closes #1338

BREAKING CHANGE: `Error` is no longer an enum to pattern match over, or
  to construct. Code will need to be updated accordingly.

  For body streams or `Service`s, inference might be unable to determine
  what error type you mean to return. Starting in Rust 1.26, you could
  just label that as `!` if you never return an error.
seanmonstar added a commit that referenced this issue Apr 10, 2018
**The `Error` is now an opaque struct**, which allows for more variants to
be added freely, and the internal representation to change without being
breaking changes.

For inspecting an `Error`, there are several `is_*` methods to check for
certain classes of errors, such as `Error::is_parse()`. The `cause` can
also be inspected, like before. This likely seems like a downgrade, but
more inspection can be added as needed!

The `Error` now knows about more states, which gives much more context
around when a certain error occurs. This is also expressed in the
description and `fmt` messages.

**Most places where a user would provide an error to hyper can now pass
any error type** (`E: Into<Box<std::error::Error>>`). This error is passed
back in relevant places, and can be useful for logging. This should make
it much clearer about what error a user should provide to hyper: any it
feels is relevant!

Closes #1128
Closes #1130
Closes #1431
Closes #1338

BREAKING CHANGE: `Error` is no longer an enum to pattern match over, or
  to construct. Code will need to be updated accordingly.

  For body streams or `Service`s, inference might be unable to determine
  what error type you mean to return. Starting in Rust 1.26, you could
  just label that as `!` if you never return an error.
@seanmonstar seanmonstar added the A-error Area: error handling label May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-error Area: error handling
Projects
None yet
Development

No branches or pull requests

1 participant