Skip to content

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

Merged
joker23 merged 5 commits into
launchdarkly:mainfrom
bradbunce:socks-proxy-support
Jul 1, 2026
Merged

feat: add a custom proxyAgent option to the Node server SDK#1438
joker23 merged 5 commits into
launchdarkly:mainfrom
bradbunce:socks-proxy-support

Conversation

@bradbunce

@bradbunce bradbunce commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Requirements

  • I have added test coverage for new or changed functionality
  • I have followed the repository's pull request submission guidelines
  • I have validated my changes against all supported platform versions

Related issues

N/A — enables proxy configurations (SOCKS in particular) for environments where outbound access to LaunchDarkly is only available through a proxy the SDK does not build itself.

Describe the solution you've provided

Adds a Node-specific proxyAgent option to @launchdarkly/node-server-sdk: an http/https agent that the SDK uses for all outgoing connections (polling fetch and the streaming event source). This is an extension point for proxy schemes the SDK does not construct itself — a SOCKS proxy being the motivating example. The application builds its own agent (e.g. new SocksProxyAgent('socks5://host:port')) and hands it to the SDK.

This mirrors how the Java server SDK lets developers bring their own socketFactory, and keeps proxy-scheme knowledge (and correctness) out of the SDK.

  • proxyAgent is added to the server-node LDOptions and wired through NodePlatform into NodeRequests.
  • When proxyAgent is set, it is used verbatim and proxyOptions/tlsParams are ignored (with a warning), because the supplied agent owns connection and TLS setup. usingProxy() reports true for a custom agent.
  • When proxyAgent is omitted, behavior is unchanged: the existing https-proxy-agent path still handles proxyOptions with http/https schemes.
  • No new runtime dependency. socks-proxy-agent is only a devDependency, used by the tests.

Describe alternatives you've considered

  • Teaching the SDK about SOCKS schemes directly (the original version of this PR): adding socks/socks4/socks4a/socks5/socks5h handling in proxyOptions backed by a socks-proxy-agent runtime dependency. Per maintainer feedback this was reworked into the agent hook — it covers SOCKS and any other scheme without the SDK owning that logic, avoids the dependency, and matches the Java SDK's bring-your-own approach.

Additional context

End-to-end tests drive a user-supplied SocksProxyAgent through the hook in polling, streaming, and username/password-auth modes, backed by a minimal RFC 1928/1929 SOCKS5 test server (__tests__/socksProxyServer.ts). The full server-node test suite passes and lint is clean. TESTING.md documents how to consume a local build and configure proxyAgent. Relying on CI for the full supported-version matrix (hence the third box left unchecked).


Note

Medium Risk
Changes how outbound HTTP/SSE connections are established for all SDK traffic when the option is used; misconfigured agents could break connectivity, though default paths are unchanged.

Overview
Adds a Node-specific proxyAgent initialization option so apps can supply their own http/https agent for all SDK outbound traffic (polling and streaming), enabling proxy schemes the SDK does not build (e.g. SOCKS via SocksProxyAgent) without new runtime dependencies.

When proxyAgent is set, it is used as-is; proxyOptions and tlsParams are ignored and a warning is logged if both are provided. usingProxy() treats a custom agent as a best-effort “proxy in use” signal. Existing proxyOptions behavior is unchanged when proxyAgent is omitted.

NodeRequests gains a new constructor parameter (shifting enableEventCompression); tests cover precedence, warnings, end-to-end client wiring for poll and stream, and agent usage on fetch.

Reviewed by Cursor Bugbot for commit 4702973. Bugbot is set up for automated code reviews on this repo. Configure here.


Open in Devin Review

Support SOCKS proxies via LDProxyOptions.scheme. Setting scheme to one of
socks, socks4, socks4a, socks5, or socks5h routes both fetch and
event-source traffic through a SOCKS proxy using socks-proxy-agent; a
single agent handles both http and https targets.

The proxy address is built as a URL so its username/password setters
percent-encode the credentials, which socks-proxy-agent then decodes, so
an auth password may safely contain characters such as ':'. TLS agent
options are forwarded to the SOCKS agent for https targets.

Adds end-to-end tests (polling, streaming, and username/password auth)
backed by a minimal RFC 1928/1929 SOCKS5 test server.
@bradbunce bradbunce requested a review from a team as a code owner June 9, 2026 21:36
@bradbunce

Copy link
Copy Markdown
Contributor Author

Heads up on CI: the failing checks are all example-app smoke tests (run-example, run-bootstrap-example, run-getting-started-example, run-react-server-example). They fail at the aws-actions/configure-aws-credentials step ("Could not load credentials from any providers") because this PR is from a fork, and fork PRs don't have access to the repo's AWS/OIDC secrets used to fetch the example SDK keys.

All SDK build/test checks relevant to this change pass, including build-test-server-node (20) / (22), build-test-sdk-server, and build-test-common. The example jobs should pass when re-run with the org's credentials.

@joker23

joker23 commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Hey @bradbunce thanks for the contribution. I will need to speak with the team to see if we agree that this is a broad SDK feature that we want to pursue.

UPDATE
Thinking about this more, I think a better approach here is to expose a way for developers to bring their own proxy agent. For example I think our Java server sdk could support SOCKS proxy because developers can specify a sockFactory implementation. In the node server sdk, allowing developers to specify a proxy agent implementation is a close equivalent (and would capture this feature)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
cursor[bot]

This comment was marked as resolved.

bradbunce and others added 2 commits June 29, 2026 11:12
Build the SOCKS proxy address with formatUrl (the same handling the HTTP
proxy path uses) so IPv6 literal hosts are bracketed and a missing port is
omitted rather than appended as the literal string 'undefined'. Also strip
the brackets off the parsed proxy host, since socks-proxy-agent derives it
from URL.hostname and hands the bracketed literal to net.connect, which
cannot resolve it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per maintainer feedback, expose a Node-specific `proxyAgent` option instead
of teaching the SDK about SOCKS schemes. Applications construct their own
http(s) agent (for example a SocksProxyAgent) and the SDK uses it for all
connections; this covers SOCKS and any other proxy scheme without the SDK
owning that logic, and drops the socks-proxy-agent runtime dependency.

When proxyAgent is set, proxyOptions and tlsParams are ignored (with a
warning) because the agent owns connection and TLS setup.

- Add `proxyAgent` to server-node LDOptions; forward it through NodePlatform
  and NodeRequests.
- Remove SOCKS scheme handling from NodeRequests and the socks-proxy-agent
  dependency (kept as a devDependency for the proxy test).
- Revert the SOCKS wording added to the shared LDProxyOptions docs.
- Rework the proxy test to drive a user-supplied SocksProxyAgent through the
  hook; update TESTING.md accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@bradbunce bradbunce changed the title feat: add SOCKS proxy support to server-node SDK feat: add a custom proxyAgent option to the Node server SDK Jun 29, 2026
@bradbunce

Copy link
Copy Markdown
Contributor Author

@joker23 thanks — that makes sense, and I've reworked the PR along those lines. Rather than the SDK knowing about SOCKS schemes, it now exposes a proxyAgent option: the app constructs its own http/https agent (e.g. new SocksProxyAgent(...)) and the SDK uses it for all outbound traffic (polling + streaming). This is the Node equivalent of bring-your-own socketFactory, covers SOCKS and any other scheme without the SDK owning that logic, and drops the socks-proxy-agent runtime dependency (it's now just a devDependency for the tests). When proxyAgent is set, proxyOptions/tlsParams are ignored with a warning. Updated description and tests are pushed — happy to adjust further.

Comment thread packages/sdk/server-node/src/platform/NodeRequests.ts
Comment thread packages/sdk/server-node/src/platform/NodeRequests.ts Outdated
Comment thread packages/sdk/server-node/__tests__/socksProxyServer.ts Outdated
Comment thread packages/sdk/server-node/TESTING.md Outdated
@joker23

joker23 commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Hey @bradbunce thanks for the revision, the PR looks a lot better... I've added some more comments. I think some of comments could be address in later PRs (eg creating example SOCKS implementation).

NOTE: I created an internal ticket to track this as SDK-2508.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit 4702973. Configure here.

// 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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proxy auth diagnostics ignore agent precedence

Low Severity

When proxyAgent and proxyOptions are both set, the SDK warns and uses only proxyAgent, but usingProxyAuth() still reflects proxyOptions.auth. Init diagnostics can report proxy authentication in use even though that auth is not applied to outbound traffic.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 4702973. Configure here.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@joker23 joker23 merged commit c74dae7 into launchdarkly:main Jul 1, 2026
48 of 54 checks passed
@github-actions github-actions Bot mentioned this pull request Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants