Release completed operations in the legacy graphql-ws handler

[patrick91]

Subscriptions that completed on their own (or failed before execution)
stayed in the handler's bookkeeping until the client sent a stop message,
reused the operation id, or disconnected. With
max_subscriptions_per_connection configured, a client using distinct
operation ids could hit 'Subscription limit reached' with no active
subscriptions. Release the operation's slot when its task finishes, and
make cleanup_operation tolerant of operations that already released
themselves so that a late 'stop' is a no-op.

Fixes GHSA-m952-2w3f-6r8h

Claude-Session: https://claude.ai/code/session_01HjtoZFcVXzJNwkytcgVp4v

Summary by Sourcery

Ensure the legacy graphql-ws handler releases subscription capacity when operations finish.

Bug Fixes:

• Release completed or pre-execution-failed operations in the legacy graphql-ws handler so they no longer consume connection subscription slots.
• Make stopping an operation that has already finished a no-op.

Tests:

• Add coverage for completed and failed subscriptions releasing max_subscriptions_per_connection capacity.

[github-actions]

Thanks for adding the RELEASE.md file!

Below is the changelog that will be used for the release.

---

This release fixes the legacy graphql-ws protocol handler so that
subscriptions which complete on their own (or fail before execution) release
their slot on the connection.

Previously, completed operations were kept in the handler's bookkeeping until
the client sent a stop message for them, reused their operation id, or
disconnected. On connections with max_subscriptions_per_connection
configured, a client using distinct operation ids could therefore hit
Subscription limit reached even though none of its earlier subscriptions
were still active. The graphql-transport-ws handler was not affected.

Sending a stop message for an operation that has already completed is now a
no-op instead of an error.

This release was contributed by @patrick91 in #4610

[sourcery-ai]

Sorry @patrick91, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 1 day and 8 hours by commenting @sourcery-ai review. Upgrade to get a review now.

[greptile-apps]

Greptile Summary

This PR releases completed legacy graphql-ws operations from per-connection bookkeeping and makes late stop messages idempotent.

• Adds self-release when an operation task finishes.
• Makes explicit cleanup tolerant of operations that already released themselves.
• Adds regression coverage for completed and pre-execution-failed operations under subscription limits.
• Adds patch-release notes describing the corrected capacity accounting.

Confidence Score: 4/5

The disconnect-time source cleanup regression should be fixed before merging; the test-helper inconsistency is non-blocking.

A result-delivery exception can now remove an active source from both bookkeeping maps before connection cleanup calls aclose, delaying or skipping application resource release.

Files Needing Attention: strawberry/subscriptions/protocols/graphql_ws/handlers.py, tests/websockets/test_graphql_ws.py

Important Files Changed

Filename	Overview
strawberry/subscriptions/protocols/graphql_ws/handlers.py	Releases finished operation bookkeeping and makes cleanup idempotent, but can discard a still-live source before disconnect cleanup closes it when result delivery raises.
tests/websockets/test_graphql_ws.py	Adds limit-release regression tests for normal completion and validation failure, while retaining protocol-specific sending helpers contrary to the repository test convention.
RELEASE.md	Clearly documents the legacy graphql-ws capacity-accounting fix and late-stop behavior.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Handler as Legacy graphql-ws handler
    participant Task as Operation task
    participant Source as Result source
    Client->>Handler: start(id)
    Handler->>Task: create task
    Task->>Source: iterate results
    Source-->>Task: result
    Task-->>Client: data
    alt Normal completion
        Source-->>Task: exhausted
        Task-->>Client: complete
        Task->>Handler: remove source and task entries
    else Stop
        Client->>Handler: stop(id)
        Handler->>Source: aclose()
        Handler->>Task: cancel and await
        Task->>Handler: remove entries if present
    else Send fails during disconnect
        Task--xClient: send raises
        Task->>Handler: remove source and task entries
        Note over Handler,Source: Connection cleanup can no longer find the live source
    end

Loading

_{Reviews (1): Last reviewed commit: "Release completed operations in the lega..." | Re-trigger Greptile}

[greptile-apps]

Disconnect drops live result source

When result delivery raises during a client disconnect, this finally removes the live result source before connection cleanup can call aclose(), causing subscription listeners and generator resources to remain active until nondeterministic garbage-collection finalization.

Knowledge Base Used: Subscriptions and streaming

[greptile-apps]

Protocol-specific test API usage

The added tests use send_legacy_message instead of the repository-required unified test-client sending API, reinforcing divergent cross-protocol test interfaces and increasing maintenance cost.

Rule Used: Unify the test client API calls (send_json vs send... (source)

Learned From
strawberry-graphql/strawberry#3868

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[botberry]

This PR was published as 0.327.2. Thank you for contributing!

[codspeed-hq]

Merging this PR will not alter performance

✅ 34 untouched benchmarks

---

_{Comparing fix/graphql-ws-release-completed-operations (cb02f51) with main (6faab98)}