Skip to content

Fix server.close recursion and stale WebSocket ref after close - #194

Open
saiyaangoku64 wants to merge 1 commit into
BrowserMCP:mainfrom
saiyaangoku64:fix/server-close-and-context-ws-reset
Open

Fix server.close recursion and stale WebSocket ref after close#194
saiyaangoku64 wants to merge 1 commit into
BrowserMCP:mainfrom
saiyaangoku64:fix/server-close-and-context-ws-reset

Conversation

@saiyaangoku64

Copy link
Copy Markdown

Summary

  • createServerWithTools in src/server.ts reassigns server.close, but the new implementation calls server.close() on itself rather than the original Server.prototype.close. This is self-recursion (infinite loop / stack overflow, or a silent no-op depending on how the SDK binds the method) instead of the intended "close server, then close the websocket server, then close the context" sequence. Fixed by capturing the original close via .bind(server) before overriding.
  • Context.close() in src/context.ts awaits this._ws.close() but never resets _ws to undefined afterward, so hasWs() keeps returning true for a socket that's already closed.

Test plan

This repo's devDependencies reference private workspace:* packages (@repo/config, @repo/messaging, @repo/types, @repo/utils, @r2r/messaging) that only exist in the internal monorepo, so npm install / npm run build / npm run typecheck cannot be run from a fork (confirmed: npm install fails with EUNSUPPORTEDPROTOCOL on the workspace:* specifiers, matching the limitation noted in the README).

  • Verified the diff is a minimal, self-contained change to the two files, with no other logic altered.
  • Ran tsc --noEmit directly against src/context.ts and src/server.ts in isolation; the only errors reported are unresolved-module errors for the @repo/*/@r2r/* workspace packages and node_modules (expected, since neither exists outside the monorepo) — no syntax or type errors on the changed lines.
  • Given the build limitation, would appreciate a maintainer running this through the internal monorepo's typecheck/build before merge.

createServerWithTools reassigned server.close to a function that called
server.close() on itself, causing infinite recursion (or a silent no-op,
depending on how the SDK binds the method) instead of calling the original
Server.prototype.close. Capture the original close via bind before
overriding it.

Context.close() also never cleared _ws after closing the socket, so
hasWs() continued reporting true for an already-closed connection.
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