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

Double fault catchers to allow catchers to access the state. #953

Closed
Arignir opened this issue Mar 22, 2019 · 2 comments
Closed

Double fault catchers to allow catchers to access the state. #953

Arignir opened this issue Mar 22, 2019 · 2 comments
Labels
request Request for new functionality

Comments

@Arignir
Copy link

Arignir commented Mar 22, 2019

Hello,

As far as I know, the current implementation forbids catchers to be dynamic, which prevent the website to implement a nice error-page with infused template depending on dynamic parameters (like a navbar that shows the user login and picture)

The reasoning given in PR #222 was

Error catchers cannot fail. They are meant to catch, not handle errors. That is, they are a sink for errors that aren't handled elsewhere. If you need custom error handling, you can use forwarding and/or Result return types to handle this.

Therefore, if we want a dynamic catcher that does nice and pretty things, the recommended implementation is for each route to:

Return a Result<T, status::Custom>, where maybe T and E are both Template, or something else entirely. Then you can set the status code and emit whatever custom response you'd like.

Unless I misunderstand what you're saying, this looks like a hackish, painful solution that requires a lot of boilerplate code for a simple problem.

Also, it won't work for 404 handlers unless an extra "match-all" route is added that returns a Template with a custom http code (which defeats the purpose of registering a catcher in the first place)

To answer that problem, I propose a system similar to what most CPU do when an exception is raised within an exception handler: a double fault handler.

Rocket could allow a catcher to access the usual dynamic parameters like the state, cookies, etc., like any other route. Therefore, such catchers could (and would be allow) to fail.

If a catcher fail, an extra "double fault" catcher is called which has the same restriction than the current ones: it cannot fail and therefore cannot access the current state. This prevent infinite recursion of catchers.

I think this solution is both elegant and powerful. What do you think?

@jebrosen
Copy link
Collaborator

As far as I know, the current implementation forbids catchers to be dynamic

Rocket could allow a catcher to access the usual dynamic parameters like the state, cookies, etc., like any other route. Therefore, such catchers could (and would be allow) to fail.

You may have missed this: it is possible to manually invoke any and all request guards, including State and Cookies, via Request::guard().

It's not possible to pass any kind of data directly from a route to an error catcher. This is one of the motivations for using Result<T, E> where T and E both implement Responder. It could still probably be done with request-local state if you really wanted to.

There was a design idea that would allow routes to pass an error value (like SomeError in Result<SomeResponder, SomeError> directly to an error catcher (#[catch(400)] fn catch_400(error: SomeError)), but it's currently impossible because of limitations of the rust language and hasn't been revisited in a while.

@jebrosen jebrosen added the request Request for new functionality label Mar 24, 2019
@Arignir
Copy link
Author

Arignir commented Mar 24, 2019

Oh, i did in fact missed this. Thanks!

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

No branches or pull requests

2 participants