Skip to content

Bug: UnboundLocalError in Exception Handler for Duplicate Subscriptions (graphql-ws protocol) #119

@Almonok

Description

@Almonok

An UnboundLocalError occurs in the _on_gql_subscribe exception handler when a duplicate subscription is attempted using the legacy graphql-ws protocol. The variable op_name is referenced in the exception handler before it has been defined, masking the original GraphQLError and preventing proper error handling. Instead we get the server-side error:

UnboundLocalError: cannot access local variable 'op_name' where it is not associated with a value

Affected Code

File: channels_graphql_ws/graphql_ws_consumer.py
Method: GraphqlWsConsumer._on_gql_subscribe (lines 700-917)
Protocol: Legacy graphql-ws protocol only (not graphql-transport-ws)

Reproduction Steps

  1. Establish a WebSocket connection using the graphql-ws subprotocol.
  2. Send a subscription request with a specific op_id.
  3. Send another subscription request with the same op_id.
  4. Expected: Proper GraphQL error response for duplicate subscription.
  5. Actual: UnboundLocalError occurs because op_name is undefined in the exception handler.

Current Code (Problematic)

async def _on_gql_subscribe(self, op_id, payload):
    try:
        # Line 712: Early check for duplicate subscription
        if op_id in self._subscriptions:
            if self._graphql_ws_subprotocol == "graphql-transport-ws":
                await self.close(code=4409)
                return

            # Line 726-727: GraphQLError raised
            message = f"Subscription with msg_id={op_id} already exists!"
            raise graphql.error.GraphQLError(message)

        # Line 731: op_name defined AFTER the above check
        query = payload["query"]
        op_name = payload.get("operationName")  # <-- First definition
        variables = payload.get("variables", {})

       ....

    except Exception as ex:
        if (
            isinstance(ex, graphql.error.GraphQLError)
            and self._graphql_ws_subprotocol == "graphql-ws"
        ):
            # Line 910: Error occurs here
            LOG.warning(
                "GraphQL error! Operation %s(%s).", op_name, op_id, exc_info=True
            )
            await self._send_gql_next(op_id, None, [ex])
            await self._send_gql_complete(op_id)

Proposed Solution

Initialize op_name early in the try block to ensure it's defined before any exceptions can occur:

async def _on_gql_subscribe(self, op_id, payload):
    try:
        # Initialize op_name early
        op_name = None

        if op_id in self._subscriptions:
            if self._graphql_ws_subprotocol == "graphql-transport-ws":
                await self.close(code=4409)
                return

            message = f"Subscription with msg_id={op_id} already exists!"
            raise graphql.error.GraphQLError(message)

        # Extract op_name from payload
        query = payload["query"]
        op_name = payload.get("operationName")
        variables = payload.get("variables", {})

        ... 

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions