Sanic: Sanic 19.12 breaks some middleware usages

[qwesda]

The commit _GIT-37: fix blueprint middleware application_ (https://github.com/huge-success/sanic/commit/a6077a179067812766293f9ab4391b5e1499a9c6#diff-c4444a35dbd5e37ee480b0e8888e0880R939) changes how the middleware are evaluated.

Before this commit the middleware where evaluated before the route was determined. This allowed for example to rewrite to requested url. In my case I was using that to remove a url-prefix that is used for routing purposes.

I do think this is a legitimate use case for middlewares. According to the documentation:

Middleware are functions which are executed before or after requests to the server. They can be used to modify the request to or response from user-defined handler functions.

modifying the requested url should be possible.

[ahopkins]

I'm not the best one to speak to this one, because I was not involved on this one. (@harshanarayana?) However, I agree that the middleware/routing needs some work still. Something that still needs work (similar to what you are using middleware for): https://github.com/huge-success/sanic/pull/1373

[nlnjnj]

this fix also crash the app

caused

[2020-01-02 11:22:14 +0800] [11996] [ERROR] Exception occurred in one of response middleware handlers
Traceback (most recent call last):
File "D:\ProgramData\Anaconda3\envs\revive_backend\lib\site-packages\sanic\app.py", line 1017, in handle_request
request, response, request_name=name
TypeError: _run_response_middleware() got an unexpected keyword argument 'request_name'
Executing result=None created at D:\ProgramData\Anaconda3\envs\revive_backend\lib\site-packages\sanic\server.py:424> took 20.313 seconds

[lrnselfreliance]

I'm seeing a similar error in my docker container:

api_1     | [2020-01-03 16:35:58,552] [wrolpi] [WARNING] Logging level: 10
api_1     | [2020-01-03 16:35:58,552] [wrolpi] [INFO] Importing configs
api_1     | [2020-01-03 16:35:58 +0000] [1] [DEBUG] 
api_1     | 
api_1     |                  Sanic
api_1     |          Build Fast. Run Fast.
api_1     | 
api_1     | 
api_1     | [2020-01-03 16:35:58 +0000] [1] [INFO] Goin' Fast @ http://0.0.0.0:8080
api_1     | [2020-01-03 16:35:58 +0000] [8] [INFO] Starting worker [8]
api_1     | [2020-01-03 16:35:58 +0000] [7] [INFO] Starting worker [7]
api_1     | [2020-01-03 16:35:58 +0000] [10] [INFO] Starting worker [10]
api_1     | [2020-01-03 16:35:58 +0000] [9] [INFO] Starting worker [9]
api_1     | [2020-01-03 16:36:05 +0000] [9] [ERROR] Exception occurred while handling uri: 'http://0.0.0.0:8080/'
api_1     | Traceback (most recent call last):
api_1     |   File "/usr/local/lib/python3.8/site-packages/sanic/app.py", line 945, in handle_request
api_1     |     response = await self._run_request_middleware(
api_1     | TypeError: _run_request_middleware() got an unexpected keyword argument 'request_name'
api_1     | [2020-01-03 16:36:05 +0000] [9] [DEBUG] CORS: Request to '/' matches CORS resource '/*'. Using options: {'origins': ['.*'], 'methods': 'DELETE, GET, HEAD, OPTIONS, PATCH, POST, PUT', 'allow_headers': ['.*'], 'expose_headers': None, 'supports_credentials': False, 'max_age': None, 'send_wildcard': False, 'automatic_options': False, 'vary_header': True, 'resources': '/*', 'intercept_exceptions': True, 'always_send': True}
api_1     | [2020-01-03 16:36:05 +0000] [9] [DEBUG] CORS: Cannot find the request context. Is request already finished?
api_1     | [2020-01-03 16:36:05 +0000] [9] [ERROR] Exception occurred in one of response middleware handlers
api_1     | Traceback (most recent call last):
api_1     |   File "/usr/local/lib/python3.8/site-packages/sanic/app.py", line 1016, in handle_request
api_1     |     response = await self._run_response_middleware(
api_1     | TypeError: _run_response_middleware() got an unexpected keyword argument 'request_name'
api_1     | [2020-01-03 16:36:05 +0000] - (sanic.access)[INFO][172.18.0.1:52176]: GET http://0.0.0.0:8080/  500 144

It seems to respond the same for every request. Here are my pip requirements:

root@6c52b77de4a5:/opt/wrolpi# pip list | grep sanic
sanic                   19.12.2    
sanic-openapi           0.6.1     

[harshanarayana]

@lrnselfreliance @qwesda @nlnjnj Do you happen to have a small sample app that you can share with me so that I can use that as a reference and make sure to include them in the unit test? I am working on looking into getting this sorted this week.

[qwesda]

hi,

you can use this as a sample app:

import sanic
import sanic.response


async def request_middleware(request):
    from httptools import parse_url

    if request.path == '/ping':
        return sanic.response.HTTPResponse('pong', status=200)
    elif request.raw_url.startswith(b'/api'):
        request.raw_url = request.raw_url[4:]
        request._parsed_url = parse_url(request.raw_url)

    return


async def endpoint(request):
    return sanic.response.HTTPResponse('endpoint', status=200)


if __name__ == '__main__':
    app = sanic.Sanic()

    app.request_middleware.append(request_middleware)
    app.add_route(endpoint, '/endpoint')

    app.run(port=8080)

The request_middleware has two functions:

• provide a /ping route to test returning a response from the middleware
• rewrite urls by removing the prefix /api from requests

A test should expect:

• response 200 for /ping with body pong
• response 200 for /api/endpoint with body endpoint
• response 200 for /endpoint with body endpoint

[ashleysommer]

Weighing in, Sanic-Plugins-Framework and Sanic-CORS author here.

Thanks @aymanbagabas for bringing this to my attention.

The tracebacks that @nlnjnj and @lrnselfreliance have provided above both show a crash within Sanic-Plugins-Framework (SPF). SPF is used as the basis for several popular Sanic plugins including Sanic-CORS.

Being busy over Late-December and Early-January period, I missed the Sanic 19.12 release, and as a result SPF is _not_ tested to work with Sanic 19.12.

This change to the handling of blueprints middleware in Sanic 19.12 breaks all released versions of SPF, so all SPF-based plugins will not work on Sanic 19.12.

In fact, SPF-current (v0.8.2) is only fully supported for 0.8.3 <= Sanic <= 19.6.3 however it has been tested to work on 19.9.0 too.

In the coming days, I will be feverishly adding support for Sanic 19.12 in SPF, however in the meantime, if you are using Sanic-CORS or any other Sanic Plugin which is built on top of SPF, please use sanic==19.6.3 or if you need newer features, try sanic==19.9.

[harshanarayana]

@ashleysommer Oh! I totally forgot about giving you a heads up when the blueprint behavior was altered in terms of handling the middleware. Sorry !

[ashleysommer]

@harshanarayana
No worries! SPF is not an official Sanic project, so its not a requirement that it is kept fully in sync. And it is normal that SPF releases lag behind Sanic releases by a few weeks while I go through the sanic commit logs to make any necessary changes to SPF.

@aymanbagabas @nlnjnj @lrnselfreliance
I've pushed a new beta version of SPF (v0.9.0.b1) which should work with Sanic 19.12.2 I'm currently verifying that it works with Sanic-CORS (it should) and my other plugins. Could you guys please test the new beta version to see if it fixes the problem you're seeing on Sanic 19.12.2?

Update: Sanic-CORS v0.10.0.b1 is released as Beta status with Sanic 19.12 compatibility

Install using PIP with --pre like this:

pip3 install --pre "sanic-cors>0.9.99"

[ahopkins]

Can we close this issue?

[qwesda]

Can we close this issue?

I don't think we can. The main issue (for me) is that it is not documented if the middleware run before the router or after the router. Before 9.12 they were run before the router and now they are run after. And for my use case I need them to run before, since I rewrite the request url.

Maybe introducing pre router request middlewares might be the best option, since I think the new middleware design requires them to run after the router ...

[ashleysommer]

@qwesda You're right, the main topic of this issue is about the router now running before the request-middleare, and that is not yet resolved.

Sorry we hijacked this thread to chase down a different unrelated issue introduced in Sanic 19.12.

[adrian-the-git]

Another consequence of this release (returning a tuple of routes, handler from the route decorator) is that multiple route decorators can no longer be used on a single handler.

@app.route('/foo')
@app.route('/bar')
async def handler(request):
    return whatever()

This fails as the object returned by the innermost decorator is a tuple rather than a function and subsequent decorator calls can't handle a tuple. A quick workaround to get things running was a simple instance check, but I think this should be investigated in greater depth, as I have no idea what further implications may exist here.

    def response(handler)
        if isinstance(handler, tuple):
            handler = handler[1]

[ashleysommer]

@adrian-the-git
That problem was already reported in #1742, and fixed in #1764.

[Sanyaorg]

Hi guys,

We have issue with Sanic middleware after switching to version >=19.12.0.
Usually to create a route we use @app decorator (something like):

@app.get('/names')
    async def names(request):
    ...

and handle all CORS issues in middleware:

@app.middleware("request")
    async def cors_options(request):
        if request.method == "OPTIONS":

After switching to version >=19.12.0 there is an error "error occured: Method OPTIONS not allowed for URL" (and for some reason I can't catch call in middleware).
Adding OPTIONS will fix the problem:

@app.route('/names', methods=["OPTIONS", "GET"])
    async def names(request):
    ...

but it requires reviewing all endpoints and adding extra functionality.

Did we miss something or the whole approach for creating routes and handling CORS and OPTIONS is not correct?

Thank you

[ashleysommer]

Hi @Sanyaorg
Yes, everyone on Sanic 19.12+ is having the same problem.
It is because (as mentioned in 1st comment of this issue), all request-middlewares now run _after_ the router, not before. That means the router has a chance to throw a method-not-allowed error or a not-found-error _before_ any request-middleware was run. So there's no way of doing any automatic CORS handling in a middleware. Who is to say this is any more or less "correct" logic than what we had before. It is just different.

This turned out to be a much bigger breaking change than was intended when the author made the change and we approved and merged it.

In the Sanic-CORS plugin, I worked around this by adding extra logic to the exception handler code.
Sanic-CORS already had logic in place to catch SanicExceptions, in order to apply CORS headers to them before they are sent to the user. I added the ability to detect the specific conditions: 1) Running on Sanic >= 19.12.0, 2) MethodNotAllowed Exception thrown, 3) Request method is "OPTIONS", 4) automatic_options is enabled in Sanic-CORS. If those conditions are met, a proper CORS response is returned rather than the MethodNotAllowed Exception.

EDIT:
There has been talk about adding another type of middleware than runs before the router, something along the lines of "pre-router-middleware", but that might conflict with the plans we have around adding signals (with extensibility using signal-hooks and signal-handlers) in the future.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

[qwesda]

this issue doesn't seem to be resolved. Has there been any consensus reached about this should be handled?

• introduce a new "pre-router-middleware"
• explicitly not support the use case of dynamically changing the route in the middleware (i.e. any kind of rewriting feature)
• revert to the the old behaviour

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.

[ahopkins]

This will be patched in 21.3, and then resolved with the release of signals later this year.