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

Running a middleware before the router? #292

Open
TimNN opened this issue Dec 2, 2018 · 3 comments
Open

Running a middleware before the router? #292

TimNN opened this issue Dec 2, 2018 · 3 comments

Comments

@TimNN
Copy link

TimNN commented Dec 2, 2018

Hi, I've recently started experimenting with Gotham and so far have to say Thank You, for an awesome library! Now to my issue:

If I understand my debug logs correctly, any middleware I add to a Router will only run if the Router successfully matches a route. I have a middleware1 I want to run irrespective of whether the Router has any matching routes (or rather, I want the middleware to run before the Router).

Is there a standard way of doing this?

I haven't had the time yet to experiment, but I've thought of the following approaches:

  • "Manually" wrap the middleware around the Router by implementing (New)Handler.
  • Use PipelineHandleChain to do the same.
  • Use two nested Routers: The outer one has the middleware I want to run and a * delegation to the inner router (not sure if that would work).

1: The actual use case is that I want to reject all unauthenticated requests with a 403. If the request is authenticated, I want the normal 404 handling of the router to take place. If Gotham has other ways to deal with this, I'd be happy to hear about them.

@TimNN TimNN changed the title Middleware running before router? Running a middleware before the router? Dec 2, 2018
@TimNN
Copy link
Author

TimNN commented Dec 3, 2018

So, I went with approach (2) by defining two types

struct PreMiddlewareWrapper<C, P, NH> {
    pipeline_chain: C,
    pipelines: PipelineSet<P>,
    new_handler: NH,
}

struct PreMiddlewareHandler<C, P, H> {
    pipeline_chain: C,
    pipelines: PipelineSet<P>,
    handler: H,
}

and associated NewHandler / Handler implementations (which was pretty straightforward, except for figuring out the correct bounds on everything).

Would there be interest in having something like this in Gotham? If so I can send a pull request with the implementation. (Which module would be most appropriate for this code?)

@whitfin
Copy link
Contributor

whitfin commented Dec 3, 2018

Hey @TimNN! This has actually come up a couple of times I think; I've never been sure on whether middleware fires on 404s or not. If it's indeed true that they don't, it seems like this should be something we support officially somehow - so it might be worth PR'ing what you wrote up for discussion.

Curious what @colinbankier and @nyarly think!

@colinbankier
Copy link
Collaborator

Thanks @TimNN. Yep - I think this is certainly an interesting case that we don't have a good answer for yet. I'm wondering if this also gives us an avenue to handle CORS/OPTIONS gracefully, i.e. #119.
Your idea sounds great. Yep, I think you're right that your 3rd option, using nested routers, probably wouldn't work off the top of my head, so option 2 sounds most promising.
We'd love a PR with what you have so far :)
Would be interesting to see how we can incorporate this idea in way that is intuitive and consistent with the middleware / pipeline apis.

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

Successfully merging a pull request may close this issue.

4 participants