Skip to content

Conversation

@cliffhall
Copy link
Member

@cliffhall cliffhall commented May 28, 2025

Reverts #380

I realized the problem is already be fixed in the #428, and there's no need to have to refactor that one to remove most of this one's changes.

@cliffhall
Copy link
Member Author

cliffhall commented May 28, 2025

@olaservo I merged #380 after having previously fully testing it and including videos of the problem and the fix working locally.

But then I realized that I had jumped the gun. This PR works with the backingServerTransport which is refactored out of existence in #428 when adding support for multiple instances of the client to connect to the same server. Part of that effort was doing away with the single backingServerTransport and having a separate server transport for every web app transport. And that PR handles closing of the client properly, cleaning up both transports.

Here is a video of connecting to an STDIO server with that code, and the server is never crashed by a disconnecting client.

Screen.Recording.2025-05-28.at.1.02.11.PM.mov

@cliffhall cliffhall requested a review from olaservo May 28, 2025 17:12
@olaservo olaservo merged commit 183f801 into main May 28, 2025
5 checks passed
@olaservo olaservo deleted the revert-380-main branch May 28, 2025 17:15
IgnacioC44 referenced this pull request in MCPJam/inspector Jun 21, 2025
Revert "fix: handle the client disconnect so that the server does not crash."
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.

3 participants