on_ws_connect return handling

[tjcutajar]

Hi there,

I noticed this small difference between the return handling of the new on_ws_connect in graphql_transport_ws vs graphql_ws, notably a None value is not handled in graphql_transport_ws:

strawberry/strawberry/subscriptions/protocols/graphql_transport_ws/handlers.py

Line 193 in 633c9bc

if isinstance(connection_ack_payload, UnsetType):

Is there a specific reason for this, or could they be made the same?

Thanks.

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]

Hi, as stated in our integrations' docs, this is intentional.

notably a None value is not handled in graphql_transport_ws

It is handled, however, differently than for the older subprotocol.

The reason for this is that in the newer subprotocol a payload can either be undefined (strawberry.UNSET in Python), the null JSON value (None in Python), or a JSON object (dict in Python). In the older subprotocol, however, a JSON value of null (None) is not valid.

For compatibility reasons, if you return None from on_ws_connect it will be mapped to a null JSON value in the new subprotocol, while for the old subprotocol it will be handled as if the payload was undefined.

[tjcutajar]

@DoctorJohn Thanks for your feedback.

With this explanation, should the None check then in fact be in the graphql_transport_ws handler? At the moment it returns
{"type": "connection_ack", "payload": null} when returning None from on_ws_connect with graphql_transport_ws.

[DoctorJohn]

graphql_transport_ws is the handler for the graphql-transport-ws WebSocket subprotocol, which is the newer GraphQL over WS protocol. Unfortunately, it lives in a repository called graphql-ws which causes a lot of confusion, because that's also the name of the legacy WebSocket subprotocol (which in turn lives in a repository called subscriptions-transport-ws causing even more confusion).

[tjcutajar]

@DoctorJohn lol ok, thanks for clarifying.