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

Param filter with user-defined rejection #293

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

hwchen
Copy link
Contributor

@hwchen hwchen commented Oct 28, 2019

updates filters::param2:

  • changes name to param_with_err. This still doesn't quite feel right to me, so let me know if there's other suggestions.
  • returns Rejection directly from FromStr Err, instead converting the Err to a Cause and always returning a not found Rejection.
  • created a fuller example in the docstring.

Let me know if this is the idea you had from #255.

@hwchen
Copy link
Contributor Author

hwchen commented Oct 28, 2019

Playing around with this a bit, I'm not sure it makes sense to have users implement their own Err -> Rejection if Causes are going to be the main way of identifying errors in a recover. Also, from notes in the api, it looks like you're moving towards using custom rejections more generally.

So, perhaps it makes more sense to have param_with_err just return a custom rejection, instead of having the user define an extra impl that's not really that powerful.

@seanmonstar
Copy link
Owner

I'm not sure it makes sense to have users implement their own Err -> Rejection if Causes are going to be the main way of identifying errors in a recover.

I'm not quite sure what you mean by this. However, relatedly, master has just seen a refactor of the rejection system which would affect this (#311).

@hwchen
Copy link
Contributor Author

hwchen commented Nov 13, 2019

Ok, I'll look at the new rejection system.

@hwchen
Copy link
Contributor Author

hwchen commented Nov 15, 2019

I've rebased on master. In this implementation, the user just has to implement reject::Reject for their error, then they can use it in param_with_err (formerly param2). Let me know if you have any questions on this now.

@hwchen
Copy link
Contributor Author

hwchen commented Nov 15, 2019

And it seems like you no longer need to implement std::error::Error to work with the custom rejection system, just reject::Reject.

@hwchen
Copy link
Contributor Author

hwchen commented Apr 22, 2020

It's been a while since this PR was submitted, I've updated it to better reflect the latest Rejection system.

Quick summary:

path::param will always return reject::not_found() if there's an error in parsing. path::param_with_reject will allow the user to specify a custom rejection in case of parsing failure.

I see that param2, which served this function previously (I think), was removed. Let me know if there's no longer interest and I can close this PR.

@jxs
Copy link
Collaborator

jxs commented Jun 30, 2021

Hi, and sorry for the delay! So if I understand correctly what you are trying to achieve is being able to replace a Known rejection with a custom one right? If so can't you do that with a recover handler and find and return the response you want on that case

@jxs jxs added the waiting-on-author awaiting some action (such as code changes) from the PR or issue author. label Jun 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
waiting-on-author awaiting some action (such as code changes) from the PR or issue author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants