engineio HTTP response of [] causes problems in Sanic middleware plugins

[ashleysommer]

Hi, I'm one of the Core-Devs for Sanic, and author of a number of Sanic plugins.

Recently when investigating an issue raised on the issue-tracker for my Sanic-CORS plugin, I came across this slight incompatibility with python-Engineio on Sanic applications.

When a socketio connection is upgraded to 'websocket' transport, the HTTP request that initiates that becomes a long-running connection. When that websocket finally closes, python-engineio returns to the Sanic request-handler with the HTTP response of an empty list ([]).
See here:

python-engineio/engineio/asyncio_server.py

Line 256 in a440498

return r if r is not None else []

The problem is, Sanic checks if the response is None before executing any response-middleware. See here:
https://github.com/huge-success/sanic/blob/d2094fed38d465eebe997b3ff054aa4a36102c2a/sanic/app.py#L980
Because the empty list is not None, Sanic will try to execute response-middleware on [] which most post-response plugins are not expecting, causing many to throw unexpected errors.

So I don't know if this is a problem with Python-Engineio (why does it return [] rather than None?) or a problem with Sanic (should it check for other potential blank responses, in addition to None before attempting to apply response-middleware?) or is it a problem with the post-response plugins (should they check if a response is invalid before manipulating it)?

[ashleysommer]

Adding:
I think the 'proper' way to handle an ended connection like this in Sanic is to raise a asyncio.CancelledError(), this tells Sanic the response is always None and to not to bother sending a HTTP response to the client after that.

[miguelgrinberg]

Thanks for the detailed report. This [] return value comes from the WSGI side of things, looks like it inadvertently made it into the asyncio side. I'll test this with all the supported servers and frameworks, but I believe that statement should be return r for asyncio.

[ashleysommer]

@miguelgrinberg
Thanks for the quick response.
Yes. return r would work to fix the bug in this case (for Sanic), I don't know about other servers and frameworks.