fix(proxy): stabilize upstream fetch transport

Refs #1686
This commit is contained in:
Tam Nhu Tran committed 2026-08-08 21:17:13 -04:00
1 parent 7fb56d62e1
commit d8210cf011
17 files changed
+889 -85

No files matched your search

+74 -4
View File
@@ -9,8 +9,13 @@ import { resolveOpenAICompatProfileConfig } from '../../../src/proxy/profile-rou
import {
attachDisconnectAbortHandlers,
buildUpstreamAgentTimeouts,
handleProxyMessagesRequest,
handleProxyMessagesRequest as handleProxyMessagesRequestImpl,
} from '../../../src/proxy/server/messages-route';
import {
closeUpstreamDispatcher,
isBunRuntime,
toLogErrorInfo,
} from '../../../src/proxy/server/upstream-transport';
import { loadSettings } from '../../../src/utils/config-manager';
class FakeSocket extends EventEmitter {
@@ -70,6 +75,24 @@ function buildProfile(profileName: string) {
return profile!;
}
function handleProxyMessagesRequest(
req: Parameters<typeof handleProxyMessagesRequestImpl>[0],
res: Parameters<typeof handleProxyMessagesRequestImpl>[1],
profile: Parameters<typeof handleProxyMessagesRequestImpl>[2],
expectedAuthToken: string,
insecureDispatcher?: Parameters<typeof handleProxyMessagesRequestImpl>[4]
) {
return handleProxyMessagesRequestImpl(
req,
res,
profile,
expectedAuthToken,
insecureDispatcher,
undefined,
globalThis.fetch
);
}
beforeEach(() => {
originalCcsHome = process.env.CCS_HOME;
originalFetch = globalThis.fetch;
@@ -191,6 +214,45 @@ describe('buildUpstreamAgentTimeouts', () => {
});
});
describe('closeUpstreamDispatcher', () => {
it('closes an owned dispatcher once across repeated cleanup', async () => {
let closeCalls = 0;
const dispatcher = {
close: async () => {
closeCalls += 1;
},
} as never;
await Promise.all([
closeUpstreamDispatcher(dispatcher),
closeUpstreamDispatcher(dispatcher),
closeUpstreamDispatcher(dispatcher),
]);
expect(closeCalls).toBe(1);
});
});
describe('toLogErrorInfo', () => {
it('preserves nested transport cause metadata for diagnostics', () => {
const cause = Object.assign(new Error('invalid onError method'), {
name: 'InvalidArgumentError',
code: 'UND_ERR_INVALID_ARG',
});
const error = new TypeError('fetch failed', { cause });
expect(toLogErrorInfo(error)).toEqual({
name: 'TypeError',
message: 'fetch failed',
cause: {
name: 'InvalidArgumentError',
message: 'invalid onError method',
code: 'UND_ERR_INVALID_ARG',
},
});
});
});
describe('handleProxyMessagesRequest', () => {
it('dispatches secure upstream fetches with an explicit dispatcher (not undici defaults)', async () => {
const activeProfile = buildProfile('hf');
@@ -217,7 +279,11 @@ describe('handleProxyMessagesRequest', () => {
// A bare global-dispatcher fallback would reapply undici's 300s
// headersTimeout/bodyTimeout and undercut the proxy's request timeout.
expect(capturedDispatcher).toBeDefined();
if (isBunRuntime()) {
expect(capturedDispatcher).toBeUndefined();
} else {
expect(capturedDispatcher).toBeDefined();
}
expect(res.statusCode).toBe(502);
});
@@ -403,9 +469,13 @@ describe('handleProxyMessagesRequest', () => {
);
await pending;
expect(capturedDispatcher).toBeInstanceOf(Agent);
if (isBunRuntime()) {
expect(capturedDispatcher).toBeUndefined();
} else {
expect(capturedDispatcher).toBeInstanceOf(Agent);
}
expect(capturedDispatcher).not.toBe(sharedDispatcher);
expect(closeCalls).toBe(1);
expect(closeCalls).toBe(isBunRuntime() ? 0 : 1);
expect(res.statusCode).toBe(502);
});
@@ -63,7 +63,9 @@ beforeEach(() => {
baselineUncaughtExceptionListeners = process.listeners('uncaughtException');
baselineUnhandledRejectionListeners = process.listeners('unhandledRejection');
mock.module('../../../../src/utils/fetch-proxy-setup', () => ({}));
mock.module('../../../../src/utils/fetch-proxy-setup', () => ({
applyGlobalFetchProxy: () => ({ enabled: false }),
}));
mock.module('../../../../src/utils/error-manager', () => ({
ErrorManager: class ErrorManager {
static async showProfileNotFound(): Promise<void> {}
@@ -1,5 +1,9 @@
import { describe, expect, it } from 'bun:test';
import { redactContext } from '../../../../src/services/logging/log-redaction';
import {
maskSecretTokens,
redactContext,
redactErrorInfo,
} from '../../../../src/services/logging/log-redaction';
describe('log redaction', () => {
it('redacts sensitive keys and preserves non-sensitive values', () => {
@@ -100,4 +104,31 @@ describe('log redaction', () => {
},
});
});
it('redacts proxy URL userinfo from nested error causes', () => {
const redacted = redactErrorInfo({
name: 'TypeError',
message: 'fetch failed',
cause: {
name: 'ProxyError',
code: 'ECONNREFUSED',
message: 'connect http://sentinel-user:sentinel-password@proxy.example:8080',
},
});
expect(redacted?.cause).toEqual({
name: 'ProxyError',
code: 'ECONNREFUSED',
message: 'connect http://[redacted]@proxy.example:8080',
});
expect(JSON.stringify(redacted)).not.toContain('sentinel-user');
expect(JSON.stringify(redacted)).not.toContain('sentinel-password');
});
it('redacts URL userinfo without changing non-credential URLs', () => {
expect(maskSecretTokens('https://user:pass@example.com/path')).toBe(
'https://[redacted]@example.com/path'
);
expect(maskSecretTokens('https://example.com/path')).toBe('https://example.com/path');
});
});
+25 -19
View File
@@ -4,7 +4,7 @@ import {
Agent,
Dispatcher,
ProxyAgent,
fetch,
fetch as undiciFetch,
getGlobalDispatcher,
setGlobalDispatcher,
} from 'undici';
@@ -26,18 +26,24 @@ const PROXY_ENV_KEYS = [
describe('global fetch proxy setup', () => {
const originalEnv = new Map<string, string | undefined>();
const originalFetch = globalThis.fetch;
const originalDispatcher = getGlobalDispatcher();
beforeEach(() => {
globalThis.fetch = originalFetch;
setGlobalDispatcher(originalDispatcher);
for (const key of PROXY_ENV_KEYS) {
originalEnv.set(key, process.env[key]);
delete process.env[key];
}
setGlobalDispatcher(originalDispatcher);
});
afterEach(() => {
afterEach(async () => {
const activeDispatcher = getGlobalDispatcher();
globalThis.fetch = originalFetch;
setGlobalDispatcher(originalDispatcher);
if (activeDispatcher !== originalDispatcher) {
await activeDispatcher.close();
}
for (const key of PROXY_ENV_KEYS) {
const value = originalEnv.get(key);
if (value === undefined) {
@@ -112,18 +118,6 @@ describe('global fetch proxy setup', () => {
});
});
it('rebinds globalThis.fetch to undici fetch when proxying is enabled', () => {
const originalFetch = globalThis.fetch;
process.env.HTTP_PROXY = 'http://proxy.example:8080';
try {
expect(applyGlobalFetchProxy()).toEqual({ enabled: true });
expect(globalThis.fetch).toBe(fetch);
} finally {
globalThis.fetch = originalFetch;
}
});
it('bypasses loopback fetches even when HTTP_PROXY is set', async () => {
process.env.HTTP_PROXY = 'http://127.0.0.1:9';
@@ -139,11 +133,14 @@ describe('global fetch proxy setup', () => {
}
try {
expect(applyGlobalFetchProxy()).toEqual({ enabled: true });
const response = await fetch(`http://127.0.0.1:${address.port}/health`);
const dispatcher = createGlobalFetchProxyDispatcher();
expect(dispatcher).not.toBeNull();
const response = await undiciFetch(`http://127.0.0.1:${address.port}/health`, {
dispatcher: dispatcher!,
});
expect(response.status).toBe(200);
expect(await response.text()).toBe('ok');
await dispatcher?.close();
} finally {
await new Promise<void>((resolve, reject) => {
server.close((error) => (error ? reject(error) : resolve()));
@@ -158,6 +155,15 @@ describe('global fetch proxy setup', () => {
expect(result).toEqual({ directCalls: 0, proxyCalls: 1 });
});
it('applies the proxy transport only when explicitly invoked', () => {
process.env.HTTP_PROXY = 'http://proxy.example:8080';
expect(globalThis.fetch).toBe(originalFetch);
expect(applyGlobalFetchProxy()).toEqual({ enabled: true });
expect(globalThis.fetch).toBe(undiciFetch);
expect(getGlobalDispatcher()).not.toBe(originalDispatcher);
});
it('routes HTTPS requests through HTTPS_PROXY', async () => {
process.env.HTTPS_PROXY = 'http://proxy.example:8443';