Skip to content

feat: support function-based dynamic target for ServerOptions#56

Open
tbontb-iaq wants to merge 2 commits into
sagemathinc:mainfrom
tbontb-iaq:feat/dynamic-target
Open

feat: support function-based dynamic target for ServerOptions#56
tbontb-iaq wants to merge 2 commits into
sagemathinc:mainfrom
tbontb-iaq:feat/dynamic-target

Conversation

@tbontb-iaq

Copy link
Copy Markdown

Allow target option to accept a function (req) => ProxyTarget in addition to a static ProxyTarget, enabling per-request dynamic routing.

Changes

  • lib/http-proxy/index.ts: Added ProxyTargetFunction type, changed ServerOptions.target to accept ProxyTarget | ProxyTargetFunction. Function target is resolved in createRightProxy before any passes run, so all downstream code sees a resolved static target.
  • lib/index.ts: Exported ProxyTargetFunction type.
  • lib/test/http/dynamic-target.test.ts: 6 new tests covering path-based routing, {port, host} object return, and per-request override.

Example

const proxy = createProxyServer({
  target: (req) => {
    if (req.url?.startsWith("/api/v2")) return "http://localhost:3001";
    return "http://localhost:3000";
  },
});

🤖 This code was written by AI (OpenCode / DeepSeek v4 Pro).

Allow `target` option to accept a function `(req) => ProxyTarget` in addition
to a static `ProxyTarget`, enabling per-request dynamic routing. The function
is resolved in createRightProxy before any passes run, so all downstream code
sees a resolved static target.

🤖 This code was written by AI (OpenCode / DeepSeek v4 Pro).
@williamstein

Copy link
Copy Markdown
Contributor

Wow, that's a good idea, I think. Code looks good.

@williamstein

Copy link
Copy Markdown
Contributor

Further feedback: Tests include a placeholder-ish override test, there is no websocket coverage even though the change affects ws too, and a throwing target function currently looks like it would escape instead of going through proxy error handling. Can you clean this up a bit?

…up tests

Address review feedback:
- Wrap function target call in try-catch to emit errors properly
- Add websocket dynamic target test coverage
- Remove placeholder-like per-request override test
- Add test for target function throwing an error

🤖 Code written by AI (OpenCode / DeepSeek v4 Pro).
@tbontb-iaq

Copy link
Copy Markdown
Author

Thanks for the review! I've addressed all three points in b0063bf:

  1. Removed the placeholder test — the first per-request "override" test was messy with dead code and unused target2, didn't actually test override. Kept the real one that tests proxy.web(req, res, { target: ... }) override.

  2. Added WebSocket test coveragelib/test/websocket/dynamic-target-ws.test.ts creates two WS echo servers and uses a dynamic target function to route /ws1 to one and /ws2 to the other, verifying correct upgrade and routing.

  3. Wrapped function target call in try-catch — if the target function throws, the error is now emitted via this.emit("error", err, req, res) and the request is aborted (same pattern as the existing "Must set target or forward" error handling).

All 310 tests pass, no regressions.

@williamstein

Copy link
Copy Markdown
Contributor

Thanks!

@tbontb-iaq

Copy link
Copy Markdown
Author

Is there any progress?

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.

2 participants