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

impl Reject for T: Display + Debug #374

Closed
wants to merge 1 commit into from
Closed

Conversation

jxs
Copy link
Collaborator

@jxs jxs commented Jan 14, 2020

  • addresses Rejection and anyhow #307
  • allows types which impl std::error::Error to not need impl Reject for T {}
  • allow one to just return:
warp::reject::custom("quick rejection, TODO: improve into a type")

so that anyhow! dependency becomes unnecessary

@seanmonstar
Copy link
Owner

When I redesigned the rejection system, one of my goals was to make rejection explicit. To make someone decide if the route should be rejected, and thus other routes should be tried, or if their error should be converted into a reply.

@jxs
Copy link
Collaborator Author

jxs commented Jan 14, 2020

oh, yeah I just realized reading reject.rs:L128 that it was assumed 👍

@jxs jxs closed this Jan 14, 2020
@udoprog
Copy link
Contributor

udoprog commented Jan 15, 2020

@seanmonstar What initially raised this was #307, I'm curious what you'd recommend to support rejections then for types foreign to the crate (like with anyhow::Error). Would that be using newtypes (or some other crate-local rejection type) as per my comment?

@seanmonstar
Copy link
Owner

@udoprog if the filter should reject and try another filter, then sure. You can define some type that you'd expect to look for later in a recover.

@thomaseizinger
Copy link
Contributor

HttpApiProblem is getting support for warp as soon as 0.2 is released: chridou/http-api-problem#17

@udoprog
Copy link
Contributor

udoprog commented Jan 15, 2020

@udoprog if the filter should reject and try another filter, then sure. You can define some type that you'd expect to look for later in a recover.

I'll be referencing the errors example below:
https://github.com/seanmonstar/warp/blob/master/examples/errors.rs#L42

My goal with rejections have never been try another filter. The way I've implemented error handling in my project is to recover immediately at the end of each unique filter to limit the number of retries.

The errors example above strictly speaking does try another filter since recover is used after the inner filter. But error customization seems to be its primary use since the paths do not match (nope != oops), meaning there is no overlap in the filters. Reading your response here and on other issues, it seems like error customization is a secondary goal of rejection?

If so, some dedicated mechanism for propagating errors down to an error handler without trying other filters might be worth considering (if one doesn't already exist?). In my experience that is currently the dominant use case for custom reject/recover. Two other people I've talked to about error handling in warp have been tripped by this so far.

@udoprog
Copy link
Contributor

udoprog commented Jan 17, 2020

@seanmonstar I'd be curious on your take of the above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants