ASGI WebSocket connections’ scope issue: KeyError: 'subprotocols'

[ancs21]

Describe the Bug

I use the granian HTTP Server support ASGI and get a issue below

  File "/usr/local/lib/python3.11/site-packages/strawberry/channels/handlers/base.py", line 88, in dispatch
    await super().dispatch(message)
  File "/usr/local/lib/python3.11/site-packages/channels/consumer.py", line 73, in dispatch
    await handler(message)
  File "/usr/local/lib/python3.11/site-packages/channels/generic/websocket.py", line 173, in websocket_connect
    await self.connect()
  File "/usr/local/lib/python3.11/site-packages/strawberry/channels/handlers/ws_handler.py", line 77, in connect
    preferred_protocol = self.pick_preferred_protocol(self.scope["subprotocols"])
                                                      ~~~~~~~~~~^^^^^^^^^^^^^^^^
KeyError: 'subprotocols'

As mentioned by @gi0baro here https://github.com/emmett-framework/granian/issues/213#issuecomment-1952012787

System Information

• Operating system: MacOS
• Strawberry version (if applicable): 0.219.2

Upvote & Fund

• We're using Polar.sh so you can upvote and help fund this issue.
• We receive the funding once the issue is completed & confirmed by you.
• Thank you in advance for helping prioritize & fund our backlog.

[DoctorJohn]

Thanks for reporting @ancs21! We can adjust the code to handle missing subprotocols more gracefully.

However, I'm not sure whether we want to reject the websocket connection in this case or pick one of the two graphql websocket protocols. While both graphql websocket protocols name the subprotocol associated with them, I'm not sure whether they're strictly required. Need to look around and check how other implementations handle this case since the specs are not clear about it.

[gi0baro]

@DoctorJohn Granian maintainer here. I think in the case you can't fallback to a default value for this, the best approach would be to reject the connection, as the lack of subprotocols optional key in ASGI scope just means the server is not implementing such feature.

As a side note, there's a specific issue in Granian to add subprotocols support, thus in the future this will be implicitly solved on Granian side, but I really think Strawberry should respect ASGI standard here :)

[DoctorJohn]

This has been addressed in #3638. Connections are now getting rejected if client and server fail to negotiate a subprotocol.