fix(lifecycle): bound the session-id probe and release its response body (#455)
The visible-ASCII session-id check sends a raw initialize POST outside the SDK transport, because the transport does not expose the MCP-Session-Id response header. That fetch carried no abort signal and its body was never read or cancelled. A server that accepts the connection and never responds hung the scenario here, before teardown, so the DELETE bound added in #316 never came into play. And since the probe's `Accept` includes `text/event-stream`, a server answering with an open SSE stream left the body unconsumed and leaked the connection for the rest of the run. Bounding the probe at 5s matches the existing session-termination bound. #427 is the structural fix for the hang class; this is still worth having for fast failure and for attributing the failure to this check rather than to whatever the runner-level timeout happens to interrupt. Fixes #428 Co-authored-by: Avi Seth <aviseth@users.noreply.github.com>
This commit is contained in:
parent
1573ad5fc8
commit
50edb98c26
2 changed files with 56 additions and 1 deletions
|
|
@ -140,4 +140,48 @@ describe('ServerInitializeScenario', () => {
|
|||
);
|
||||
expect(deleteCalls).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('bounds the session-id probe so a server that never answers cannot hang it', async () => {
|
||||
fetchMock.mockResolvedValue(new Response(null));
|
||||
|
||||
await new ServerInitializeScenario().run(testContext(serverUrl));
|
||||
|
||||
const probe = fetchMock.mock.calls.find(
|
||||
([, init]) => (init as RequestInit | undefined)?.method === 'POST'
|
||||
);
|
||||
expect(probe?.[1]?.signal).toBeInstanceOf(AbortSignal);
|
||||
});
|
||||
|
||||
it('cancels the probe response body so an SSE answer does not leak the connection', async () => {
|
||||
// `Accept` on the probe includes `text/event-stream`, so a server is free
|
||||
// to answer with a stream that stays open. Only the header is read, so the
|
||||
// body has to be released explicitly.
|
||||
const cancel = vi.fn().mockResolvedValue(undefined);
|
||||
const body = new ReadableStream({
|
||||
start() {
|
||||
// Never enqueue, never close: an SSE stream awaiting its first event.
|
||||
},
|
||||
cancel
|
||||
});
|
||||
|
||||
fetchMock.mockResolvedValueOnce(
|
||||
new Response(body, {
|
||||
headers: {
|
||||
'content-type': 'text/event-stream',
|
||||
'mcp-session-id': 'session-123_ABC'
|
||||
}
|
||||
})
|
||||
);
|
||||
fetchMock.mockResolvedValue(new Response(null));
|
||||
|
||||
const checks = await new ServerInitializeScenario().run(
|
||||
testContext(serverUrl)
|
||||
);
|
||||
|
||||
expect(cancel).toHaveBeenCalled();
|
||||
expect(checks[1]).toMatchObject({
|
||||
id: 'server-session-id-visible-ascii',
|
||||
status: 'SUCCESS'
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -15,6 +15,11 @@ import {
|
|||
|
||||
const VISIBLE_ASCII_REGEX = /^[\x21-\x7E]+$/;
|
||||
|
||||
// The session-id probe only needs the response header. Bound the request so a
|
||||
// server that accepts the connection and never answers fails this check
|
||||
// instead of hanging the scenario before teardown ever runs.
|
||||
const SESSION_ID_PROBE_TIMEOUT_MS = 5000;
|
||||
|
||||
const SESSION_SPEC_REFERENCES = [
|
||||
{
|
||||
id: 'MCP-Session-Management',
|
||||
|
|
@ -115,12 +120,18 @@ and validates session ID format if one is assigned.`;
|
|||
version: '1.0.0'
|
||||
}
|
||||
}
|
||||
})
|
||||
}),
|
||||
signal: AbortSignal.timeout(SESSION_ID_PROBE_TIMEOUT_MS)
|
||||
});
|
||||
|
||||
const sessionId = response.headers.get('mcp-session-id');
|
||||
rawSessionId = sessionId;
|
||||
|
||||
// Only the header is needed. `Accept` includes `text/event-stream`, so a
|
||||
// server answering with an open SSE stream would otherwise leave this
|
||||
// body unconsumed and leak the connection for the rest of the run.
|
||||
await response.body?.cancel();
|
||||
|
||||
if (!sessionId) {
|
||||
checks.push({
|
||||
id: 'server-session-id-visible-ascii',
|
||||
|
|
|
|||
Loading…
Reference in a new issue