0.41.0 - Network errors with proxies, testCertificates request option

CHANGELOG.md

@@ -1,6 +1,10 @@
 # Change Log
 Notable changes will be documented here.
 
+## [0.41.0]
+- Surface network errors with proxies ([microsoft/vscode#298236](https://github.com/microsoft/vscode/issues/298236))
+- Support `testCertificates` request option for adding test certificates via request options instead of global state
+
 ## [0.40.0]
 - Surface `responseHeaders`, `responseStatusCode`, and `responseStatusText` on patched WebSocket for all response types (success, non-upgrade, and proxy errors) ([microsoft/vscode-proxy-agent#94](https://github.com/microsoft/vscode-proxy-agent/pull/94))
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@vscode/proxy-agent",
-  "version": "0.40.0",
+  "version": "0.41.0",
   "description": "NodeJS http(s) agent implementation for VS Code",
   "main": "out/index.js",
   "types": "out/index.d.ts",

src/agent.ts

@@ -269,4 +269,5 @@ type PacProxyAgentOptions =
 	fallbackToDirect?: boolean;
 	originalAgent?: false | http.Agent;
 	_vscodeTestReplaceCaCerts?: boolean;
+	testCertificates?: (string | Buffer)[];
 }

src/index.ts

@@ -162,7 +162,7 @@ export function createProxyResolver(params: ProxyAgentParams) {
 		results = [];
 	}
 
-	function resolveProxyWithRequest(flags: { useProxySettings: boolean, addCertificatesV1: boolean }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) {
+	function resolveProxyWithRequest(flags: { useProxySettings: boolean, addCertificatesV1: boolean, testCertificates?: (string | Buffer)[] }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) {
 		if (!timeout) {
 			timeout = setTimeout(logEvent, 10 * 60 * 1000);
 		}
@@ -175,7 +175,7 @@ export function createProxyResolver(params: ProxyAgentParams) {
 				return;
 			}
 			useProxySettings(url, req, stackText, callback);
-		});
+		}, flags.testCertificates);
 	}
 
 	function useProxySettings(url: string, req: http.ClientRequest | undefined, stackText: string, callback: (proxy?: string) => void) {
@@ -364,7 +364,7 @@ function noProxyFromConfig(noProxy: string[]) {
 
 export type ProxySupportSetting = 'override' | 'fallback' | 'on' | 'off';
 
-export type ResolveProxyWithRequest = (flags: { useProxySettings: boolean, addCertificatesV1: boolean }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) => void;
+export type ResolveProxyWithRequest = (flags: { useProxySettings: boolean, addCertificatesV1: boolean, testCertificates?: (string | Buffer)[] }, req: http.ClientRequest, opts: http.RequestOptions, url: string, callback: (proxy?: string) => void) => void;
 
 export function createHttpPatch(params: ProxyAgentParams, originals: typeof http | typeof https, resolveProxy: ResolveProxyWithRequest) {
 	return {
@@ -419,15 +419,15 @@ export function createHttpPatch(params: ProxyAgentParams, originals: typeof http
 				} else {
 					options = { ...options };
 				}
-				const resolveP = (req: http.ClientRequest, opts: http.RequestOptions, url: string): Promise<string | undefined> => new Promise<string | undefined>(resolve => resolveProxy({ useProxySettings, addCertificatesV1 }, req, opts, url, resolve));
+				const resolveP = (req: http.ClientRequest, opts: http.RequestOptions, url: string): Promise<string | undefined> => new Promise<string | undefined>(resolve => resolveProxy({ useProxySettings, addCertificatesV1, testCertificates: (options as SecureContextOptionsPatch).testCertificates }, req, opts, url, resolve));
 				const host = options.hostname || options.host;
 				const isLocalhost = !host || host === 'localhost' || host === '127.0.0.1'; // Avoiding https://github.com/microsoft/vscode/issues/120354
 				const agent = createPacProxyAgent(resolveP, {
 					originalAgent: (!useProxySettings || isLocalhost || config === 'fallback') ? originalAgent : undefined,
 					lookupProxyAuthorization: params.lookupProxyAuthorization,
 					// keepAlive: ((originalAgent || originals.globalAgent) as { keepAlive?: boolean }).keepAlive, // Skipping due to https://github.com/microsoft/vscode/issues/228872.
 					_vscodeTestReplaceCaCerts: (options as SecureContextOptionsPatch)._vscodeTestReplaceCaCerts,
-				}, opts => new Promise<void>(resolve => addCertificatesToOptionsV1(params, params.addCertificatesV1(), opts, resolve)));
+				}, opts => new Promise<void>(resolve => addCertificatesToOptionsV1(params, params.addCertificatesV1(), opts, resolve, (options as SecureContextOptionsPatch).testCertificates)));
 				agent.protocol = isHttps ? 'https:' : 'http:';
 				options.agent = agent
 				if (isHttps) {
@@ -445,6 +445,7 @@ export function createHttpPatch(params: ProxyAgentParams, originals: typeof http
 export interface SecureContextOptionsPatch {
 	_vscodeAdditionalCaCerts?: (string | Buffer)[];
 	_vscodeTestReplaceCaCerts?: boolean;
+	testCertificates?: (string | Buffer)[];
 }
 
 export function createNetPatch(params: ProxyAgentParams, originals: typeof net) {
@@ -501,6 +502,7 @@ function patchTlsConnect(params: ProxyAgentParams, original: typeof tls.connect)
 		if (!params.addCertificatesV2() || options?.ca) {
 			return original.apply(null, arguments as any);
 		}
+		const testCerts = (options as SecureContextOptionsPatch | undefined)?.testCertificates;
 		let secureConnectListener: (() => void) | undefined = args.find(arg => typeof arg === 'function');
 		if (!options) {
 			options = {};
@@ -532,12 +534,22 @@ function patchTlsConnect(params: ProxyAgentParams, original: typeof tls.connect)
 					for (const cert of _certs.get(!!params.loadSystemCertificatesFromNode())?.result || []) {
 						options!.secureContext!.context.addCACert(cert);
 					}
+					if (testCerts) {
+						for (const cert of testCerts) {
+							options!.secureContext!.context.addCACert(cert);
+						}
+					}
 				});
 			} else {
 				params.log.trace('ProxyResolver#tls.connect existing socket already connected - adding certs');
 				for (const cert of certificates) {
 					options!.secureContext!.context.addCACert(cert);
 				}
+				if (testCerts) {
+					for (const cert of testCerts) {
+						options!.secureContext!.context.addCACert(cert);
+					}
+				}
 			}
 		} else {
 			if (!options.secureContext) {
@@ -552,6 +564,11 @@ function patchTlsConnect(params: ProxyAgentParams, original: typeof tls.connect)
 					for (const cert of caCertificates) {
 						options!.secureContext!.context.addCACert(cert);
 					}
+					if (testCerts) {
+						for (const cert of testCerts) {
+							options!.secureContext!.context.addCACert(cert);
+						}
+					}
 					if (options?.timeout) {
 						socket.setTimeout(options.timeout);
 						socket.once('timeout', () => {
@@ -825,6 +842,11 @@ export function createWebSocketPatch(params: ProxyAgentParams, originalWebSocket
 			enumerable: true,
 			configurable: true,
 		});
+		Object.defineProperty(ws, 'networkError', {
+			get() { return proxyDispatcher.networkError; },
+			enumerable: true,
+			configurable: true,
+		});
 		return ws;
 	} as unknown as typeof globalThis.WebSocket;
 
@@ -843,6 +865,7 @@ class ProxyDispatcher extends undici.Dispatcher {
 	responseHeaders?: Record<string, string | string[] | undefined>;
 	responseStatusCode?: number;
 	responseStatusText?: string;
+	networkError?: Error;
 
 	constructor(
 		private params: ProxyAgentParams,
@@ -856,31 +879,53 @@ class ProxyDispatcher extends undici.Dispatcher {
 
 	dispatch(opts: undici.Dispatcher.DispatchOptions, handler: undici.Dispatcher.DispatchHandler): boolean {
 		const self = this;
+		function captureError(err: Error): void {
+			const connectHeaders = (err as any).connectResponseHeaders as IncomingHttpHeaders | undefined;
+			if (connectHeaders) {
+				self.responseHeaders = convertHeaders(connectHeaders);
+			}
+			if ((err as any).connectResponseStatusCode !== undefined) {
+				self.responseStatusCode = (err as any).connectResponseStatusCode;
+			}
+			if (!self.networkError) {
+				self.networkError = err;
+			}
+		}
 		const wrappedHandler: undici.Dispatcher.DispatchHandler = {
 			...handler,
-			// Deprecated API used by undici's internal WebSocket/fetch flow.
+			// Current API.
+			onResponseStart(controller: undici.Dispatcher.DispatchController, statusCode: number, headers: IncomingHttpHeaders, statusMessage?: string): void {
+				self.responseStatusCode = statusCode;
+				self.responseStatusText = statusMessage;
+				self.responseHeaders = convertHeaders(headers);
+				return handler.onResponseStart?.(controller, statusCode, headers, statusMessage);
+			},
+			onResponseError(controller: undici.Dispatcher.DispatchController, err: Error): void {
+				captureError(err);
+				return handler.onResponseError?.(controller, err);
+			},
+			onRequestUpgrade(controller: undici.Dispatcher.DispatchController, statusCode: number, headers: IncomingHttpHeaders, socket: stream.Duplex): void {
+				self.responseStatusCode = statusCode;
+				self.responseHeaders = convertHeaders(headers);
+				return handler.onRequestUpgrade?.(controller, statusCode, headers, socket);
+			},
+			// Deprecated API still used by undici's internal core.
 			onHeaders(statusCode: number, rawHeaders: Buffer[], resume: () => void, statusText: string): boolean {
 				self.responseStatusCode = statusCode;
 				self.responseStatusText = statusText;
 				self.responseHeaders = parseRawHeaders(rawHeaders);
-				return handler.onHeaders?.(statusCode, rawHeaders, resume, statusText) ?? true;
+				return (handler as any).onHeaders?.(statusCode, rawHeaders, resume, statusText) ?? true;
 			},
 			onError(err: Error): void {
-				const connectHeaders = (err as any).connectResponseHeaders as IncomingHttpHeaders | undefined;
-				if (connectHeaders) {
-					self.responseHeaders = convertHeaders(connectHeaders);
-				}
-				if ((err as any).connectResponseStatusCode !== undefined) {
-					self.responseStatusCode = (err as any).connectResponseStatusCode;
-				}
-				return handler.onError?.(err);
+				captureError(err);
+				return (handler as any).onError?.(err);
 			},
 			onUpgrade(statusCode: number, rawHeaders: Buffer[] | string[] | null, socket: stream.Duplex): void {
 				self.responseStatusCode = statusCode;
 				if (rawHeaders) {
 					self.responseHeaders = parseRawHeaders(rawHeaders);
 				}
-				return handler.onUpgrade?.(statusCode, rawHeaders, socket);
+				return (handler as any).onUpgrade?.(statusCode, rawHeaders, socket);
 			},
 		};
 		const url = opts.origin?.toString();
@@ -908,11 +953,7 @@ class ProxyDispatcher extends undici.Dispatcher {
 				}
 				dispatcher.dispatch(opts, wrappedHandler);
 			} catch (err: any) {
-				if (typeof (handler as any).onResponseError === 'function') {
-					(handler as any).onResponseError(null, err);
-				} else if (typeof (handler as any).onError === 'function') {
-					(handler as any).onError(err);
-				}
+				handler.onResponseError?.(null as any, err);
 			}
 		})();
 		return true;
@@ -1068,14 +1109,15 @@ function getAgentOptions(requestInit: RequestInit | undefined) {
 	return { dispatcher, allowH2, requestCA, proxyCA, socketPath };
 }
 
-function addCertificatesToOptionsV1(params: ProxyAgentParams, addCertificatesV1: boolean, opts: http.RequestOptions | tls.ConnectionOptions, callback: () => void) {
+function addCertificatesToOptionsV1(params: ProxyAgentParams, addCertificatesV1: boolean, opts: http.RequestOptions | tls.ConnectionOptions, callback: () => void, testCertificates?: (string | Buffer)[]) {
 	if (addCertificatesV1) {
 		getOrLoadAdditionalCertificates(params)
 			.then(caCertificates => {
+				const allCerts: (string | Buffer)[] = testCertificates?.length ? [...caCertificates, ...testCertificates] : caCertificates;
 				if ((opts as SecureContextOptionsPatch)._vscodeTestReplaceCaCerts) {
-					(opts as https.RequestOptions).ca = caCertificates;
+					(opts as https.RequestOptions).ca = allCerts;
 				} else {
-					(opts as SecureContextOptionsPatch)._vscodeAdditionalCaCerts = caCertificates;
+					(opts as SecureContextOptionsPatch)._vscodeAdditionalCaCerts = allCerts;
 				}
 				callback();
 			})
@@ -1242,7 +1284,7 @@ export function toLogString(args: any[]) {
 			const t = typeof value;
 			if (t === 'object') {
 				if (key) {
-					if ((key === 'ca' || key === '_vscodeAdditionalCaCerts') && Array.isArray(value)) {
+					if ((key === 'ca' || key === '_vscodeAdditionalCaCerts' || key === 'testCertificates') && Array.isArray(value)) {
 						return `[${value.length} certs]`;
 					}
 					if (key === 'ca' && (typeof value === 'string' || Buffer.isBuffer(value))) {
@@ -1267,9 +1309,4 @@ export function toLogString(args: any[]) {
 		})).join(', ')}]`;
 }
 
-/**
- * Certificates for testing. These are not automatically used, but can be added in
- * ProxyAgentParams#loadAdditionalCertificates(). This just provides a shared array
- * between production code and tests.
- */
-export const testCertificates: string[] = [];
+

tests/test-client/src/direct.test.ts

@@ -194,6 +194,28 @@ describe('Direct client', function () {
 			_vscodeTestReplaceCaCerts: true,
 		});
 	});
+	it('should use testCertificates request option', async function () {
+		const noAdditionalCertsParams: vpa.ProxyAgentParams = {
+			...directProxyAgentParamsV1,
+			loadAdditionalCertificates: async () => [],
+		};
+		vpa.resetCaches();
+		try {
+			const { resolveProxyWithRequest: resolveProxy } = vpa.createProxyResolver(noAdditionalCertsParams);
+			const patchedHttps: typeof https = {
+				...https,
+				...vpa.createHttpPatch(noAdditionalCertsParams, https, resolveProxy),
+			} as any;
+			await testRequest(patchedHttps, {
+				hostname: 'test-https-server',
+				path: '/test-path',
+				_vscodeTestReplaceCaCerts: true,
+				testCertificates: ca,
+			});
+		} finally {
+			vpa.resetCaches();
+		}
+	});
 	it('should use ca request option', async function () {
 		const { resolveProxyWithRequest: resolveProxy } = vpa.createProxyResolver(directProxyAgentParamsV1);
 		const patchedHttps: typeof https = {

tests/test-client/src/websocket.test.ts

@@ -164,4 +164,48 @@ describe('WebSocket proxied', function () {
 			};
 		});
 	});
+
+	it('should capture networkError on HTTP proxy connection refused', async function () {
+		const params: vpa.ProxyAgentParams = {
+			...directProxyAgentParams,
+			resolveProxy: async () => 'PROXY 127.0.0.1:1',
+		};
+		const { resolveProxyURL } = vpa.createProxyResolver(params);
+		const PatchedWebSocket = vpa.createWebSocketPatch(params, WebSocket as any, resolveProxyURL);
+		const ws = new PatchedWebSocket('wss://test-wss-server');
+		await new Promise<void>((resolve, reject) => {
+			ws.onopen = () => {
+				ws.close();
+				reject(new Error('WebSocket should not have connected'));
+			};
+			ws.onerror = () => {
+				const networkError: Error | undefined = (ws as any).networkError;
+				assert.ok(networkError, 'networkError should be defined');
+				assert.ok(networkError instanceof Error, 'networkError should be an Error');
+				resolve();
+			};
+		});
+	});
+
+	it('should capture networkError on HTTPS proxy connection refused', async function () {
+		const params: vpa.ProxyAgentParams = {
+			...directProxyAgentParams,
+			resolveProxy: async () => 'HTTPS 127.0.0.1:1',
+		};
+		const { resolveProxyURL } = vpa.createProxyResolver(params);
+		const PatchedWebSocket = vpa.createWebSocketPatch(params, WebSocket as any, resolveProxyURL);
+		const ws = new PatchedWebSocket('wss://test-wss-server');
+		await new Promise<void>((resolve, reject) => {
+			ws.onopen = () => {
+				ws.close();
+				reject(new Error('WebSocket should not have connected'));
+			};
+			ws.onerror = () => {
+				const networkError: Error | undefined = (ws as any).networkError;
+				assert.ok(networkError, 'networkError should be defined');
+				assert.ok(networkError instanceof Error, 'networkError should be an Error');
+				resolve();
+			};
+		});
+	});
 });