fix: use server.close() for SSE reconnection and guard connect() failure #36

Merged

elvis merged 1 commit from fix-sse-reconnection-robust into main

2026-06-01 20:39:39 +00:00

elvis commented

2026-06-01 20:36:06 +00:00

Owner

Summary

Use server.close() instead of transport.close() — the SDK's Protocol.connect() throws if this._transport is still set. _transport is only cleared inside Protocol._onclose(), which fires via the transport's onclose callback. Calling transport.close() directly risks leaving _transport set if close() throws before onclose fires; server.close() is the correct SDK cleanup path.
Log errors on close instead of silently swallowing them with catch {}.
Guard server.connect() with a try/catch that returns HTTP 500 and clears the local transport ref if the connect fails, rather than letting the SSE response hang indefinitely.
Add two reconnection tests in src/server.test.ts using InMemoryTransport: explicit server.close() → re-connect, and client-initiated disconnect → re-connect.

Test plan

npm test — all 130 tests pass including the two new reconnection tests
Manual SSE smoke-test: connect two clients sequentially in SSE mode, verify the second connection succeeds and tools respond correctly

## Summary - **Use `server.close()` instead of `transport.close()`** — the SDK's `Protocol.connect()` throws if `this._transport` is still set. `_transport` is only cleared inside `Protocol._onclose()`, which fires via the transport's `onclose` callback. Calling `transport.close()` directly risks leaving `_transport` set if `close()` throws before `onclose` fires; `server.close()` is the correct SDK cleanup path. - **Log errors on close** instead of silently swallowing them with `catch {}`. - **Guard `server.connect()`** with a try/catch that returns HTTP 500 and clears the local transport ref if the connect fails, rather than letting the SSE response hang indefinitely. - **Add two reconnection tests** in `src/server.test.ts` using `InMemoryTransport`: explicit `server.close()` → re-connect, and client-initiated disconnect → re-connect. ## Test plan - [ ] `npm test` — all 130 tests pass including the two new reconnection tests - [ ] Manual SSE smoke-test: connect two clients sequentially in SSE mode, verify the second connection succeeds and tools respond correctly

elvis added 1 commit

2026-06-01 20:36:06 +00:00

fix: use server.close() for SSE reconnection and guard connect() failure

Run Unit Tests / test (pull_request) Successful in 2m34s

Details

27b6f06497

- Replace transport.close() with server.close() so the SDK's full
  cleanup path runs (Protocol._onclose clears _transport via the
  onclose callback chain); calling transport.close() directly risks
  leaving _transport set if close() throws before onclose fires,
  which would cause the next server.connect() to throw
- Log errors on close instead of silently swallowing them
- Wrap server.connect() in try/catch to return HTTP 500 and clear
  the transport ref rather than leaving the SSE response hanging
- Add two reconnection tests: explicit server.close() → re-connect,
  and client-initiated disconnect → re-connect

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

elvis merged commit 3bf2da7b5d into main

2026-06-01 20:39:39 +00:00

elvis deleted branch fix-sse-reconnection-robust

2026-06-01 20:39:40 +00:00

elvis referenced this pull request from a commit

2026-06-01 20:39:41 +00:00

Merge pull request 'fix: use server.close() for SSE reconnection and guard connect() failure' (#36) from fix-sse-reconnection-robust into main