From f64601b469d6732ccf6b3ee4bab203c208a78704 Mon Sep 17 00:00:00 2001 From: chenxi Date: Sat, 10 May 2025 13:35:30 +0800 Subject: [PATCH 1/9] optimization: When using the custom SSE request, the `Authorization` header can still be automatically attached to the SSE request. --- src/client/sse.test.ts | 31 +++++++++++++++++++++--- src/client/sse.ts | 53 +++++++++++++++++++++++++++++++----------- 2 files changed, 68 insertions(+), 16 deletions(-) diff --git a/src/client/sse.test.ts b/src/client/sse.test.ts index 77b28508..222ac364 100644 --- a/src/client/sse.test.ts +++ b/src/client/sse.test.ts @@ -1,4 +1,4 @@ -import { createServer, type IncomingMessage, type Server } from "http"; +import { createServer, IncomingMessage, Server, ServerResponse } from "http"; import { AddressInfo } from "net"; import { JSONRPCMessage } from "../types.js"; import { SSEClientTransport } from "./sse.js"; @@ -10,8 +10,21 @@ describe("SSEClientTransport", () => { let transport: SSEClientTransport; let baseUrl: URL; let lastServerRequest: IncomingMessage; + const serverRequests: Record = {}; let sendServerMessage: ((message: string) => void) | null = null; + const recordServerRequest = (req: IncomingMessage, res: ServerResponse) => { + lastServerRequest = req; + + const key = `${req.method} ${req.url}`; + serverRequests[key] = serverRequests[key] || []; + serverRequests[key].push(req); + + res.on('finish', () => { + console.log(`[server] ${req.method} ${req.url} -> ${res.statusCode} ${res.statusMessage}`); + }); + }; + beforeEach((done) => { // Reset state lastServerRequest = null as unknown as IncomingMessage; @@ -487,7 +500,7 @@ describe("SSEClientTransport", () => { let connectionAttempts = 0; server = createServer((req, res) => { - lastServerRequest = req; + recordServerRequest(req, res); if (req.url === "/token" && req.method === "POST") { // Handle token refresh request @@ -496,7 +509,7 @@ describe("SSEClientTransport", () => { req.on("end", () => { const params = new URLSearchParams(body); if (params.get("grant_type") === "refresh_token" && - params.get("refresh_token") === "refresh-token" && + params.get("refresh_token")?.includes("refresh-token") && params.get("client_id") === "test-client-id" && params.get("client_secret") === "test-client-secret") { res.writeHead(200, { "Content-Type": "application/json" }); @@ -531,6 +544,7 @@ describe("SSEClientTransport", () => { }); res.write("event: endpoint\n"); res.write(`data: ${baseUrl.href}\n\n`); + res.end(); connectionAttempts++; return; } @@ -548,6 +562,14 @@ describe("SSEClientTransport", () => { transport = new SSEClientTransport(baseUrl, { authProvider: mockAuthProvider, + eventSourceInit: { + fetch: (url, init) => { + return fetch(url, { ...init, headers: { + ...init?.headers, + 'X-Custom-Header': 'custom-value' + } }); + } + }, }); await transport.start(); @@ -559,6 +581,9 @@ describe("SSEClientTransport", () => { }); expect(connectionAttempts).toBe(1); expect(lastServerRequest.headers.authorization).toBe("Bearer new-token"); + expect(serverRequests["GET /"]).toHaveLength(2); + expect(serverRequests["GET /"] + .every(req => req.headers["x-custom-header"] === "custom-value")).toBe(true); }); it("refreshes expired token during POST request", async () => { diff --git a/src/client/sse.ts b/src/client/sse.ts index 5e9f0cf0..8ba86402 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -96,7 +96,7 @@ export class SSEClientTransport implements Transport { return await this._startOrAuth(); } - private async _commonHeaders(): Promise { + private async _commonHeaders(): Promise> { const headers: HeadersInit = {}; if (this._authProvider) { const tokens = await this._authProvider.tokens(); @@ -110,18 +110,7 @@ export class SSEClientTransport implements Transport { private _startOrAuth(): Promise { return new Promise((resolve, reject) => { - this._eventSource = new EventSource( - this._url.href, - this._eventSourceInit ?? { - fetch: (url, init) => this._commonHeaders().then((headers) => fetch(url, { - ...init, - headers: { - ...headers, - Accept: "text/event-stream" - } - })), - }, - ); + this._eventSource = new EventSource(this._url.href, this._getEventSourceInit()); this._abortController = new AbortController(); this._eventSource.onerror = (event) => { @@ -175,6 +164,44 @@ export class SSEClientTransport implements Transport { }); } + private _getEventSourceInit(): EventSourceInit { + let eventSourceInit: EventSourceInit; + + if (this._eventSourceInit) { + const originalFetch = this._eventSourceInit.fetch; + + if (originalFetch && this._authProvider) { + // merge the new headers with the existing headers + eventSourceInit = { + ...this._eventSourceInit, + fetch: async (url, init) => { + const newHeaders: Record = await this._commonHeaders(); + return originalFetch(url, { + ...init, + headers: { + ...newHeaders, + ...init?.headers + } + }); + } + }; + } else { + eventSourceInit = this._eventSourceInit; + } + } else { + eventSourceInit = { + fetch: (url, init) => this._commonHeaders().then((headers) => fetch(url, { + ...init, + headers: { + ...headers, + Accept: "text/event-stream" + } + })), + }; + } + return eventSourceInit; + } + async start() { if (this._eventSource) { throw new Error( From d93cfca7f228418379340f793cdb3865c3243070 Mon Sep 17 00:00:00 2001 From: chenxi Date: Sat, 10 May 2025 13:40:49 +0800 Subject: [PATCH 2/9] chore: update .gitignore to include .vscode directory --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 6c4bf1a6..7a8221a3 100644 --- a/.gitignore +++ b/.gitignore @@ -120,6 +120,7 @@ out # Stores VSCode versions used for testing VSCode extensions .vscode-test +.vscode/ # yarn v2 .yarn/cache From 05bc65c81b4156e1c8afb03efa82e5b05ec8c2e4 Mon Sep 17 00:00:00 2001 From: chenxi Date: Sat, 10 May 2025 14:21:32 +0800 Subject: [PATCH 3/9] chore: update commment --- src/client/sse.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/client/sse.ts b/src/client/sse.ts index 8ba86402..8b29b5fb 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -35,11 +35,6 @@ export type SSEClientTransportOptions = { /** * Customizes the initial SSE request to the server (the request that begins the stream). - * - * NOTE: Setting this property will prevent an `Authorization` header from - * being automatically attached to the SSE request, if an `authProvider` is - * also given. This can be worked around by setting the `Authorization` header - * manually. */ eventSourceInit?: EventSourceInit; From a179c12a3c5753db54830340fbbb5727844709fd Mon Sep 17 00:00:00 2001 From: Olivier Chafik Date: Thu, 10 Jul 2025 17:08:47 +0100 Subject: [PATCH 4/9] refactor _eventSourceInit in sse.ts (move to ctor) --- src/client/sse.ts | 56 +++++++++++++---------------------------------- 1 file changed, 15 insertions(+), 41 deletions(-) diff --git a/src/client/sse.ts b/src/client/sse.ts index 8b29b5fb..917804bd 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -53,7 +53,7 @@ export class SSEClientTransport implements Transport { private _endpoint?: URL; private _abortController?: AbortController; private _url: URL; - private _eventSourceInit?: EventSourceInit; + private _eventSourceInit: EventSourceInit; private _requestInit?: RequestInit; private _authProvider?: OAuthClientProvider; @@ -66,7 +66,19 @@ export class SSEClientTransport implements Transport { opts?: SSEClientTransportOptions, ) { this._url = url; - this._eventSourceInit = opts?.eventSourceInit; + + const actualFetch = opts?.eventSourceInit?.fetch ?? fetch; + this._eventSourceInit = { + ...(opts?.eventSourceInit ?? {}), + fetch: (url, init) => this._commonHeaders().then((headers) => actualFetch(url, { + ...init, + headers: { + ...headers, + Accept: "text/event-stream" + } + })), + }; + this._requestInit = opts?.requestInit; this._authProvider = opts?.authProvider; } @@ -105,7 +117,7 @@ export class SSEClientTransport implements Transport { private _startOrAuth(): Promise { return new Promise((resolve, reject) => { - this._eventSource = new EventSource(this._url.href, this._getEventSourceInit()); + this._eventSource = new EventSource(this._url.href, this._eventSourceInit); this._abortController = new AbortController(); this._eventSource.onerror = (event) => { @@ -159,44 +171,6 @@ export class SSEClientTransport implements Transport { }); } - private _getEventSourceInit(): EventSourceInit { - let eventSourceInit: EventSourceInit; - - if (this._eventSourceInit) { - const originalFetch = this._eventSourceInit.fetch; - - if (originalFetch && this._authProvider) { - // merge the new headers with the existing headers - eventSourceInit = { - ...this._eventSourceInit, - fetch: async (url, init) => { - const newHeaders: Record = await this._commonHeaders(); - return originalFetch(url, { - ...init, - headers: { - ...newHeaders, - ...init?.headers - } - }); - } - }; - } else { - eventSourceInit = this._eventSourceInit; - } - } else { - eventSourceInit = { - fetch: (url, init) => this._commonHeaders().then((headers) => fetch(url, { - ...init, - headers: { - ...headers, - Accept: "text/event-stream" - } - })), - }; - } - return eventSourceInit; - } - async start() { if (this._eventSource) { throw new Error( From f6a748ba6f6a054ea031661afd62ffca4b9c2bed Mon Sep 17 00:00:00 2001 From: chenxi Date: Fri, 11 Jul 2025 10:48:50 +0800 Subject: [PATCH 5/9] chore: fix the last Git merging --- src/client/sse.test.ts | 1 + src/client/sse.ts | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/client/sse.test.ts b/src/client/sse.test.ts index 287212f2..25859569 100644 --- a/src/client/sse.test.ts +++ b/src/client/sse.test.ts @@ -664,6 +664,7 @@ describe("SSEClientTransport", () => { let connectionAttempts = 0; resourceServer = createServer((req, res) => { + recordServerRequest(req, res); lastServerRequest = req; if (req.url === "/.well-known/oauth-protected-resource") { diff --git a/src/client/sse.ts b/src/client/sse.ts index f71cb646..110660a8 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -87,7 +87,7 @@ export class SSEClientTransport implements Transport { fetch: (url, init) => this._commonHeaders().then((headers) => actualFetch(url, { ...init, headers: { - ...headers, + ...Object.fromEntries(headers.entries()), Accept: "text/event-stream" } })), From b247c39aaf19dd69e0cd7f6e6d2c2fd22e2bb1d0 Mon Sep 17 00:00:00 2001 From: chenxi Date: Fri, 11 Jul 2025 10:50:25 +0800 Subject: [PATCH 6/9] chore: .gitignore --- .gitignore | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 7a8221a3..236a41ba 100644 --- a/.gitignore +++ b/.gitignore @@ -120,7 +120,6 @@ out # Stores VSCode versions used for testing VSCode extensions .vscode-test -.vscode/ # yarn v2 .yarn/cache @@ -129,5 +128,9 @@ out .yarn/install-state.gz .pnp.* +# IDE +.vscode/ +.idea/ + .DS_Store dist/ From 056ce81e402a830094b7f045f5159949ab46d9a4 Mon Sep 17 00:00:00 2001 From: chenxi Date: Fri, 11 Jul 2025 11:39:07 +0800 Subject: [PATCH 7/9] fix: compatible with the custom `opts.fetch` --- src/client/sse.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client/sse.ts b/src/client/sse.ts index 110660a8..d8ba53ea 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -81,7 +81,7 @@ export class SSEClientTransport implements Transport { this._url = url; this._resourceMetadataUrl = undefined; - const actualFetch = opts?.eventSourceInit?.fetch ?? fetch; + const actualFetch = opts?.eventSourceInit?.fetch ?? opts?.fetch ?? fetch; this._eventSourceInit = { ...(opts?.eventSourceInit ?? {}), fetch: (url, init) => this._commonHeaders().then((headers) => actualFetch(url, { From 1b3874e3b362fb3ce56ffe593bdb33d21eb24b60 Mon Sep 17 00:00:00 2001 From: chenxi Date: Sat, 12 Jul 2025 00:01:48 +0800 Subject: [PATCH 8/9] refactor: simplify the code + update javadoc --- src/client/sse.ts | 25 +++++-------------------- 1 file changed, 5 insertions(+), 20 deletions(-) diff --git a/src/client/sse.ts b/src/client/sse.ts index d8ba53ea..ed6e39f5 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -35,11 +35,6 @@ export type SSEClientTransportOptions = { /** * Customizes the initial SSE request to the server (the request that begins the stream). - * - * NOTE: Setting this property will prevent an `Authorization` header from - * being automatically attached to the SSE request, if an `authProvider` is - * also given. This can be worked around by setting the `Authorization` header - * manually. */ eventSourceInit?: EventSourceInit; @@ -64,7 +59,7 @@ export class SSEClientTransport implements Transport { private _abortController?: AbortController; private _url: URL; private _resourceMetadataUrl?: URL; - private _eventSourceInit: EventSourceInit; + private _eventSourceInit?: EventSourceInit; private _requestInit?: RequestInit; private _authProvider?: OAuthClientProvider; private _fetch?: FetchLike; @@ -80,19 +75,7 @@ export class SSEClientTransport implements Transport { ) { this._url = url; this._resourceMetadataUrl = undefined; - - const actualFetch = opts?.eventSourceInit?.fetch ?? opts?.fetch ?? fetch; - this._eventSourceInit = { - ...(opts?.eventSourceInit ?? {}), - fetch: (url, init) => this._commonHeaders().then((headers) => actualFetch(url, { - ...init, - headers: { - ...Object.fromEntries(headers.entries()), - Accept: "text/event-stream" - } - })), - }; - + this._eventSourceInit = opts?.eventSourceInit; this._requestInit = opts?.requestInit; this._authProvider = opts?.authProvider; this._fetch = opts?.fetch; @@ -147,7 +130,9 @@ export class SSEClientTransport implements Transport { headers.set("Accept", "text/event-stream"); const response = await fetchImpl(url, { ...init, - headers, + headers: { + ...Object.fromEntries(headers.entries()), + } }) if (response.status === 401 && response.headers.has('www-authenticate')) { From b08ad9fafc7bc43ef3e09dc4e1a6dcb7a879b5e2 Mon Sep 17 00:00:00 2001 From: chenxi Date: Wed, 16 Jul 2025 21:09:34 +0800 Subject: [PATCH 9/9] refactor: based on CR advice --- src/client/sse.test.ts | 2 +- src/client/sse.ts | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/client/sse.test.ts b/src/client/sse.test.ts index 25859569..efbafedc 100644 --- a/src/client/sse.test.ts +++ b/src/client/sse.test.ts @@ -717,7 +717,7 @@ describe("SSEClientTransport", () => { eventSourceInit: { fetch: (url, init) => { return fetch(url, { ...init, headers: { - ...init?.headers, + ...(init?.headers instanceof Headers ? Object.fromEntries(init.headers.entries()) : init?.headers), 'X-Custom-Header': 'custom-value' } }); } diff --git a/src/client/sse.ts b/src/client/sse.ts index ed6e39f5..827d1700 100644 --- a/src/client/sse.ts +++ b/src/client/sse.ts @@ -130,9 +130,7 @@ export class SSEClientTransport implements Transport { headers.set("Accept", "text/event-stream"); const response = await fetchImpl(url, { ...init, - headers: { - ...Object.fromEntries(headers.entries()), - } + headers, }) if (response.status === 401 && response.headers.has('www-authenticate')) {