feat: add a custom proxyAgent option to the Node server SDK (#1438)

Author: bradbunce

packages/sdk/server-node/__tests__/LDClientNode.proxyAgent.test.ts

@@ -0,0 +1,90 @@
+import * as http from 'http';
+
+import {
+  AsyncQueue,
+  SSEItem,
+  TestHttpHandlers,
+  TestHttpServer,
+} from 'launchdarkly-js-test-helpers';
+
+import { basicLogger, LDLogger } from '../src';
+import LDClientNode from '../src/LDClientNode';
+
+const sdkKey = 'sdkKey';
+const flagKey = 'flagKey';
+const expectedFlagValue = 'yes';
+const flag = {
+  key: flagKey,
+  version: 1,
+  on: false,
+  offVariation: 0,
+  variations: [expectedFlagValue, 'no'],
+};
+const allData = { flags: { flagKey: flag }, segments: {} };
+
+// This proves the proxyAgent option actually reaches the underlying request, through the full
+// LDOptions -> NodePlatform -> NodeRequests wiring, rather than only the NodeRequests constructor.
+describe('When using a custom proxyAgent', () => {
+  let logger: LDLogger;
+  let closeable: { close: () => void }[];
+
+  beforeEach(() => {
+    closeable = [];
+    logger = basicLogger({
+      destination: () => {},
+    });
+  });
+
+  afterEach(() => {
+    closeable.forEach((item) => item.close());
+  });
+
+  it('uses the supplied proxyAgent for polling requests', async () => {
+    const server = await TestHttpServer.start();
+    server.forMethodAndPath('get', '/sdk/latest-all', TestHttpHandlers.respondJson(allData));
+
+    const agent = new http.Agent({ keepAlive: false });
+    // addRequest exists at runtime but is not part of the public http.Agent type.
+    // @ts-ignore
+    const addRequestSpy = jest.spyOn(agent, 'addRequest');
+
+    const client = new LDClientNode(sdkKey, {
+      baseUri: server.url,
+      proxyAgent: agent,
+      stream: false,
+      sendEvents: false,
+      logger,
+    });
+
+    closeable.push(server, client);
+
+    await client.waitForInitialization({ timeout: 10 });
+    expect(client.initialized()).toBe(true);
+    expect(addRequestSpy).toHaveBeenCalled();
+  });
+
+  it('uses the supplied proxyAgent for streaming requests', async () => {
+    const server = await TestHttpServer.start();
+    const events = new AsyncQueue<SSEItem>();
+    events.add({ type: 'put', data: JSON.stringify({ data: allData }) });
+    server.forMethodAndPath('get', '/all', TestHttpHandlers.sseStream(events));
+
+    const agent = new http.Agent({ keepAlive: false });
+    // addRequest exists at runtime but is not part of the public http.Agent type.
+    // @ts-ignore
+    const addRequestSpy = jest.spyOn(agent, 'addRequest');
+
+    const client = new LDClientNode(sdkKey, {
+      streamUri: server.url,
+      proxyAgent: agent,
+      sendEvents: false,
+      logger,
+    });
+
+    closeable.push(server, events, client);
+
+    await client.waitForInitialization({ timeout: 10 });
+    expect(client.initialized()).toBe(true);
+    expect(addRequestSpy).toHaveBeenCalled();
+  });
+});

packages/sdk/server-node/__tests__/platform/NodeRequests.test.ts

@@ -214,7 +214,7 @@ describe('given a request to a server that never responds', () => {
 });
 
 describe('given an instance of NodeRequests with enableEventCompression turned on', () => {
-  const requests = new NodeRequests(undefined, undefined, undefined, true);
+  const requests = new NodeRequests(undefined, undefined, undefined, undefined, true);
   it('can make a basic post with compressBodyIfPossible enabled', async () => {
     await requests.fetch(`http://localhost:${PORT}`, {
       method: 'POST',
@@ -239,3 +239,42 @@ describe('given an instance of NodeRequests with enableEventCompression turned o
     expect(serverResult.body).toEqual('BODY TEXT');
   });
 });
+
+describe('given an instance of NodeRequests with a proxyAgent and proxyOptions both supplied', () => {
+  it('logs a warning and uses the supplied proxyAgent instead of building one from proxyOptions', async () => {
+    const agent = new http.Agent({ keepAlive: false });
+    // addRequest exists at runtime but is not part of the public http.Agent type.
+    // @ts-ignore
+    const addRequestSpy = jest.spyOn(agent, 'addRequest');
+    const logger = { warn: jest.fn(), error: jest.fn(), info: jest.fn(), debug: jest.fn() };
+
+    const requests = new NodeRequests(
+      undefined,
+      { host: 'proxy.invalid', port: 8080 },
+      agent,
+      logger,
+    );
+    await requests.fetch(`http://localhost:${PORT}`);
+
+    expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('proxyAgent'));
+    expect(addRequestSpy).toHaveBeenCalled();
+  });
+});
+
+describe('given an instance of NodeRequests with only a proxyAgent supplied', () => {
+  it('uses the supplied agent for requests and reports usingProxy as a best-effort true', async () => {
+    const agent = new http.Agent({ keepAlive: false });
+    // addRequest exists at runtime but is not part of the public http.Agent type.
+    // @ts-ignore
+    const addRequestSpy = jest.spyOn(agent, 'addRequest');
+
+    const requests = new NodeRequests(undefined, undefined, agent);
+    await requests.fetch(`http://localhost:${PORT}`);
+
+    expect(addRequestSpy).toHaveBeenCalled();
+    // The SDK can't verify a caller-supplied agent is actually proxying (it could just as easily
+    // be for mTLS), but reporting true is the better default for this option's motivating case
+    // (a SOCKS proxy), so usingProxy() treats any supplied proxyAgent as a best-effort signal.
+    expect(requests.usingProxy()).toBe(true);
+  });
+});

packages/sdk/server-node/src/api/LDOptions.ts

@@ -1,3 +1,6 @@
+import * as http from 'http';
+import * as https from 'https';
+
 import { LDOptions as LDOptionsCommon } from '@launchdarkly/js-server-sdk-common';
 
 import { LDPlugin } from './LDPlugin';
@@ -16,4 +19,24 @@ export interface LDOptions extends LDOptionsCommon {
    * Plugin support is currently experimental and subject to change.
    */
   plugins?: LDPlugin[];
+
+  /**
+   * An HTTP(S) agent used for all outgoing SDK connections.
+   *
+   * This is an extension point for proxy configurations the SDK does not build itself. For a
+   * basic HTTP/HTTPS proxy, prefer {@link LDOptionsCommon.proxyOptions}. For other schemes (for
+   * example a SOCKS proxy), construct the appropriate agent (such as `SocksProxyAgent` from the
+   * `socks-proxy-agent` package) and supply it here.
+   *
+   * When this is set, `proxyOptions` and `tlsParams` are ignored, because the agent is
+   * responsible for connection and TLS setup.
+   *
+   * @remarks
+   * The SDK cannot inspect a caller-supplied agent, so diagnostic reporting treats any agent
+   * supplied here as a best-effort signal that a proxy is in use. This may be inaccurate if the
+   * agent is used for something other than proxying, such as custom certificates. Credentials
+   * carried by the agent itself (for example a SOCKS URL's embedded username/password) are not
+   * reflected in proxy-authentication diagnostics, since the SDK has no way to inspect them.
+   */
+  proxyAgent?: https.Agent | http.Agent;
 }

packages/sdk/server-node/src/platform/NodePlatform.ts

@@ -1,5 +1,6 @@
-import { LDOptions, platform } from '@launchdarkly/js-server-sdk-common';
+import { platform } from '@launchdarkly/js-server-sdk-common';
 
+import { LDOptions } from '../api/LDOptions';
 import NodeCrypto from './NodeCrypto';
 import NodeFilesystem from './NodeFilesystem';
 import NodeInfo from './NodeInfo';
@@ -19,6 +20,7 @@ export default class NodePlatform implements platform.Platform {
     this.requests = new NodeRequests(
       options.tlsParams,
       options.proxyOptions,
+      options.proxyAgent,
       options.logger,
       options.enableEventCompression,
     );

packages/sdk/server-node/src/platform/NodeRequests.ts

@@ -98,6 +98,27 @@ function createAgent(
   return undefined;
 }
 
+// A caller-supplied agent is a completely different way of constructing the agent than the
+// tlsOptions/proxyOptions path above: it takes precedence and is used verbatim, covering proxy
+// schemes the SDK does not build itself (for example a SOCKS proxy via socks-proxy-agent). When
+// it is set, proxyOptions and tlsParams are ignored because the agent owns connection setup.
+function resolveAgent(
+  tlsOptions?: LDTLSOptions,
+  proxyOptions?: LDProxyOptions,
+  proxyAgent?: https.Agent | http.Agent,
+  logger?: LDLogger,
+): https.Agent | http.Agent | undefined {
+  if (proxyAgent) {
+    if (proxyOptions || tlsOptions) {
+      logger?.warn(
+        'Both proxyAgent and proxyOptions/tlsParams were provided; using proxyAgent and ignoring proxyOptions/tlsParams.',
+      );
+    }
+    return proxyAgent;
+  }
+  return createAgent(tlsOptions, proxyOptions, logger);
+}
+
 export default class NodeRequests implements platform.Requests {
   private _agent: https.Agent | http.Agent | undefined;
 
@@ -112,11 +133,17 @@ export default class NodeRequests implements platform.Requests {
   constructor(
     tlsOptions?: LDTLSOptions,
     proxyOptions?: LDProxyOptions,
+    proxyAgent?: https.Agent | http.Agent,
     logger?: LDLogger,
     enableEventCompression?: boolean,
   ) {
-    this._agent = createAgent(tlsOptions, proxyOptions, logger);
-    this._hasProxy = !!proxyOptions;
+    this._agent = resolveAgent(tlsOptions, proxyOptions, proxyAgent, logger);
+    // A caller-supplied proxyAgent is treated as a best-effort proxy signal: the SDK cannot
+    // inspect an opaque agent to know whether it actually proxies (it could just as easily be a
+    // certificate-only agent for mTLS). Reporting true is the better default here because
+    // proxying is this option's primary motivation, while other connection concerns (such as
+    // custom TLS) are handled by tlsParams.
+    this._hasProxy = !!proxyOptions || !!proxyAgent;
     this._hasProxyAuth = !!proxyOptions?.auth;
     this._enableBodyCompression = !!enableEventCompression;
   }