Rocket icon indicating copy to clipboard operation
Rocket copied to clipboard

Introduce `FromRequest::on_entry()`

Open dpc opened this issue 8 years ago • 5 comments

Feature Requests

Why you believe this feature is necessary.

Right now FromRequest::from_request logic can only operate before the route is taken, typically to decide if the route should be taken or not. It would make request guards more powerful if they were able to do some stuff after the route was actually taken.

Convincing use-case

I have a rate-limiting type, and I'd like the rate-limiting logic to check rete limits after the actual route was matched and taken, but I don't want to have to call it everywhere manually.

Potentially this could be used to fix #466 by clearing the Flash only when route was taken.

Why this feature can't or shouldn't exist outside of Rocket.

Rocket controls what gets called and when.

Notes

This could be implemented by adding new method to FromRequest with no-op default implementation and generated code calling it on all request guards before calling the handler function.

Maybe on_exit would be useful too.

dpc avatar Nov 20 '17 01:11 dpc

Can you detail further why it's important that you know which route was taken in your example of rate-limiting? In on_request, you have the complete request information, including the path. I'm wondering: why isn't this sufficient?

I believe you can also implement what you're suggesting as a request guard. Then you'd simple need to add the type to the route's signature to get the functionality you're looking. This way, you also have the flexibility to order guards the way you'd like.

SergioBenitez avatar Nov 22 '17 15:11 SergioBenitez

@SergioBenitez : Yes. So I think I want on_request-like hook for request guards. Right now in from_request I don't know if the route will be actually taken, so I have to manually call something at the top of the function:

#[(get(...)]
fn some_api_call(rl : RateLimiter) -> Result<Template> {
    rl.limit()?;

}

Instead, I'd like RateLimiter to do rl.limit() in a on_enter hook, so I don't have to remember about calling it.

dpc avatar Nov 22 '17 18:11 dpc

If you have a route that looks like this:

#[(get(...)]
fn something(a: A, b: B, c: C, rl: RateLimiter) -> T {
   ...
}

Then if the from_request() method of RateLimiter gets called, the route something will be called as long as RateLimiter's guard doesn't fail. Request guards are the final mechanisms by which routing can fail. If the last request guard (the right-most one) gets called, routing will succeed as long as that request guard doesn't fail or forward the request.

SergioBenitez avatar Nov 22 '17 18:11 SergioBenitez

@SergioBenitez I don't think users should have to figure out the right order of elements, especially that nothing can enforce it, and mistakes would lead to weird bugs. That's why I think from_request could be split into "check" and "do" part. "check"s should be fast and idempotent, while "do" can have side effects.

There are basically two classes of request guards: failable matches (that can prevent taking the route at all), and infallible utilities, and they are both used the same way, but the user has to keep their order straight.

Hmmm... Maybe the macro could take care of the ordering internally. Like one of the could be marked / distinct and macro would try them in the right order. Is that possible?

dpc avatar Nov 22 '17 20:11 dpc

Hmmm... But then in my RateLimiter I'm returning a special error case, that is being handled in Response implementation of `Result, and this displays a nice "too fast, try in a minute" page. I wish that would still be possible.

dpc avatar Nov 22 '17 20:11 dpc

I believe the general notion being asked for here will eventually be answered by #749. Closing in favor of that issue.

SergioBenitez avatar Apr 16 '24 22:04 SergioBenitez