diff --git a/packages/fetch/src/fetch.e2e.test.ts b/packages/fetch/src/fetch.e2e.test.ts index fa0a3c4fc87..a708830e349 100644 --- a/packages/fetch/src/fetch.e2e.test.ts +++ b/packages/fetch/src/fetch.e2e.test.ts @@ -6,6 +6,7 @@ import * as path from "node:path"; import { execSync } from "node:child_process"; import { afterEach, describe, expect, test } from "vitest"; import { fetchwithRequestOptions } from "./fetch.js"; +import { clearHttpAgentCache } from "./httpAgentCache.js"; // Test server ports const HTTP_PORT = 3001; @@ -15,7 +16,10 @@ const HTTPS_PORT = 3002; const serversToCleanup: Array = []; const tempDirsToCleanup: string[] = []; -afterEach(() => { +afterEach(async () => { + // Drop keep-alive sockets so the next test does not reuse a closed server. + await clearHttpAgentCache(); + // Clean up all servers serversToCleanup.forEach((server) => server.close()); serversToCleanup.length = 0; diff --git a/packages/fetch/src/fetch.ts b/packages/fetch/src/fetch.ts index e965899d4c9..821ee0a5e2b 100644 --- a/packages/fetch/src/fetch.ts +++ b/packages/fetch/src/fetch.ts @@ -1,14 +1,9 @@ import { RequestOptions } from "@continuedev/config-types"; -import * as followRedirects from "follow-redirects"; -import { HttpProxyAgent } from "http-proxy-agent"; -import { HttpsProxyAgent } from "https-proxy-agent"; import { BodyInit, RequestInit, Response } from "node-fetch"; -import { getAgentOptions } from "./getAgentOptions.js"; +import { getOrCreateAgent } from "./httpAgentCache.js"; import patchedFetch from "./node-fetch-patch.js"; import { getProxy, shouldBypassProxy } from "./util.js"; -const { http, https } = (followRedirects as any).default; - function logRequest( method: string, url: URL, @@ -89,22 +84,14 @@ export async function fetchwithRequestOptions( url.host = "127.0.0.1"; } - const agentOptions = await getAgentOptions(requestOptions); - - // Get proxy from options or environment variables const proxy = getProxy(url.protocol, requestOptions); - - // Check if should bypass proxy based on requestOptions or NO_PROXY env var const shouldBypass = shouldBypassProxy(url.hostname, requestOptions); - - // Create agent - const protocol = url.protocol === "https:" ? https : http; - const agent = - proxy && !shouldBypass - ? protocol === https - ? new HttpsProxyAgent(proxy, agentOptions) - : new HttpProxyAgent(proxy, agentOptions) - : new protocol.Agent(agentOptions); + const agent = await getOrCreateAgent( + url.protocol, + proxy, + shouldBypass, + requestOptions, + ); let headers: { [key: string]: string } = {}; diff --git a/packages/fetch/src/httpAgentCache.test.ts b/packages/fetch/src/httpAgentCache.test.ts new file mode 100644 index 00000000000..48b64ac31d7 --- /dev/null +++ b/packages/fetch/src/httpAgentCache.test.ts @@ -0,0 +1,64 @@ +import { afterEach, expect, test } from "vitest"; +import { clearHttpAgentCache, getOrCreateAgent } from "./httpAgentCache.js"; + +afterEach(async () => { + await clearHttpAgentCache(); +}); + +test("reuses Agent for identical request options", async () => { + const first = await getOrCreateAgent("https:", undefined, true); + const second = await getOrCreateAgent("https:", undefined, true); + expect(second).toBe(first); +}); + +test("does not reuse Agent when verifySsl differs", async () => { + const trusted = await getOrCreateAgent("https:", undefined, true, { + verifySsl: true, + }); + const insecure = await getOrCreateAgent("https:", undefined, true, { + verifySsl: false, + }); + expect(insecure).not.toBe(trusted); +}); + +test("does not reuse Agent when proxy differs", async () => { + const direct = await getOrCreateAgent("https:", undefined, true); + const proxied = await getOrCreateAgent("https:", "http://127.0.0.1:9", false); + expect(proxied).not.toBe(direct); +}); + +test("does not reuse Agent when timeout or protocol differs", async () => { + const def = await getOrCreateAgent("https:", undefined, true); + const short = await getOrCreateAgent("https:", undefined, true, { + timeout: 30, + }); + const httpAgent = await getOrCreateAgent("http:", undefined, true); + expect(short).not.toBe(def); + expect(httpAgent).not.toBe(def); +}); + +test("treats bypassed proxy as the same as no proxy", async () => { + const direct = await getOrCreateAgent("https:", undefined, true); + const bypassed = await getOrCreateAgent("https:", "http://127.0.0.1:9", true); + expect(bypassed).toBe(direct); +}); + +test("created Agent has keepAlive enabled", async () => { + const agent = await getOrCreateAgent("https:", undefined, true); + expect((agent as { keepAlive?: boolean }).keepAlive).toBe(true); +}); + +test("clearHttpAgentCache drops the cached instance", async () => { + const first = await getOrCreateAgent("https:", undefined, true); + await clearHttpAgentCache(); + const second = await getOrCreateAgent("https:", undefined, true); + expect(second).not.toBe(first); +}); + +test("concurrent first requests share one Agent", async () => { + const [a, b] = await Promise.all([ + getOrCreateAgent("https:", undefined, true), + getOrCreateAgent("https:", undefined, true), + ]); + expect(a).toBe(b); +}); diff --git a/packages/fetch/src/httpAgentCache.ts b/packages/fetch/src/httpAgentCache.ts new file mode 100644 index 00000000000..b701a8dc300 --- /dev/null +++ b/packages/fetch/src/httpAgentCache.ts @@ -0,0 +1,84 @@ +import { RequestOptions } from "@continuedev/config-types"; +import * as followRedirects from "follow-redirects"; +import { HttpProxyAgent } from "http-proxy-agent"; +import { HttpsProxyAgent } from "https-proxy-agent"; +import { getAgentOptions } from "./getAgentOptions.js"; + +const { http, https } = (followRedirects as any).default; + +export function agentCacheKey( + protocol: string, + proxy: string | undefined, + shouldBypass: boolean, + requestOptions?: RequestOptions, +): string { + const ca = requestOptions?.caBundlePath; + const caPart = Array.isArray(ca) ? [...ca].sort().join("|") : (ca ?? ""); + const client = requestOptions?.clientCertificate; + return JSON.stringify({ + protocol, + proxy: shouldBypass ? "" : (proxy ?? ""), + timeout: requestOptions?.timeout ?? null, + verifySsl: requestOptions?.verifySsl ?? null, + ca: caPart, + cert: client?.cert ?? "", + key: client?.key ?? "", + passphrase: client?.passphrase ?? "", + }); +} + +const agentCache = new Map void }>>(); + +function createAgent( + protocol: string, + proxy: string | undefined, + shouldBypass: boolean, + agentOptions: { [key: string]: any }, +) { + const httpModule = protocol === "https:" ? https : http; + if (proxy && !shouldBypass) { + return protocol === "https:" + ? new HttpsProxyAgent(proxy, agentOptions) + : new HttpProxyAgent(proxy, agentOptions); + } + return new httpModule.Agent(agentOptions); +} + +export async function getOrCreateAgent( + protocol: string, + proxy: string | undefined, + shouldBypass: boolean, + requestOptions?: RequestOptions, +) { + const key = agentCacheKey(protocol, proxy, shouldBypass, requestOptions); + const cached = agentCache.get(key); + if (cached) { + return cached; + } + const pending = (async () => { + const agentOptions = await getAgentOptions(requestOptions); + return createAgent(protocol, proxy, shouldBypass, agentOptions); + })(); + agentCache.set(key, pending); + try { + return await pending; + } catch (error) { + if (agentCache.get(key) === pending) { + agentCache.delete(key); + } + throw error; + } +} + +export async function clearHttpAgentCache() { + const pending = [...agentCache.values()]; + agentCache.clear(); + for (const created of pending) { + try { + const agent = await created; + agent.destroy?.(); + } catch { + // Creation failed; nothing to destroy. + } + } +} diff --git a/packages/fetch/vitest.config.ts b/packages/fetch/vitest.config.ts index d4dab2104b8..0a29542812f 100644 --- a/packages/fetch/vitest.config.ts +++ b/packages/fetch/vitest.config.ts @@ -15,6 +15,7 @@ export default defineConfig({ "./util.js": "./util.ts", "./certs.js": "./certs.ts", "./fetch.js": "./fetch.ts", + "./httpAgentCache.js": "./httpAgentCache.ts", }, }, });