[RFC] extensible routing in Tide

[gameldar]

This is the PR version of #271 to carry over the discussing in the PR format which allows better ways to have discussions around individual elements of the PR.

Rendered

[fairingrey]

Following #271 (comment), I may just try writing a PR using that instead, although I'm curious as to whether it provides the routing behavior that Nemo157 is asking for, i.e. it should behave similarly to #271 (comment) sans the last bullet point -- /{foo}*/{bar} should work.

[tirr-c]

If we serve routing as middleware, other middleware cannot see the routing result as it should be run before routing. We need to either special-case routing, or not provide the result to middleware.

I see routing as Fn(&Request) -> Option<(Endpoint, Params)> (+ Sync). With this signature, we can think of a trait responsible for routing:

trait Router {
    /// Perform routing of given request.
    ///
    /// If there is no match, this method returns `None`.
    fn route(&'a self, request: &Request) -> Option<(&'a DynEndpoint, Params)>;
}

[gameldar]

If we serve routing as middleware, other middleware cannot see the routing result as it should be run before routing. We need to either special-case routing, or not provide the result to middleware.

That is what I've been thinking too. If you can change the endpoint via middleware what is the right response for continuing processing?

I was playing around with making the Endpoint effectively editable by the middleware. The question then you have answer is what happens if you change the routing in middleware - do you start all the middleware again, do you continue on with the chain, or do you go directly to the endpoint immediately.

With routing in the middleware (doing internal redirects) you likely have to say 'if we change the "path" we have to rerun everything' as we have no way of determining where we are in terms of performing all the middleware processing. If routing is accessible to internal redirects we can at least determine that behaviour. Arguably you can do the same with middleware by putting that determination into the middleware implementation e.g. through the Next struct implementation trait you could specify more than the run function that would do the alternatives:

• forward changes the endpoint and then calls run internally,
• redirect that changes the endpoint and starts the routing again.

However it is a lot more complex and I wonder what advantage that provides with a pure middleware implementation - you might be better off just implementing your own routing in your endpoint and having a 'catch all' (/*) route to get everything to your custom routing implementation anyway.

[Nemo157]

I'll try and sketch out how I see routing as middleware working later today. The basics are that a router is a middleware that is configured with other middleware, it somehow looks at the current path and determines which of those other middleware to invoke next (maybe after modifying the current context to put parameters it has parsed out of the path).

One of the upsides of this sort of setup is it means you aren't limited to putting middleware at the top-level of the application, if you want to do a top-level router that just checks if the initial path segment is /api and then runs the request through a whole series of API specific middleware before it hits the actual API router, then you don't need to have those middleware applied during non-API requests.

In this sort of scheme there is no Endpoint, an endpoint is just a middleware that doesn't delegate to some other middleware and will always return a result.

[Nemo157]

@fundon

What won't be resolved through this RFC. While these may come up in passing because of determining the above questions it shouldn't be the focus of the discussion:

• What the best default implementation is. I.e. this shouldn't become a discussion of the final precedence rules, or what the 'default' should look like.

This is purely about whether it's worth supporting more extensible/user-replaceable routing, and what the best way to achieve that is. The actual rules for how the router(/default router if it is fully replaceable) should parse a path aren't really relevant.

[fairingrey]

Sorry if I haven't made my comments fully related to the issue at hand, as I'm still unsure about whether extensible/replaceable routing should be an included feature pursuant to what Tide is intended to be. I'm largely toying around with how routing might change if we are to follow what a webserver ought to do (as mentioned in the earlier issue with 404 not found resources/405 invalid methods/default options/etc), and if there is a case for custom, plugable routing that users or organizations might prefer over the default.

I've been playing around with actix-web 1.0 lately and while they don't have extensible routing, their default routing already makes a lot of sense for all but the most uncommon cases I can imagine. I'll say again that if I had to decide, I'd probably settle on having good default routing -- the codebase would remain relatively simple and easy to pick up.

That said, if a good design can be made for it, then I'm for considering it. Though as mentioned before, the router is currently closely integrated with Tide so I imagine it may get somewhat difficult or complicated.

[Nemo157]

Finally got around to throwing together a full (synchronous) sketch of integrating routers as normal middleware. This was just the first example I could get going so some of the naming is a bit off, but all the basics are there and working.

Start by running it and scrolling down to main to see how the app is defined, in this case App itself is just a normal middleware as well, it is basically a serial applicator for other middleware to chain them together. Then the two routers are sort of like "fan-out" middleware, they take the current request and if it matches one of their routes call out to the associated middleware for that route after updating the context. The currently routed path is kept track of on the context (and for a router that parsed fields out while routing those could also be placed on the context somewhere), and the remaining path for the prefix router is left on the request for any sub-router to match.

If the routers don't match then they act like other middleware and just call the next middleware in the chain, so you can add custom 404 handling by just chaining on a middleware to do that.

You can see what I mention about being able to add route specific middleware under the /api path, this has a middleware to set the content-type. In a more realistic application you might instead have a middleware that handles the Accept header here, while static paths will not need that, and server-side rendered HTML might have middleware relating to the rendering framework.

[gameldar]

Finally got around to throwing together a full (synchronous) sketch of integrating routers as normal middleware. This was just the first example I could get going so some of the naming is a bit off, but all the basics are there and working.

Conceptually I do like this. I think it allows greater power/flexibility while still allowing a way for good defaults to be applied with a simpler setup.

It also actually addresses one of the other questions I had as well - how to apply middleware to only portions of the application (I was prompted about this by looking through the Java Servlet spec and noting that their middleware, aka filters, are applied against a path match). Otherwise the onus is on the middleware to validate that it should be doing anything depending on the path.

The advantage I see here is that it does allow for greater flexibility for those that need it as well being able to provide sane setup for simpler use cases. I think with a good builder to set up the application we could then provide a default routing middleware at the head of the middleware chain to do the routing as it currently is.

Two things I liked but weren't entirely obvious:

1. As you pointed out you can have custom error handling for the different routing paths - e.g. you return json body for the error on the API and just a standard 404 otherwise.
2. You can still apply middleware to all paths (which is currently the same in the existing middleware now I realised it) - by calling the next first within the middleware implementation and then dealing with the response. I was worried the split of routing would affect this but it doesn't.

One question I did have - Is this going to be a greater problem in terms of determining at startup that configuration is wrong? Particularly around having a default routing implementation that kicks in via a builder somewhere?

I'm not sure having the nested App makes total sense either - just from a naming convention as such. i.e. it might make more sense to have something like MiddlewareExecutor that is part of the App and so you can nest a new one of those. But that is part of the roughness of the sketch!

Lastly I wonder how the handling should work if no successful routing is performed. e.g. if you remove the error handling at the moment it panics (trying to execute a next which doesn't exist). i.e. does it make sense to have a catch all '404' that is always present if you get to the end of the chain and there is nothing next to call (and can this be done if the work is actually handled by the middleware itself)

[tirr-c]

It seems that there isn't any strong objection to extensible routing. With extensible routing, having a sensible default router would be sufficient. @Nemo157's sketch looks nice for me, maybe it could be the starting point? I do think we need to check how much boilerplate it requires, though.

[gameldar]

It seems that there isn't any strong objection to extensible routing. With extensible routing, having a sensible default router would be sufficient. @Nemo157's sketch looks nice for me, maybe it could be the starting point? I do think we need to check how much boilerplate it requires, though.

Yeah that is what I'm currently looking at (but has mostly been a thought experiment so far... I've fiddled with the code just a little) - the work involved in integrating it and particularly how we can handle default routing. The main problem I'm seeing is that if routing is just middleware (even when using a builder) how do we determine that we need to add default routing. I guess there are two possible options here:

1. We leave the at methods and continue to use those - but when first called it'll add in the default routing middleware. If we don't use a builder then the problem is determining where to stick the default router in the middleware chain. With a builder we can resolve that at the end.
2. The second option is to leave in the default routing as non middleware, but make it optional. Therefore you can still do routing as middleware or use the default routing. From @Nemo157 's sketch this actually would tie in with making App middleware as well - as it can then perform the routing at any point within the chain as required and the at functions are then just the configuration options for the routing.

Looking through the current implementation option 2 is more or less currently possible already (bar it doesn't currently implement Middleware) - routing as such is already optional. The one problem (even currently) is that you can actually set it up so there is no routing at all and it produces no warnings (other than 'function is never used' warnings by the compiler if you have handling functions that aren't referenced), but I'm not sure we can actually determine that anyway.

[yoshuawuyts]

I'd like to see more in terms of motivation here. Could you repeat what the problems are stated in the links?

[yoshuawuyts]

Also the stakeholders section talks about "users with specialized routing requirements". It'd be good to lay out here what those requirements are.

[gameldar]

I've just updated this with feedback and comments around this - note that some of the changes also were a reflection of a discussion with @yoshuawuyts which was happening while github was down (I don't know if you want to extrapolate on that more @yoshuawuyts?). The gist of that discussion was around effectively how can we provide routing in Tide such that it can be used in a great variety of use cases some of which we don't really know at this point - such use cases could include aws lambda backends, cloudflare, browsers through service workers.

Hence the desirability to provide flexibility in routing.

The other half was when looking into routing as middleware I realised that it is already possible currently - there is no evaluation that there is any route before calling into middleware so you don't have to define any rules for routing and then can do all the routing in middleware. The main part you can't do here is have middleware change the routing endpoint - it has to handle it all currently.

[yoshuawuyts]

@gameldar yeah dang, the review I did for this RFC got eaten by GitHub as it was going down.

My Tl;Dr: is that it feels like this RFC touches on some excellent points about what we may want from a routing, but I feel it unnecessarily ties itself to a particular to a specific direction.

There are several tradeoffs listed, and open questions at the end which seems necessary to be answered before we could confidently say that having swappable routers is indeed the right approach.

I'd love to see the two points separated, and have this RFC focused on the challenges in routing we currently face. If we could crystallize what behavior we want from a routing solution would be an excellent outcome to me, and we could then leave open the discussion of which exact direction to take for a later RFC.

[gameldar]

My Tl;Dr: is that it feels like this RFC touches on some excellent points about what we may want from a routing, but I feel it unnecessarily ties itself to a particular to a specific direction.

There are several tradeoffs listed, and open questions at the end which seems necessary to be answered before we could confidently say that having swappable routers is indeed the right approach.

Ah I took the opposite thought from our discussions (it was 1AM my time when we finished... so I'm arguing that is the cause!) - that effectively "we don't know what the future routing needs will be so we should make it replaceable so we future proof for those different requirements". Hence the changes I made in the last update. However, the other approach is to cross that bridge when we come to it.

I'd love to see the two points separated, and have this RFC focused on the challenges in routing we currently face. If we could crystallize what behavior we want from a routing solution would be an excellent outcome to me, and we could then leave open the discussion of which exact direction to take for a later RFC.

This was effectively what I was saying we'd discuss later because we need to solve that question either way - but I can see your point. I'd actually argue doing the reverse though - shelving this RFC (for now) and starting a new one to answer that question. The 'what are the problems with the existing routing' question actually has implementable bits for the now as well - and is less about 'what we might not know' for the future.

Arguably we have at least vaguely answered the question 'should Tide have extensible routing' with a 'yes most likely' without necessarily committing to 'this is the way forward'. If we consider that is enough then I'd be tempted to at least table that as a decision in some form (e.g. I'll update the RFC and we can merge that) but leave it open to a future RFC to look at the implementation when it is needed. I'm not sure 100% of the value of doing that at this point however - as I mentioned this kind of RFC has been hard to know where to fit it as it isn't so much about a technical implementation as a 'high level design' goal so of thing.

[pickfire]

Is it possible to have routing checked at compile time? Or there is an option to have it optionally not checked instead? I do think static type checking is a good use case for routing.

[gameldar]

Is it possible to have routing checked at compile time? Or there is an option to have it optionally not checked instead? I do think static type checking is a good use case for routing.

Aaron addressed this in the discussion around the original idea for Tide: https://internals.rust-lang.org/t/routing-and-extraction-in-tide-a-first-sketch/8587/2

Essentially it boils down to a trade off between complexity in the code base and for users of the API too (e.g. using macros that hide the implementation details) - where failing on initial startup can provide better failure messages and can be verified with a simple test.

[gameldar]

Discussion around this has reached its conclusion - which is to not make changes at present until we find a need for it. I'll close this RFC as the history will still be here if we need to come back to it.