Unexpected dependency of ordering with static dir and dynamic routes

Questions and Help
#1

I’ve been hitting an issue trying to get Sanic request parameters working, which it turns out is due to the fact I’m using app.static() at the same time. This appears to be https://github.com/huge-success/sanic/issues/1543, which went stale without being addressed. I’d suggest this should probably be reopened, if people agree there’s an issue here.

I’ve narrowed it down to the following example:

import sanic, sanic.request, sanic.response

app = sanic.app.Sanic()

app.static("/", "./static")

@app.route("/")
@app.route("/<param>")
async def route(req, param=None):
    return sanic.response.text(param if param else "homepage")

app.run(port=3333)

The above gives a “file not found” error when navigating to http://<baseurl>/foo, since it’s looking for a file called “foo”, and fails to fall back on the parameter match app.route("/<param>").

A workaround is to have the app.static("/", "./static") line come after the ‘route’ definition, i.e. after "/<param>" has been added as a route. It feels like the order shouldn’t matter here though!

Seen on Sanic 19.3.1 and 20.6.3.

#2

@LewisGaul, first off… welcome to the Sanic community.

Hmm … I am not sure I agree. Routes are added to a list. That list is evaluated and the first time it finds a match, it stops. Otherwise, you would needlessly loop through potentially more routes than you would need to.

Static handler works a little differently than others. There is no way to raise a RouteExists on static because the files that will be there are unknown.

Contrast that with this:

def foo(request):
    return sanic.response.text("foo")

def bar(request):
    return sanic.response.text("bar")

app.route("/foo")(foo)
app.route("/foo")(bar)

This will throw and exception, which makes sense, because it is a redeclaration. By placing app.static before or after your route declarations, you are defining its priority. When it has priority over everything else, a handler exists. So when there is a matching pattern, that handler will take over. If there is no file there, the routing is already done and cannot go backwards.

On the other hand, when the app.route is defined with priority, there is still a fall back handler for app.static. Given your use case, I think this is what you are looking for and is actually a solution and not an issue.

If you disagree, please let me know why.

#3

Thanks for your response :slight_smile:

To be honest, I assumed this was an oversight rather than intentional behaviour. It’s fine if I was wrong about that - I just wanted to make sure a potential bug wasn’t missed!

To give my reasoning… In my opinion, the use of decorators in this way gives the appearance of a static declaration, in this case “this function handles this route”, which gives me a feeling that position in the file shouldn’t matter. Of course, I understand that’s not how decorators actually work in python, and perhaps a reasonable answer would be “don’t add routes with decorators then”!

Also, on your example of redefining a route - I agree that behaviour is expected. I’m just not convinced it naturally explains the behaviour I brought up - when there’s a conflict between route declarations and static, one silently takes precedence over the other. In itself that seems reasonable enough, but I’d assume when one fails to match then the other would be there as a fallback, which appears to only be the case with static being the fallback. This just feels like an asymmetry to me, but perhaps there’s a good reason that I’m not fully understanding. Bear in mind I’m arguing from a user’s point of view, and it seems your argument has a bit more of an implementation stance - to be expected of course! Both viewpoints can be useful to consider :slight_smile:

I didn’t see anything in the documentation stating that the order matters, perhaps that could be made more explicit to save someone some confusion in future? :slight_smile:

#4

You are indeed 100% correct from a user’s perspective. And that is valuable. To be honest, the router is something I have been fighting with a lot the last couple years and I am just waiting for the day to rip it out and start over. But, that will be a major change, so you can imagine we are very hesitant to do so.

As for documentation… :sigh: Yes. That too is something that probably should be explained better.

#5

Understood! For now I’ll post a link to this discussion on the github issue I originally found to help people who might hit this in the future.

Thanks for talking it through, best of luck with any potential rewrite one day :wink:

1 Like
#6

Oh, it will happen! :laughing: Probably not until next year sometime though.