Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uvicorn error: Expected ASGI message 'http.response.start', but got 'websocket.close'. #210

Closed
eirikur-grid opened this issue Jan 13, 2021 · 2 comments · Fixed by RC-MODULE/rumboot-tools#10
Assignees
Labels

Comments

@eirikur-grid
Copy link

Steps to Reproduce

  1. Create a bare-bones socket.io server using uvicorn (0.13.3) and python-socketio (5.0.4)
# app.py
import socketio

sio = socketio.AsyncServer(async_mode="asgi")
app = socketio.ASGIApp(sio, socketio_path="/socket.io")
  1. Start the server: uvicorn app:app
  2. Issue a bad HTTP GET request that still includes a Sec-Websocket-Version: header. For example:
curl -v "http://127.0.0.1:8000/socket.io/?EIO=4&transport=websocket" -H "Sec-Websocket-Version: 13"

Observed Behavior

The server returns HTTP status 500 - Internal Server Error
Uvicorn logs the following error message:

ERROR:    Exception in ASGI application
Traceback (most recent call last):
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/uvicorn/protocols/http/h11_impl.py", line 394, in run_asgi
    result = await app(self.scope, self.receive, self.send)
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/uvicorn/middleware/proxy_headers.py", line 45, in __call__
    return await self.app(scope, receive, send)
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/engineio/async_drivers/asgi.py", line 54, in __call__
    await self.engineio_server.handle_request(scope, receive, send)
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/socketio/asyncio_server.py", line 383, in handle_request
    return await self.eio.handle_request(*args, **kwargs)
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/engineio/asyncio_server.py", line 308, in handle_request
    return await self._make_response(r, environ)
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/engineio/asyncio_server.py", line 371, in _make_response
    response = await make_response(
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/engineio/async_drivers/asgi.py", line 198, in make_response
    await environ['asgi.send']({'type': 'websocket.close'})
  File "/Users/eirikur/Projects/uvicorn-socketi-error-repro/.venv/lib/python3.9/site-packages/uvicorn/protocols/http/h11_impl.py", line 447, in send
    raise RuntimeError(msg % message_type)
RuntimeError: Expected ASGI message 'http.response.start', but got 'websocket.close'.

Expected Behavior

Server returns HTTP status code 400, just like it does if the Sec-Websocket-Version header is missing.
No error in uvicorn.

Proposed Solution

While I'm not very familiar with the ASGI spec, I believe the problem lies in the make_response function in async_drivers/asgi.py, where the presence of the Sec-Websocket-Version HTTP header determines the course of action.

if 'HTTP_SEC_WEBSOCKET_VERSION' in environ:

I would expect that the type of the asgi.scope should be inspected instead, e.g. if environ['asgi.scope']['type'] == 'websocket':

@eirikur-grid
Copy link
Author

eirikur-grid commented Jan 13, 2021

Why is this a problem?

We sometimes get bad requests from:

a) users behind content security gateways/proxies (e.g. websense) that don't properly support websockets
b) Facebook bots, masquerading as IOS devices

The unhandled exceptions and 500 status codes trigger alarms in our monitoring systems that we manually need to silence/ignore. This increases the risk that we miss critical issues.

@miguelgrinberg miguelgrinberg self-assigned this Jan 13, 2021
@miguelgrinberg
Copy link
Owner

Okay, I'll see if I can make it respond with the expected 400 here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants