fix(runner): bound server scenarios with a per-scenario timeout (#454)
The client runner has had a 30s per-scenario timeout since it was written, but `runServerConformanceTest` awaited `scenario.run(ctx)` with no bound. A server under test that accepts the TCP connection and then never answers an initialize POST, a tools call, or an SSE read stalled the entire suite run, not just its own scenario. #316 bounded the session-termination DELETEs in teardown, but that is a call-site fix: covering the rest that way needs the same treatment on every raw fetch in every scenario. Bounding at the runner makes those per-call bounds defense-in-depth instead of the only protection. `--timeout` now applies to server mode too, with the same 30s default as client mode. On expiry the scenario reports a `scenario-timeout` FAILURE naming the elapsed bound, so a wedged server reads as a failing scenario rather than a hung process. The timed-out scenario promise is left pending deliberately — a read blocked on a socket has no cancellation channel — with its rejection swallowed so a late failure can't surface against whichever scenario is running by then. Fixes #427 Co-authored-by: Avi Seth <aviseth@users.noreply.github.com>
This commit is contained in:
parent
31c5e7aa55
commit
88ac3e595f
3 changed files with 105 additions and 4 deletions
|
|
@ -555,11 +555,13 @@ program
|
||||||
'--requirements <revision>',
|
'--requirements <revision>',
|
||||||
'Run exactly the scenarios a spec revision requires, frozen at its release (e.g. 2026-07-28). Replaces --suite and --spec-version'
|
'Run exactly the scenarios a spec revision requires, frozen at its release (e.g. 2026-07-28). Replaces --suite and --spec-version'
|
||||||
)
|
)
|
||||||
|
.option('--timeout <ms>', 'Per-scenario timeout in milliseconds', '30000')
|
||||||
.option('--verbose', 'Show verbose output (JSON instead of pretty print)')
|
.option('--verbose', 'Show verbose output (JSON instead of pretty print)')
|
||||||
.action(async (options, cmd) => {
|
.action(async (options, cmd) => {
|
||||||
try {
|
try {
|
||||||
// Validate options with Zod
|
// Validate options with Zod
|
||||||
const validated = ServerOptionsSchema.parse(options);
|
const validated = ServerOptionsSchema.parse(options);
|
||||||
|
const timeout = parseInt(options.timeout, 10);
|
||||||
|
|
||||||
const verbose = options.verbose ?? false;
|
const verbose = options.verbose ?? false;
|
||||||
const outputDir = options.outputDir;
|
const outputDir = options.outputDir;
|
||||||
|
|
@ -588,7 +590,8 @@ program
|
||||||
validated.scenario,
|
validated.scenario,
|
||||||
outputDir,
|
outputDir,
|
||||||
specVersionFilter,
|
specVersionFilter,
|
||||||
options.force ?? false
|
options.force ?? false,
|
||||||
|
timeout
|
||||||
);
|
);
|
||||||
|
|
||||||
// Inapplicable scenario/spec-version combination (already logged by
|
// Inapplicable scenario/spec-version combination (already logged by
|
||||||
|
|
@ -675,7 +678,8 @@ program
|
||||||
specVersionFilter,
|
specVersionFilter,
|
||||||
// a requirement set decides membership, so its choice outranks a
|
// a requirement set decides membership, so its choice outranks a
|
||||||
// scenario's own applicability window at the pinned revision
|
// scenario's own applicability window at the pinned revision
|
||||||
Boolean(requirements)
|
Boolean(requirements),
|
||||||
|
timeout
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
allResults.push({ scenario: scenarioName, checks: result.checks });
|
allResults.push({ scenario: scenarioName, checks: result.checks });
|
||||||
|
|
|
||||||
|
|
@ -53,6 +53,52 @@ describe('runServerConformanceTest spec-version applicability', () => {
|
||||||
}, 60000);
|
}, 60000);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('runServerConformanceTest per-scenario timeout', () => {
|
||||||
|
// A server that completes the TCP handshake and then never writes a byte.
|
||||||
|
// Before the runner bounded `scenario.run`, this hung the whole suite: the
|
||||||
|
// scenario had no timeout of its own, so nothing downstream ever ran.
|
||||||
|
let server: http.Server;
|
||||||
|
let url: string;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
server = http.createServer(() => {
|
||||||
|
// Deliberately never respond, and never destroy the socket.
|
||||||
|
});
|
||||||
|
await new Promise<void>((resolve) =>
|
||||||
|
server.listen(0, '127.0.0.1', resolve)
|
||||||
|
);
|
||||||
|
const port = (server.address() as AddressInfo).port;
|
||||||
|
url = `http://127.0.0.1:${port}/mcp`;
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(async () => {
|
||||||
|
server.closeAllConnections();
|
||||||
|
await new Promise<void>((resolve) => server.close(() => resolve()));
|
||||||
|
});
|
||||||
|
|
||||||
|
test('fails the scenario instead of hanging when the server never responds', async () => {
|
||||||
|
const start = Date.now();
|
||||||
|
const result = await runServerConformanceTest(
|
||||||
|
url,
|
||||||
|
'server-initialize',
|
||||||
|
undefined,
|
||||||
|
undefined,
|
||||||
|
false,
|
||||||
|
1000
|
||||||
|
);
|
||||||
|
const elapsed = Date.now() - start;
|
||||||
|
|
||||||
|
const timeoutCheck = result.checks.find((c) => c.id === 'scenario-timeout');
|
||||||
|
expect(timeoutCheck).toBeDefined();
|
||||||
|
expect(timeoutCheck?.status).toBe('FAILURE');
|
||||||
|
expect(timeoutCheck?.errorMessage).toContain('1000ms');
|
||||||
|
|
||||||
|
// The bound is what makes this a failure rather than a stall; without it
|
||||||
|
// the call never returns and this assertion is never reached.
|
||||||
|
expect(elapsed).toBeLessThan(15000);
|
||||||
|
}, 30000);
|
||||||
|
});
|
||||||
|
|
||||||
describe('runServerConformanceTest wire selection for draft-only scenarios', () => {
|
describe('runServerConformanceTest wire selection for draft-only scenarios', () => {
|
||||||
// Regression: the CLI used to silently emit the legacy initialize+session
|
// Regression: the CLI used to silently emit the legacy initialize+session
|
||||||
// wire when running a draft-only scenario, producing requests with no
|
// wire when running a draft-only scenario, producing requests with no
|
||||||
|
|
|
||||||
|
|
@ -27,12 +27,59 @@ function formatMarkdown(text: string): string {
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Bound `scenario.run` so a server that accepts connections but never answers
|
||||||
|
* fails one scenario instead of stalling the whole suite.
|
||||||
|
*
|
||||||
|
* The losing promise is left pending on purpose: a scenario blocked on a socket
|
||||||
|
* read has no cancellation channel, so there is nothing to await. Its rejection
|
||||||
|
* is swallowed to keep a late failure from surfacing as an unhandled rejection
|
||||||
|
* against whichever scenario happens to be running by then.
|
||||||
|
*/
|
||||||
|
async function runScenarioBounded(
|
||||||
|
run: Promise<ConformanceCheck[]>,
|
||||||
|
scenarioName: string,
|
||||||
|
timeout: number
|
||||||
|
): Promise<ConformanceCheck[]> {
|
||||||
|
const timedOut = Symbol('timed-out');
|
||||||
|
let timeoutHandle: NodeJS.Timeout | undefined;
|
||||||
|
|
||||||
|
run.catch(() => {});
|
||||||
|
|
||||||
|
const result = await Promise.race([
|
||||||
|
run,
|
||||||
|
new Promise<typeof timedOut>((resolve) => {
|
||||||
|
timeoutHandle = setTimeout(() => resolve(timedOut), timeout);
|
||||||
|
})
|
||||||
|
]);
|
||||||
|
clearTimeout(timeoutHandle);
|
||||||
|
|
||||||
|
if (result !== timedOut) {
|
||||||
|
return result;
|
||||||
|
}
|
||||||
|
|
||||||
|
console.log(`\nScenario timed out after ${timeout}ms`);
|
||||||
|
return [
|
||||||
|
{
|
||||||
|
id: 'scenario-timeout',
|
||||||
|
name: 'Scenario completes within the timeout',
|
||||||
|
description:
|
||||||
|
'The scenario must finish within the configured timeout. A server that ' +
|
||||||
|
'accepts the connection but never responds leaves it running forever.',
|
||||||
|
status: 'FAILURE',
|
||||||
|
timestamp: new Date().toISOString(),
|
||||||
|
errorMessage: `Scenario '${scenarioName}' did not complete within ${timeout}ms. The server under test accepted the connection but did not finish the exchange.`
|
||||||
|
}
|
||||||
|
];
|
||||||
|
}
|
||||||
|
|
||||||
export async function runServerConformanceTest(
|
export async function runServerConformanceTest(
|
||||||
serverUrl: string,
|
serverUrl: string,
|
||||||
scenarioName: string,
|
scenarioName: string,
|
||||||
outputDir?: string,
|
outputDir?: string,
|
||||||
specVersion?: SpecVersion,
|
specVersion?: SpecVersion,
|
||||||
force = false
|
force = false,
|
||||||
|
timeout: number = 30000
|
||||||
): Promise<{
|
): Promise<{
|
||||||
checks: ConformanceCheck[];
|
checks: ConformanceCheck[];
|
||||||
resultDir?: string;
|
resultDir?: string;
|
||||||
|
|
@ -98,7 +145,11 @@ export async function runServerConformanceTest(
|
||||||
connect: (opts) => connectFor(resolvedSpecVersion)(serverUrl, opts)
|
connect: (opts) => connectFor(resolvedSpecVersion)(serverUrl, opts)
|
||||||
};
|
};
|
||||||
resetWireValidation();
|
resetWireValidation();
|
||||||
const checks = await scenario.run(ctx);
|
const checks = await runScenarioBounded(
|
||||||
|
scenario.run(ctx),
|
||||||
|
scenarioName,
|
||||||
|
timeout
|
||||||
|
);
|
||||||
checks.push(...wireSchemaChecks(resolvedSpecVersion));
|
checks.push(...wireSchemaChecks(resolvedSpecVersion));
|
||||||
|
|
||||||
if (resultDir) {
|
if (resultDir) {
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue