From 7aeb67988a5d4d5d3d065087f0d5cefb2fdfe767 Mon Sep 17 00:00:00 2001 From: ihrpr Date: Mon, 14 Apr 2025 09:37:19 +0100 Subject: [PATCH] improve comments --- src/client/streamableHttp.test.ts | 10 +++++----- src/client/streamableHttp.ts | 10 +++++----- src/server/streamableHttp.ts | 3 ++- src/shared/protocol.ts | 7 +++---- 4 files changed, 15 insertions(+), 15 deletions(-) diff --git a/src/client/streamableHttp.test.ts b/src/client/streamableHttp.test.ts index a73927ac..2dfa5869 100644 --- a/src/client/streamableHttp.test.ts +++ b/src/client/streamableHttp.test.ts @@ -164,7 +164,7 @@ describe("StreamableHTTPClientTransport", () => { // We expect the 405 error to be caught and handled gracefully // This should not throw an error that breaks the transport await transport.start(); - await expect(transport["_startOrAuthStandaloneSSE"]()).resolves.not.toThrow("Failed to open SSE stream: Method Not Allowed"); + await expect(transport["_startOrAuthSse"]()).resolves.not.toThrow("Failed to open SSE stream: Method Not Allowed"); // Check that GET was attempted expect(global.fetch).toHaveBeenCalledWith( expect.anything(), @@ -208,7 +208,7 @@ describe("StreamableHTTPClientTransport", () => { transport.onmessage = messageSpy; await transport.start(); - await transport["_startOrAuthStandaloneSSE"](); + await transport["_startOrAuthSse"](); // Give time for the SSE event to be processed await new Promise(resolve => setTimeout(resolve, 50)); @@ -313,9 +313,9 @@ describe("StreamableHTTPClientTransport", () => { await transport.start(); // Type assertion to access private method const transportWithPrivateMethods = transport as unknown as { - _startOrAuthStandaloneSSE: (lastEventId?: string) => Promise + _startOrAuthSse: (lastEventId?: string) => Promise }; - await transportWithPrivateMethods._startOrAuthStandaloneSSE("test-event-id"); + await transportWithPrivateMethods._startOrAuthSse("test-event-id"); // Verify fetch was called with the lastEventId header expect(fetchSpy).toHaveBeenCalled(); @@ -382,7 +382,7 @@ describe("StreamableHTTPClientTransport", () => { await transport.start(); - await transport["_startOrAuthStandaloneSSE"](); + await transport["_startOrAuthSse"](); expect((actualReqInit.headers as Headers).get("x-custom-header")).toBe("CustomValue"); requestInit.headers["X-Custom-Header"] = "SecondCustomValue"; diff --git a/src/client/streamableHttp.ts b/src/client/streamableHttp.ts index 77acc9c4..43030239 100644 --- a/src/client/streamableHttp.ts +++ b/src/client/streamableHttp.ts @@ -123,7 +123,7 @@ export class StreamableHTTPClientTransport implements Transport { throw new UnauthorizedError(); } - return await this._startOrAuthStandaloneSSE(); + return await this._startOrAuthSse(); } private async _commonHeaders(): Promise { @@ -144,7 +144,7 @@ export class StreamableHTTPClientTransport implements Transport { ); } - private async _startOrAuthStandaloneSSE(lastEventId?: string): Promise { + private async _startOrAuthSse(lastEventId?: string): Promise { try { // Try to open an initial SSE stream with GET to listen for server messages // This is optional according to the spec - server may not support it @@ -238,7 +238,7 @@ export class StreamableHTTPClientTransport implements Transport { // Schedule the reconnection setTimeout(() => { // Use the last event ID to resume where we left off - this._startOrAuthStandaloneSSE(lastEventId).catch(error => { + this._startOrAuthSse(lastEventId).catch(error => { this.onerror?.(new Error(`Failed to reconnect SSE stream: ${error instanceof Error ? error.message : String(error)}`)); // Schedule another attempt if this one failed, incrementing the attempt counter this._scheduleReconnection(lastEventId, attemptCount + 1); @@ -343,7 +343,7 @@ export class StreamableHTTPClientTransport implements Transport { const { lastEventId, onLastEventIdUpdate } = options ?? {}; if (lastEventId) { // If we have at last event ID, we need to reconnect the SSE stream - this._startOrAuthStandaloneSSE(lastEventId).catch(err => this.onerror?.(err)); + this._startOrAuthSse(lastEventId).catch(err => this.onerror?.(err)); return; } @@ -390,7 +390,7 @@ export class StreamableHTTPClientTransport implements Transport { // if it's supported by the server if (isJSONRPCNotification(message) && message.method === "notifications/initialized") { // Start without a lastEventId since this is a fresh connection - this._startOrAuthStandaloneSSE().catch(err => this.onerror?.(err)); + this._startOrAuthSse().catch(err => this.onerror?.(err)); } return; } diff --git a/src/server/streamableHttp.ts b/src/server/streamableHttp.ts index 7740a08f..0487e1af 100644 --- a/src/server/streamableHttp.ts +++ b/src/server/streamableHttp.ts @@ -346,7 +346,8 @@ export class StreamableHTTPServerTransport implements Transport { ); const mcpSessionId = req.headers["mcp-session-id"] as string | undefined; if (isInitializationRequest) { - // if generateSessionId is not set, the server does not support session management + // If it's a server with session management and the session ID is already set we should reject the request + // to avoid re-initialization. if (this._initialized && this.sessionId !== undefined && mcpSessionId !== this.sessionId) { res.writeHead(400).end(JSON.stringify({ jsonrpc: "2.0", diff --git a/src/shared/protocol.ts b/src/shared/protocol.ts index 6193f5f1..bc7882e4 100644 --- a/src/shared/protocol.ts +++ b/src/shared/protocol.ts @@ -87,16 +87,15 @@ export type RequestOptions = { * May be used to indicate to the transport which incoming request to associate this outgoing request with. */ relatedRequestId?: RequestId; - /** - * May be used to indicate to the transport which last event ID to associate this outgoing request with. - * This is used to resume a long-running requests that may have been interrupted and a new instance of a client is being created. + * The last event ID to associate with the request. + * Used to resume long-running requests that were interrupted when creating a new client instance. */ lastEventId?: string; /** * A callback that is invoked when the last event ID is updated. - * This is used to notidy the client that the last event ID has changed, so that client can update its state accordingly. + * This is used to notify the client that the last event ID has changed, so the client can update its state accordingly. */ onLastEventIdUpdate?: (event: string) => void; };