Skip to content

Implement cancellation in ReadableStream handling - #1

Open
wardpeet wants to merge 1 commit into
mhart:mainfrom
wardpeet:patch-1
Open

Implement cancellation in ReadableStream handling#1
wardpeet wants to merge 1 commit into
mhart:mainfrom
wardpeet:patch-1

Conversation

@wardpeet

Copy link
Copy Markdown

Add cancellation support to the ReadableStream in the HTTP server.

Related issue mastra-ai/mastra#13899

Add cancellation support to the ReadableStream in the HTTP server.
@roaminro

roaminro commented Aug 3, 2026

Copy link
Copy Markdown

I tested this patch against a deterministic MCP Streamable-HTTP repro and found one additional cancellation-propagation requirement. The proposed cancelled guard prevents the fatal ERR_INVALID_STATE, but cancel() only sets a local flag; it does not notify consumers of the simulated Node ServerResponse.

In this path, @hono/node-server is piping an inner Web ReadableStream into FetchServerResponse and listens for writable.on("close", () => reader.cancel()). Without a close event, its reader is never cancelled, so @modelcontextprotocol/sdk never runs its inner ReadableStream.cancel() and its 15-second SSE keepalive interval remains orphaned, writing forever into the now-discarded response.

Adding close propagation in addition to this PR's guards fixed both the crash and the timer leak:

cancel() {
  cancelled = true;
  _this.emit("close");
}

(or an equivalent destroy/close implementation that emits close). One already-in-flight buffered write can land in the same millisecond as cancellation, so the existing cancelled guard/try-catch is still necessary.

Proof: one MCP client connect+disconnect kills the unpatched server exactly at the SDK's 15,000ms keepalive. With the guards + close propagation, instrumentation shows outer cancel -> inner MCP cancel, no writes at 15s/30s, and Mastra's full API suite passes 363/363. Full trace: mastra-ai/mastra#20332 (comment)

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.

2 participants