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

An Error that Users can create to give to hyper #1431

Closed
seanmonstar opened this issue Feb 1, 2018 · 2 comments
Closed

An Error that Users can create to give to hyper #1431

seanmonstar opened this issue Feb 1, 2018 · 2 comments
Labels
A-error Area: error handling C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.
Milestone

Comments

@seanmonstar
Copy link
Member

seanmonstar commented Feb 1, 2018

Part of the Error reform.

Motivation

Unlike the hyper::Error, which is meant to describe an error that occurred inside hyper and that information given the user, there a few instances where the user must tell hyper that an error has happened outside, and to stop it's scheduled action.

  • In the Client (see also A better Client Error experience #1130):
    • The user can create a Request<B>, with a custom Stream. They may need to tell hyper that the stream cannot be completed.
  • In the Server (see also A better Server Error experience #1128):
    • The user can create a Response<B>, with the same characterics and needs as the Client's Request<B>.
    • The user can provide a Service (and NewService), which returns Future<Item=Response<B>>, where that Future might also fail in creating a Response. In most cases, a robust server will want to send a response anyways, with a 4XX or 5XX status code, but there could exist instances where the user can't even (or doesn't want to) create those responses.

While hyper was using tokio-proto, there was a limitation on what error types could be used (only a single error type was allowed), but now that hyper has moved off, we can solve this (even now, before 0.12, I believe).

Design

(updated from #1431 (comment))

Allow E: Into<Box<std::error::Error>> in any place a user could return an error to hyper. That means:

  • In the server, Service::Error: Into<Box<std::error::Error>>.
  • The body streams (Entity) a user can provide should be changed to just type Error: Into<Box<std::error::Error>>;
  • The Connect trait should update to type Error: Into<Box<std::error::Error>>, and the bounds C::Error: io::Error on client should be removed.
@dekellum
Copy link

dekellum commented Feb 1, 2018

Assuming that by users you mean developers/code using the Hyper crate, perhaps a better name for this is (error::)Application? "User" might otherwise get confused, for example, with some authorization mechanism.

@seanmonstar seanmonstar added this to the 0.12 milestone Feb 7, 2018
@seanmonstar
Copy link
Member Author

Instead of some hyper::error::User, this could just be Box<std::error::Error> instead. This punts allowing the error type to indicate an HTTP2 error code to reset with, but it's probably otherwise much clearer. Basically, all things in hyper that would allow a user to signal an error occurred could just be E: Into<Box<std::error::Error>, which is the same bounds like for std::io::Error::new(kind, my_error).

Some benefits here:

  • Users can return their own error type, making it easier to plug their app in, and logs can show exactly the message that they wanted.
  • This still works with hyper::Error, so proxying one hyper::Body as the body of another message works fine. With specialization, we can remove the box internally, but not a big problem either way.
  • We still benefit from not having weird hyper::Error::from(io_err).

@seanmonstar seanmonstar added C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful. labels Mar 19, 2018
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 C-feature Category: feature. This is adding a new feature. E-medium Effort: medium. Some knowledge of how hyper internal works would be useful.
Projects
None yet
Development

No branches or pull requests

2 participants