Skip to content

fix: handle SSE transport send errors after SSE disconnects - #1733

Closed
MumuTW wants to merge 1 commit into
modelcontextprotocol:v1/mainfrom
MumuTW:fix/sse-transport-crash-1014
Closed

fix: handle SSE transport send errors after SSE disconnects#1733
MumuTW wants to merge 1 commit into
modelcontextprotocol:v1/mainfrom
MumuTW:fix/sse-transport-crash-1014

Conversation

@MumuTW

@MumuTW MumuTW commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

  • catch rejected webAppTransport.send() promises in the stdio stderr handler
  • keep transport cleanup in .finally() for the MODULE_NOT_FOUND path so cleanup still runs if the SSE client already disconnected
  • avoid unhandled Not connected rejections when multiple SSE connections race and one closes before stderr forwarding completes

Testing

  • npm ci
  • npm run build-server

Fixes #1014.

Supersedes the earlier stale attempt in #1129.

…ontextprotocol#1014)

When the SSE connection is closed (e.g. client disconnects during rapid
reconnection), webAppTransport.send() rejects with "Not connected".
The stderr handler was calling send() without awaiting or catching the
returned promise, causing unhandled rejections that crash the server.

Add .catch() to both send() call sites in the stderr data handler so
that errors from a closed SSE connection are silently ignored instead
of crashing the process. Also move the cleanup logic in the
MODULE_NOT_FOUND branch into .finally() so transports are cleaned up
regardless of whether the notification send succeeded.
@cliffhall
cliffhall changed the base branch from main to v1/main July 28, 2026 03:03
@MumuTW

MumuTW commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Gentle maintainer follow-up: this PR remains mergeable, and the build and Playwright checks are green; GitHub currently shows it blocked only on required review. Is the SSE-disconnect error-handling direction acceptable, or is there a change you would like before review?

@cliffhall

cliffhall commented Jul 31, 2026

Copy link
Copy Markdown
Member

Closing: v1 is deprecated.

Thank you for this contribution, and apologies for the long wait for a response.

v1 will receive security fixes only. We reviewed every open v1 PR for security impact before closing — see the backlog triage in #1819 — and a small number were retained for a final 1.0.5 patch release. This one is a functionality, compatibility, or cleanup change rather than a vulnerability fix, so it is being closed unmerged. This is not a judgment on the quality of the work — it's a consequence of the branch it targets being frozen.

If the underlying problem still exists in v2, we'd genuinely like to know. Please open an issue describing it against v2. Note that we accept external contributions as issues rather than pull requests — maintainers handle design and implementation through a prompt-driven workflow. See CONTRIBUTORS.md.

Thanks again for taking the time to contribute to the Inspector.

@cliffhall cliffhall closed this Jul 31, 2026
@cliffhall cliffhall added the closed-v1-deprecated Closed: v1 is deprecated and accepting security fixes only label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

closed-v1-deprecated Closed: v1 is deprecated and accepting security fixes only

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSE transport crashes with "Not connected" error when multiple connections occur

2 participants