The response for async requests may be returned as None

[joente]

Describe the bug The handle_request(..) function in async_server.py may return None instead of a valid response type.

To Reproduce Steps to reproduce the behavior:

1. Version engine IO: 4.8.0
2. Set-up socket connection

Expected behavior I would expect the handle_request() function to return with a valid Response, not None.

Logs The following logs are when using socket.io in combination with aiohttp and aiohttp-sessions:

[E 231023 12:49:09 web_protocol:403] Error handling request
Traceback (most recent call last):
  File "/usr/local/lib/python3.9/site-packages/aiohttp/web_protocol.py", line 433, in _handle_request
    resp = await request_handler(request)
  File "/usr/local/lib/python3.9/site-packages/aiohttp/web_app.py", line 504, in _handle
    resp = await handler(request)
  File "/usr/local/lib/python3.9/site-packages/aiohttp/web_middlewares.py", line 117, in impl
    return await handler(request)
  File "/code/lib/app.py", line 121, in middleware_handler
    return await handler(request)
  File "/usr/local/lib/python3.9/site-packages/aiohttp_session/__init__.py", line 204, in factory
    raise RuntimeError(f"Expect response, not {type(response)!r}")
RuntimeError: Expect response, not <class 'NoneType'>

Additional context The problem is with this code (line 280, async_server.py):

                        packets = await socket.handle_get_request(environ)
                        if isinstance(packets, list):
                            r = self._ok(packets, jsonp_index=jsonp_index)
                        else:
                            r = packets  # <-- packets can be None

Followed by this code (line 314, async_server.py):

        if not isinstance(r, dict):
            return r  # <-- Can be None when packets was None

The reason is the following commit: https://github.com/miguelgrinberg/python-engineio/commit/785c77c39d8c8fee37f969981f5a28ed2cc1b769

The remove of return at line 314 in async_socket.py

    ws = self.server._async['websocket'](
            self._websocket_handler, self.server)
    return await ws(environ)  # this `return` is required

which get called by handle_get_request(..):

        if 'upgrade' in connections and transport in self.upgrade_protocols:
            self.server.logger.info('%s: Received request to upgrade to %s',
                                    self.sid, transport)
            return await getattr(self, '_upgrade_' + transport)(environ)

[miguelgrinberg]

What version of aiohttp are you using? With the latest version I do not reproduce the error you reported.

Edit: you are using aiohttp-session, that may be why I don't see the error.

[joente]

What version of aiohttp are you using? With the latest version I do not reproduce the error you reported.

Edit: you are using aiohttp-session, that may be why I don't see the error.

The error occurs when using aiohttp-sessions because it uses middleware which checks the response. (version 2.12.0) in combination with aiohttp (version 3.8.6). The lib aiohttp_session contains this code in the __init__.py:

        if not isinstance(response, (web.StreamResponse, web.HTTPException)):
            raise RuntimeError(f"Expect response, not {type(response)!r}")

I don't think this code is wrong. I think the response should not be None, or is this intentionally?

For now I did patch the code to solve the issue:

from engineio.async_socket import AsyncSocket

async def _upgrade_websocket(self, environ):
    """Upgrade the connection from polling to websocket."""
    if self.upgraded:
        raise IOError('Socket has been upgraded already')
    if self.server._async['websocket'] is None:
        # the selected async mode does not support websocket
        return self.server._bad_request()
    ws = self.server._async['websocket'](
        self._websocket_handler, self.server)
    return await ws(environ)  # added return to solve RunTime exception by aiohttp-session

# patch engineio version 4.8.0
# https://github.com/miguelgrinberg/python-engineio/issues/332
AsyncSocket._upgrade_websocket = _upgrade_websocket

[joente]

I think another solution would be to change the code in async_socket.py:

        if 'upgrade' in connections and transport in self.upgrade_protocols:
            self.server.logger.info('%s: Received request to upgrade to %s',
                                    self.sid, transport)
            return await getattr(self, '_upgrade_' + transport)(environ)

Into something like this:

        if 'upgrade' in connections and transport in self.upgrade_protocols:
            self.server.logger.info('%s: Received request to upgrade to %s',
                                    self.sid, transport)
            await getattr(self, '_upgrade_' + transport)(environ)
            return self.server._ok()

Note: I haven't tested the above yet, but I'm happy to do so if needed.

[miguelgrinberg]

@joente this is a WebSocket request. There is no response. The response was returned during the connection handshake and it was a 101 response. Returning a response for a completed WebSocket request caused problems for other frameworks. I will see if I can have a generic response added there. I will also look for other examples that use aiohttp-session with WebSocket requests to see how this was handled.

Edit: I also see that I have been inconsistent and I do have a _ok() return on the sync version of this code, which did not appear to cause problems. So I will also try what you suggested above and make sure it does not upset other async frameworks I support (asgi, tornado, etc.)