Skip to content

Fix: make StreamableHTTP#close and Native::Client#stop exception-safe - #156

Open
raySavignone wants to merge 1 commit into
patvice:mainfrom
raySavignone:fix/exception-safe-teardown
Open

Fix: make StreamableHTTP#close and Native::Client#stop exception-safe#156
raySavignone wants to merge 1 commit into
patvice:mainfrom
raySavignone:fix/exception-safe-teardown

Conversation

@raySavignone

Copy link
Copy Markdown

Problem

Client teardown isn't exception-safe in two places, so a failing session DELETE strands resources. For hosts that build short-lived StreamableHTTP clients inside a long-running process, these accumulate — leaked SSE threads, HTTPX clients, and per-client HumanInTheLoopRegistry entries (each of which owns a live scheduler thread).

  1. StreamableHTTP#close runs terminate_session → cleanup_sse_resources → cleanup_connection with no ensure. terminate_session raises a TransportError on a failed session DELETE (5xx / network error), skipping the other two steps.
  2. Native::Client#stop releases the registry (and resets state) only after @transport.close, so a raising close skips it.

Fix

Wrap the teardown in ensure in both methods so the remaining cleanup always runs, while the original error still propagates. This matches the gem's existing defensive pattern — close_client, close_all_clients, and HumanInTheLoopRegistry#shutdown already use rescue/ensure.

Tests

Added specs that simulate a raising session teardown and assert the SSE/HTTPX resources and the per-client registry are released, and that the original error still propagates.

Wrap teardown in ensure so cleanup always runs when session termination
raises, while the original error still propagates. Prevents leaked SSE
threads, HTTPX clients, and per-client HumanInTheLoopRegistry entries in
long-lived processes that build short-lived StreamableHTTP clients.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant