diff --git a/.changeset/periodic-ping.md b/.changeset/periodic-ping.md new file mode 100644 index 000000000..b7c9e3ac7 --- /dev/null +++ b/.changeset/periodic-ping.md @@ -0,0 +1,16 @@ +--- +"@modelcontextprotocol/core": minor +"@modelcontextprotocol/client": minor +"@modelcontextprotocol/server": minor +--- + +feat: add opt-in periodic ping for connection health monitoring + +Adds a `pingIntervalMs` option to `ProtocolOptions` that enables automatic +periodic pings to verify the remote side is still responsive. Per the MCP +specification, implementations SHOULD periodically issue pings to detect +connection health, with configurable frequency. + +The feature is disabled by default. When enabled, pings begin after +initialization completes and stop automatically when the connection closes. +Failures are reported via the `onerror` callback without stopping the timer. diff --git a/packages/client/src/client/client.ts b/packages/client/src/client/client.ts index 1a2694617..b3b29a32e 100644 --- a/packages/client/src/client/client.ts +++ b/packages/client/src/client/client.ts @@ -498,6 +498,8 @@ export class Client extends Protocol { if (this._negotiatedProtocolVersion !== undefined && transport.setProtocolVersion) { transport.setProtocolVersion(this._negotiatedProtocolVersion); } + // Restart periodic ping since _onclose() clears the timer on disconnect + this.startPeriodicPing(); return; } try { @@ -541,6 +543,9 @@ export class Client extends Protocol { this._setupListChangedHandlers(this._pendingListChangedConfig); this._pendingListChangedConfig = undefined; } + + // Start periodic ping after successful initialization + this.startPeriodicPing(); } catch (error) { // Disconnect if initialization fails. void this.close(); diff --git a/packages/core/src/shared/protocol.ts b/packages/core/src/shared/protocol.ts index 57eab6932..306a24bc1 100644 --- a/packages/core/src/shared/protocol.ts +++ b/packages/core/src/shared/protocol.ts @@ -33,6 +33,7 @@ import type { TaskCreationParams } from '../types/index.js'; import { + EmptyResultSchema, getNotificationSchema, getRequestSchema, getResultSchema, @@ -92,6 +93,22 @@ export type ProtocolOptions = { * so they should NOT be included here. */ tasks?: TaskManagerOptions; + + /** + * Interval (in milliseconds) between periodic ping requests sent to the remote side + * to verify connection health. When set, pings begin after initialization completes + * ({@linkcode Client} starts them after the MCP handshake; {@linkcode Server} starts + * them on `notifications/initialized`) and stop automatically when the connection closes. + * + * Per the MCP specification, implementations SHOULD periodically issue pings to + * detect connection health, with configurable frequency. + * + * Disabled by default (no periodic pings). Typical values: 15000-60000 (15s-60s). + * + * Ping failures are reported via the {@linkcode Protocol.onerror | onerror} callback + * and do not stop the periodic loop. + */ + pingIntervalMs?: number; }; /** @@ -308,6 +325,10 @@ export abstract class Protocol { private _taskManager: TaskManager; + private _pingTimer?: ReturnType; + private _pingIntervalMs?: number; + private _closing = false; + protected _supportedProtocolVersions: string[]; /** @@ -336,6 +357,7 @@ export abstract class Protocol { constructor(private _options?: ProtocolOptions) { this._supportedProtocolVersions = _options?.supportedProtocolVersions ?? SUPPORTED_PROTOCOL_VERSIONS; + this._pingIntervalMs = _options?.pingIntervalMs; // Create TaskManager from protocol options this._taskManager = _options?.tasks ? new TaskManager(_options.tasks) : new NullTaskManager(); @@ -454,6 +476,7 @@ export abstract class Protocol { */ async connect(transport: Transport): Promise { this._transport = transport; + this._closing = false; const _onclose = this.transport?.onclose; this._transport.onclose = () => { try { @@ -490,6 +513,8 @@ export abstract class Protocol { } private _onclose(): void { + this.stopPeriodicPing(); + const responseHandlers = this._responseHandlers; this._responseHandlers = new Map(); this._progressHandlers.clear(); @@ -728,10 +753,70 @@ export abstract class Protocol { return this._transport; } + /** + * Starts sending periodic ping requests at the configured interval. + * Pings are used to verify that the remote side is still responsive. + * Failures are reported via the {@linkcode onerror} callback but do not + * stop the loop; pings continue until the connection is closed. + * + * This is not called automatically by the base {@linkcode Protocol.connect | connect()} + * method. {@linkcode Client} calls it after the MCP initialization handshake + * (and on reconnection), and {@linkcode Server} calls it when the + * `notifications/initialized` notification is received. Custom `Protocol` + * subclasses must call this explicitly after their own initialization. + * + * Has no effect if periodic ping is already running or if no interval + * is configured. + */ + protected startPeriodicPing(): void { + if (this._pingTimer || !this._pingIntervalMs) { + return; + } + + const schedulePing = (): void => { + this._pingTimer = setTimeout(async () => { + try { + await this._requestWithSchema({ method: 'ping' }, EmptyResultSchema, { + timeout: this._pingIntervalMs + }); + } catch (error) { + // Suppress errors caused by intentional shutdown + if (!this._closing) { + this._onerror(error instanceof Error ? error : new Error(`Periodic ping failed: ${String(error)}`)); + } + } finally { + // Schedule the next ping only if we have not been stopped + if (this._pingTimer) { + schedulePing(); + } + } + }, this._pingIntervalMs); + + // Allow the process to exit even if the timer is still running + if (typeof this._pingTimer === 'object' && 'unref' in this._pingTimer) { + this._pingTimer.unref(); + } + }; + + schedulePing(); + } + + /** + * Stops periodic ping requests. Called automatically when the connection closes. + */ + protected stopPeriodicPing(): void { + if (this._pingTimer) { + clearTimeout(this._pingTimer); + this._pingTimer = undefined; + } + } + /** * Closes the connection. */ async close(): Promise { + this._closing = true; + this.stopPeriodicPing(); await this._transport?.close(); } diff --git a/packages/core/test/shared/periodicPing.test.ts b/packages/core/test/shared/periodicPing.test.ts new file mode 100644 index 000000000..2cf836881 --- /dev/null +++ b/packages/core/test/shared/periodicPing.test.ts @@ -0,0 +1,271 @@ +import { vi, beforeEach, afterEach, describe, test, expect } from 'vitest'; + +import type { BaseContext } from '../../src/shared/protocol.js'; +import { Protocol } from '../../src/shared/protocol.js'; +import type { Transport, TransportSendOptions } from '../../src/shared/transport.js'; +import type { JSONRPCMessage } from '../../src/types/types.js'; + +class MockTransport implements Transport { + onclose?: () => void; + onerror?: (error: Error) => void; + onmessage?: (message: unknown) => void; + + async start(): Promise {} + async close(): Promise { + this.onclose?.(); + } + async send(_message: JSONRPCMessage, _options?: TransportSendOptions): Promise {} +} + +function createProtocol(options?: { pingIntervalMs?: number }) { + return new (class extends Protocol { + protected assertCapabilityForMethod(): void {} + protected assertNotificationCapability(): void {} + protected assertRequestHandlerCapability(): void {} + protected assertTaskCapability(): void {} + protected buildContext(ctx: BaseContext): BaseContext { + return ctx; + } + protected assertTaskHandlerCapability(): void {} + // Expose protected methods for testing + public testStartPeriodicPing(): void { + this.startPeriodicPing(); + } + public testStopPeriodicPing(): void { + this.stopPeriodicPing(); + } + })(options); +} + +/** Configure the spy to auto-respond to pings with a success result. */ +function autoRespondPings(transport: MockTransport, sendSpy: ReturnType): void { + sendSpy.mockImplementation(async (message: JSONRPCMessage) => { + const msg = message as { id?: number; method?: string }; + if (msg.method === 'ping' && msg.id !== undefined) { + transport.onmessage?.({ + jsonrpc: '2.0', + id: msg.id, + result: {} + }); + } + }); +} + +/** Count ping messages in spy call history. */ +function countPings(sendSpy: ReturnType): number { + return sendSpy.mock.calls.filter((call: unknown[]) => { + const msg = call[0] as { method?: string }; + return msg.method === 'ping'; + }).length; +} + +describe('Periodic Ping', () => { + let transport: MockTransport; + + beforeEach(() => { + vi.useFakeTimers(); + transport = new MockTransport(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + test('should not send periodic pings when pingIntervalMs is not set', async () => { + const protocol = createProtocol(); + const sendSpy = vi.spyOn(transport, 'send'); + + await protocol.connect(transport); + + // Advance time well past any reasonable interval + await vi.advanceTimersByTimeAsync(120_000); + + expect(countPings(sendSpy)).toBe(0); + }); + + test('should send periodic pings when pingIntervalMs is set and startPeriodicPing is called', async () => { + const protocol = createProtocol({ pingIntervalMs: 10_000 }); + const sendSpy = vi.spyOn(transport, 'send'); + + await protocol.connect(transport); + autoRespondPings(transport, sendSpy); + + // Start periodic ping (in real usage, Client.connect() calls this after init) + protocol.testStartPeriodicPing(); + + // No ping yet (first fires after one interval) + expect(countPings(sendSpy)).toBe(0); + + // Advance past one interval + await vi.advanceTimersByTimeAsync(10_000); + expect(countPings(sendSpy)).toBe(1); + + // Advance past another interval + await vi.advanceTimersByTimeAsync(10_000); + expect(countPings(sendSpy)).toBe(2); + }); + + test('should stop periodic pings on close', async () => { + const protocol = createProtocol({ pingIntervalMs: 5_000 }); + const sendSpy = vi.spyOn(transport, 'send'); + + await protocol.connect(transport); + autoRespondPings(transport, sendSpy); + + protocol.testStartPeriodicPing(); + + // One ping fires + await vi.advanceTimersByTimeAsync(5_000); + expect(countPings(sendSpy)).toBe(1); + + // Close the connection + await protocol.close(); + + // Advance more time; no new pings should be sent + sendSpy.mockClear(); + await vi.advanceTimersByTimeAsync(20_000); + + expect(countPings(sendSpy)).toBe(0); + }); + + test('should report ping errors via onerror without stopping the loop', async () => { + const protocol = createProtocol({ pingIntervalMs: 5_000 }); + const errors: Error[] = []; + protocol.onerror = error => { + errors.push(error); + }; + + await protocol.connect(transport); + + // Respond with an error to simulate a failed ping + const sendSpy = vi.spyOn(transport, 'send'); + sendSpy.mockImplementation(async (message: JSONRPCMessage) => { + const msg = message as { id?: number; method?: string }; + if (msg.method === 'ping' && msg.id !== undefined) { + transport.onmessage?.({ + jsonrpc: '2.0', + id: msg.id, + error: { + code: -32000, + message: 'Server error' + } + }); + } + }); + + protocol.testStartPeriodicPing(); + + // First ping fails + await vi.advanceTimersByTimeAsync(5_000); + expect(errors).toHaveLength(1); + + // Second ping also fails, proving the loop was not stopped + await vi.advanceTimersByTimeAsync(5_000); + expect(errors).toHaveLength(2); + }); + + test('should not start duplicate timers if startPeriodicPing is called multiple times', async () => { + const protocol = createProtocol({ pingIntervalMs: 5_000 }); + const sendSpy = vi.spyOn(transport, 'send'); + + await protocol.connect(transport); + autoRespondPings(transport, sendSpy); + + // Call startPeriodicPing multiple times + protocol.testStartPeriodicPing(); + protocol.testStartPeriodicPing(); + protocol.testStartPeriodicPing(); + + await vi.advanceTimersByTimeAsync(5_000); + + // Should only have one ping, not three + expect(countPings(sendSpy)).toBe(1); + }); + + test('should stop periodic pings when transport closes unexpectedly', async () => { + const protocol = createProtocol({ pingIntervalMs: 5_000 }); + const sendSpy = vi.spyOn(transport, 'send'); + + await protocol.connect(transport); + autoRespondPings(transport, sendSpy); + + protocol.testStartPeriodicPing(); + + // One ping fires + await vi.advanceTimersByTimeAsync(5_000); + + // Simulate transport closing unexpectedly + transport.onclose?.(); + + sendSpy.mockClear(); + await vi.advanceTimersByTimeAsync(20_000); + + expect(countPings(sendSpy)).toBe(0); + }); + + test('should not fire onerror when close() races with an in-flight ping', async () => { + const protocol = createProtocol({ pingIntervalMs: 5_000 }); + const errors: Error[] = []; + protocol.onerror = error => { + errors.push(error); + }; + + await protocol.connect(transport); + + // Do NOT auto-respond: the ping will remain in-flight so close() races with it + protocol.testStartPeriodicPing(); + + // Advance to fire the first ping (it is now awaiting a response) + await vi.advanceTimersByTimeAsync(5_000); + + // Close while the ping is in-flight; this should NOT produce an onerror + await protocol.close(); + + // Drain any pending microtasks + await vi.advanceTimersByTimeAsync(0); + + expect(errors).toHaveLength(0); + }); + + test('pings are strictly sequential (no concurrent overlap)', async () => { + const protocol = createProtocol({ pingIntervalMs: 5_000 }); + const sendSpy = vi.spyOn(transport, 'send'); + let inFlightPings = 0; + let maxConcurrent = 0; + + await protocol.connect(transport); + + sendSpy.mockImplementation(async (message: JSONRPCMessage) => { + const msg = message as { id?: number; method?: string }; + if (msg.method === 'ping' && msg.id !== undefined) { + inFlightPings++; + if (inFlightPings > maxConcurrent) { + maxConcurrent = inFlightPings; + } + // Simulate a slow response: resolve after a short delay + setTimeout(() => { + inFlightPings--; + transport.onmessage?.({ + jsonrpc: '2.0', + id: msg.id, + result: {} + }); + }, 2_000); + } + }); + + protocol.testStartPeriodicPing(); + + // Advance enough for multiple ping cycles + for (let i = 0; i < 5; i++) { + await vi.advanceTimersByTimeAsync(5_000); + // Let the delayed response resolve + await vi.advanceTimersByTimeAsync(2_000); + } + + // With setTimeout-based scheduling, pings are strictly sequential + expect(maxConcurrent).toBe(1); + // Verify pings were actually sent + expect(countPings(sendSpy)).toBeGreaterThanOrEqual(3); + }); +}); diff --git a/packages/server/src/server/server.ts b/packages/server/src/server/server.ts index 98bbfbb1b..e8d6db341 100644 --- a/packages/server/src/server/server.ts +++ b/packages/server/src/server/server.ts @@ -133,13 +133,18 @@ export class Server extends Protocol { } this.setRequestHandler('initialize', request => this._oninitialize(request)); - this.setNotificationHandler('notifications/initialized', () => this.oninitialized?.()); + this.setNotificationHandler('notifications/initialized', () => this._handleInitialized()); if (this._capabilities.logging) { this._registerLoggingHandler(); } } + private _handleInitialized(): void { + this.startPeriodicPing(); + this.oninitialized?.(); + } + private _registerLoggingHandler(): void { this.setRequestHandler('logging/setLevel', async (request, ctx) => { const transportSessionId: string | undefined =