Skip to content

Commit 86beac9

Browse files
committed
feat(server): standardize setup-failure reporting via reportSetupFailure helper
Scenarios that can't execute (connect failure, missing fixture, capability not advertised) currently hand-roll a try/catch around connect and pin the error onto whichever check ID happens to be first. That mislabels the failure and silently drops any *other* checks the scenario would emit (e.g. a connect failure in tools-list also makes tools-name-format vanish). Add `reportSetupFailure(scenarioName, error, specReferences?)` to sdk-client.ts, emitting a single dedicated `<scenario>-setup` check as FAILURE with the error detail. Route the setup path of four representative multi-check server scenarios through it (server-initialize, tools-list, prompts-list, resources-list): connect runs in its own try/catch that returns the setup check; genuine post-connect failures keep their real check IDs. Rebased onto the version-aware connection abstraction (modelcontextprotocol#318): scenarios now take a RunContext and connect via ctx.connect(); the setup/exec split and the reportSetupFailure helper carry over unchanged. This is the "-setup check as a first cut" from modelcontextprotocol#248. Refs modelcontextprotocol#248.
1 parent 25fd443 commit 86beac9

7 files changed

Lines changed: 157 additions & 27 deletions

File tree

src/connection/sdk-client.test.ts

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { reportSetupFailure } from './sdk-client';
3+
4+
describe('reportSetupFailure', () => {
5+
it('emits a single FAILURE check id-d "<scenario>-setup"', () => {
6+
const checks = reportSetupFailure(
7+
'tools-list',
8+
new Error('connect ECONNREFUSED')
9+
);
10+
11+
expect(checks).toHaveLength(1);
12+
expect(checks[0]).toMatchObject({
13+
id: 'tools-list-setup',
14+
status: 'FAILURE',
15+
errorMessage: 'Setup failed: connect ECONNREFUSED'
16+
});
17+
});
18+
19+
it('stringifies a non-Error thrown value', () => {
20+
const checks = reportSetupFailure('prompts-list', 'boom');
21+
22+
expect(checks[0]?.errorMessage).toBe('Setup failed: boom');
23+
});
24+
25+
it('attaches spec references when provided and omits the field otherwise', () => {
26+
const withRefs = reportSetupFailure('resources-list', new Error('nope'), [
27+
{ id: 'MCP-Resources-List' }
28+
]);
29+
expect(withRefs[0]?.specReferences).toEqual([{ id: 'MCP-Resources-List' }]);
30+
31+
const withoutRefs = reportSetupFailure('resources-list', new Error('nope'));
32+
expect(withoutRefs[0]).not.toHaveProperty('specReferences');
33+
});
34+
35+
it('sets a timestamp', () => {
36+
const checks = reportSetupFailure('server-initialize', new Error('x'));
37+
expect(typeof checks[0]?.timestamp).toBe('string');
38+
expect(checks[0]?.timestamp.length).toBeGreaterThan(0);
39+
});
40+
});

src/connection/sdk-client.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,55 @@ import {
88
LoggingMessageNotificationSchema,
99
ProgressNotificationSchema
1010
} from '@modelcontextprotocol/sdk/types.js';
11+
import { ConformanceCheck } from '../types';
1112

1213
export interface MCPClientConnection {
1314
client: Client;
1415
close: () => Promise<void>;
1516
}
1617

18+
/**
19+
* Emit a single `<scenarioName>-setup` check as FAILURE for a scenario that
20+
* could not get far enough to evaluate its real checks (connect failure,
21+
* missing fixture, capability not advertised, etc.).
22+
*
23+
* See #248: previously each scenario hand-rolled a try/catch around connect
24+
* and pinned the setup error onto whichever check ID happened to be first.
25+
* That mislabels the failure — the error ends up under a check that has
26+
* nothing to do with the actual problem, and any *other* checks the scenario
27+
* would have emitted silently disappear. Routing setup failures through this
28+
* helper gives them a dedicated, semantically honest ID and a consistent
29+
* output shape across scenarios.
30+
*
31+
* The convention is that a scenario that cannot execute counts as a FAILURE;
32+
* the escape hatches are scenario filtering (`--suite`/`--scenario`) and the
33+
* expected-failures baseline, not in-scenario skipping or silent passes.
34+
*
35+
* @param scenarioName The scenario's `name`; the emitted check id is
36+
* `<scenarioName>-setup`.
37+
* @param error The thrown setup error.
38+
* @param specReferences Optional spec references to attach to the check.
39+
* @returns A one-element array, so a scenario can `return reportSetupFailure(...)`.
40+
*/
41+
export function reportSetupFailure(
42+
scenarioName: string,
43+
error: unknown,
44+
specReferences?: ConformanceCheck['specReferences']
45+
): ConformanceCheck[] {
46+
const message = error instanceof Error ? error.message : String(error);
47+
return [
48+
{
49+
id: `${scenarioName}-setup`,
50+
name: `${scenarioName} setup`,
51+
description: `Scenario "${scenarioName}" could not be set up (connect/fixture/capability)`,
52+
status: 'FAILURE',
53+
timestamp: new Date().toISOString(),
54+
errorMessage: `Setup failed: ${message}`,
55+
...(specReferences ? { specReferences } : {})
56+
}
57+
];
58+
}
59+
1760
/**
1861
* Create and connect an MCP client to a server
1962
*/

src/scenarios/server/lifecycle.test.ts

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,15 @@ import { testContext } from '../../connection/testing';
22
import { ServerInitializeScenario } from './lifecycle';
33
import { connectToServer } from '../../connection/sdk-client';
44

5-
vi.mock('../../connection/sdk-client', () => ({
6-
connectToServer: vi.fn()
7-
}));
5+
vi.mock(import('../../connection/sdk-client'), async (importOriginal) => {
6+
const actual = await importOriginal();
7+
return {
8+
...actual,
9+
// Only the connection factory is mocked; reportSetupFailure stays real so
10+
// the scenario's setup-failure path is exercised end-to-end.
11+
connectToServer: vi.fn()
12+
};
13+
});
814

915
describe('ServerInitializeScenario', () => {
1016
const serverUrl = 'http://localhost:3000/mcp';
@@ -98,4 +104,25 @@ describe('ServerInitializeScenario', () => {
98104
}
99105
});
100106
});
107+
108+
it('reports a single setup FAILURE when the connection cannot be established', async () => {
109+
vi.mocked(connectToServer).mockRejectedValueOnce(
110+
new Error('connect ECONNREFUSED 127.0.0.1:3000')
111+
);
112+
113+
const checks = await new ServerInitializeScenario().run(
114+
testContext(serverUrl)
115+
);
116+
117+
// A connect failure should not be mislabeled as the initialize or
118+
// session-id check failing (#248): a single dedicated setup check instead.
119+
expect(checks).toHaveLength(1);
120+
expect(checks[0]).toMatchObject({
121+
id: 'server-initialize-setup',
122+
status: 'FAILURE',
123+
errorMessage: 'Setup failed: connect ECONNREFUSED 127.0.0.1:3000'
124+
});
125+
// The session-id check is never reached, so no raw fetch is attempted.
126+
expect(fetchMock).not.toHaveBeenCalled();
127+
});
101128
});

src/scenarios/server/lifecycle.ts

Lines changed: 8 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,10 @@ import {
88
DRAFT_PROTOCOL_VERSION
99
} from '../../types';
1010
import type { RunContext } from '../../connection';
11-
import { connectToServer } from '../../connection/sdk-client';
11+
import {
12+
connectToServer,
13+
reportSetupFailure
14+
} from '../../connection/sdk-client';
1215

1316
const VISIBLE_ASCII_REGEX = /^[\x21-\x7E]+$/;
1417

@@ -70,22 +73,10 @@ and validates session ID format if one is assigned.`;
7073

7174
await connection.close();
7275
} catch (error) {
73-
checks.push({
74-
id: 'server-initialize',
75-
name: 'ServerInitialize',
76-
description:
77-
'Server responds to initialize request with valid structure',
78-
status: 'FAILURE',
79-
timestamp: new Date().toISOString(),
80-
errorMessage: `Failed to initialize: ${error instanceof Error ? error.message : String(error)}`,
81-
specReferences: [
82-
{
83-
id: 'MCP-Initialize',
84-
url: 'https://modelcontextprotocol.io/specification/2025-06-18/basic/lifecycle#initialization'
85-
}
86-
]
87-
});
88-
return checks;
76+
// The handshake never completed, so neither the initialize check nor the
77+
// session-id check below can be evaluated. Report a single setup failure
78+
// rather than mislabeling it as one specific check failing (#248).
79+
return reportSetupFailure(this.name, error);
8980
}
9081

9182
// Check: Session ID visible ASCII validation

src/scenarios/server/prompts.ts

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@
33
*/
44

55
import { ClientScenario, ConformanceCheck } from '../../types';
6-
import type { RunContext } from '../../connection';
6+
import type { Connection, RunContext } from '../../connection';
7+
import { reportSetupFailure } from '../../connection/sdk-client';
78
import type {
89
ListPromptsResult,
910
GetPromptResult
@@ -28,9 +29,16 @@ export class PromptsListScenario implements ClientScenario {
2829
async run(ctx: RunContext): Promise<ConformanceCheck[]> {
2930
const checks: ConformanceCheck[] = [];
3031

32+
let conn: Connection;
3133
try {
32-
const conn = await ctx.connect();
34+
conn = await ctx.connect();
35+
} catch (error) {
36+
// A connect failure isn't a `prompts-list` failure; report it as a setup
37+
// failure rather than mislabeling the check (#248).
38+
return reportSetupFailure(this.name, error);
39+
}
3340

41+
try {
3442
const result = await conn.request<ListPromptsResult>('prompts/list');
3543

3644
// Validate response structure

src/scenarios/server/resources.ts

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@ import {
77
ConformanceCheck,
88
DRAFT_PROTOCOL_VERSION
99
} from '../../types';
10-
import { JsonRpcError, type RunContext } from '../../connection';
10+
import {
11+
JsonRpcError,
12+
type Connection,
13+
type RunContext
14+
} from '../../connection';
15+
import { reportSetupFailure } from '../../connection/sdk-client';
1116
import type {
1217
ListResourcesResult,
1318
ReadResourceResult,
@@ -36,9 +41,16 @@ export class ResourcesListScenario implements ClientScenario {
3641
async run(ctx: RunContext): Promise<ConformanceCheck[]> {
3742
const checks: ConformanceCheck[] = [];
3843

44+
let conn: Connection;
3945
try {
40-
const conn = await ctx.connect();
46+
conn = await ctx.connect();
47+
} catch (error) {
48+
// A connect failure isn't a `resources-list` failure; report it as a
49+
// setup failure rather than mislabeling the check (#248).
50+
return reportSetupFailure(this.name, error);
51+
}
4152

53+
try {
4254
const result = await conn.request<ListResourcesResult>('resources/list');
4355

4456
// Validate response structure

src/scenarios/server/tools.ts

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,15 @@ import {
77
ConformanceCheck,
88
DRAFT_PROTOCOL_VERSION
99
} from '../../types';
10-
import type { RunContext } from '../../connection';
10+
import type { Connection, RunContext } from '../../connection';
1111
import type {
1212
ListToolsResult,
1313
CallToolResult
1414
} from '../../spec-types/2025-06-18';
1515
import {
1616
connectToServer,
17-
NotificationCollector
17+
NotificationCollector,
18+
reportSetupFailure
1819
} from '../../connection/sdk-client';
1920
import {
2021
CreateMessageRequestSchema,
@@ -117,9 +118,17 @@ export class ToolsListScenario implements ClientScenario {
117118
async run(ctx: RunContext): Promise<ConformanceCheck[]> {
118119
const checks: ConformanceCheck[] = [];
119120

121+
let conn: Connection;
120122
try {
121-
const conn = await ctx.connect();
123+
conn = await ctx.connect();
124+
} catch (error) {
125+
// A connect failure isn't a `tools-list` failure; pinning it there would
126+
// also drop the `tools-name-format` check entirely. Report it as setup
127+
// (#248).
128+
return reportSetupFailure(this.name, error);
129+
}
122130

131+
try {
123132
const result = await conn.request<ListToolsResult>('tools/list');
124133

125134
// Validate response structure

0 commit comments

Comments
 (0)