Skip to content

Commit 87876a3

Browse files
TakhoffmanTakhoffman
authored
Fix env proxy bootstrap for model traffic (#43248)
* Fix env proxy bootstrap for model traffic * Address proxy dispatcher review followups * Fix proxy env precedence for empty lowercase vars
1 parent 1435fce commit 87876a3

9 files changed

Lines changed: 208 additions & 33 deletions

File tree

‎src/agents/pi-embedded-runner/run/attempt.spawn-workspace.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ vi.mock("../../../infra/machine-name.js", () => ({
7979
}));
8080

8181
vi.mock("../../../infra/net/undici-global-dispatcher.js", () => ({
82+
ensureGlobalUndiciEnvProxyDispatcher: () => {},
8283
ensureGlobalUndiciStreamTimeouts: () => {},
8384
}));
8485

‎src/agents/pi-embedded-runner/run/attempt.ts‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,10 @@ import { resolveHeartbeatPrompt } from "../../../auto-reply/heartbeat.js";
1111
import { resolveChannelCapabilities } from "../../../config/channel-capabilities.js";
1212
import type { OpenClawConfig } from "../../../config/config.js";
1313
import { getMachineDisplayName } from "../../../infra/machine-name.js";
14-
import { ensureGlobalUndiciStreamTimeouts } from "../../../infra/net/undici-global-dispatcher.js";
14+
import {
15+
ensureGlobalUndiciEnvProxyDispatcher,
16+
ensureGlobalUndiciStreamTimeouts,
17+
} from "../../../infra/net/undici-global-dispatcher.js";
1518
import { MAX_IMAGE_BYTES } from "../../../media/constants.js";
1619
import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js";
1720
import type {
@@ -749,6 +752,9 @@ export async function runEmbeddedAttempt(
749752
const resolvedWorkspace = resolveUserPath(params.workspaceDir);
750753
const prevCwd = process.cwd();
751754
const runAbortController = new AbortController();
755+
// Proxy bootstrap must happen before timeout tuning so the timeouts wrap the
756+
// active EnvHttpProxyAgent instead of being replaced by a bare proxy dispatcher.
757+
ensureGlobalUndiciEnvProxyDispatcher();
752758
ensureGlobalUndiciStreamTimeouts();
753759

754760
log.debug(

‎src/infra/net/proxy-env.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import { describe, expect, it } from "vitest";
2+
import { hasEnvHttpProxyConfigured, resolveEnvHttpProxyUrl } from "./proxy-env.js";
3+
4+
describe("resolveEnvHttpProxyUrl", () => {
5+
it("uses lower-case https_proxy before upper-case HTTPS_PROXY", () => {
6+
const env = {
7+
https_proxy: "http://lower.test:8080",
8+
HTTPS_PROXY: "http://upper.test:8080",
9+
} as NodeJS.ProcessEnv;
10+
11+
expect(resolveEnvHttpProxyUrl("https", env)).toBe("http://lower.test:8080");
12+
});
13+
14+
it("treats empty lower-case https_proxy as authoritative over upper-case HTTPS_PROXY", () => {
15+
const env = {
16+
https_proxy: "",
17+
HTTPS_PROXY: "http://upper.test:8080",
18+
} as NodeJS.ProcessEnv;
19+
20+
expect(resolveEnvHttpProxyUrl("https", env)).toBeUndefined();
21+
expect(hasEnvHttpProxyConfigured("https", env)).toBe(false);
22+
});
23+
24+
it("treats empty lower-case http_proxy as authoritative over upper-case HTTP_PROXY", () => {
25+
const env = {
26+
http_proxy: " ",
27+
HTTP_PROXY: "http://upper-http.test:8080",
28+
} as NodeJS.ProcessEnv;
29+
30+
expect(resolveEnvHttpProxyUrl("http", env)).toBeUndefined();
31+
expect(hasEnvHttpProxyConfigured("http", env)).toBe(false);
32+
});
33+
34+
it("falls back from HTTPS proxy vars to HTTP proxy vars for https requests", () => {
35+
const env = {
36+
HTTP_PROXY: "http://upper-http.test:8080",
37+
} as NodeJS.ProcessEnv;
38+
39+
expect(resolveEnvHttpProxyUrl("https", env)).toBe("http://upper-http.test:8080");
40+
expect(hasEnvHttpProxyConfigured("https", env)).toBe(true);
41+
});
42+
});

‎src/infra/net/proxy-env.ts‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,40 @@ export function hasProxyEnvConfigured(env: NodeJS.ProcessEnv = process.env): boo
1616
}
1717
return false;
1818
}
19+
20+
function normalizeProxyEnvValue(value: string | undefined): string | null | undefined {
21+
if (typeof value !== "string") {
22+
return undefined;
23+
}
24+
const trimmed = value.trim();
25+
return trimmed.length > 0 ? trimmed : null;
26+
}
27+
28+
/**
29+
* Match undici EnvHttpProxyAgent semantics for env-based HTTP/S proxy selection:
30+
* - lower-case vars take precedence over upper-case
31+
* - HTTPS requests prefer https_proxy/HTTPS_PROXY, then fall back to http_proxy/HTTP_PROXY
32+
* - ALL_PROXY is ignored by EnvHttpProxyAgent
33+
*/
34+
export function resolveEnvHttpProxyUrl(
35+
protocol: "http" | "https",
36+
env: NodeJS.ProcessEnv = process.env,
37+
): string | undefined {
38+
const lowerHttpProxy = normalizeProxyEnvValue(env.http_proxy);
39+
const lowerHttpsProxy = normalizeProxyEnvValue(env.https_proxy);
40+
const httpProxy =
41+
lowerHttpProxy !== undefined ? lowerHttpProxy : normalizeProxyEnvValue(env.HTTP_PROXY);
42+
const httpsProxy =
43+
lowerHttpsProxy !== undefined ? lowerHttpsProxy : normalizeProxyEnvValue(env.HTTPS_PROXY);
44+
if (protocol === "https") {
45+
return httpsProxy ?? httpProxy ?? undefined;
46+
}
47+
return httpProxy ?? undefined;
48+
}
49+
50+
export function hasEnvHttpProxyConfigured(
51+
protocol: "http" | "https" = "https",
52+
env: NodeJS.ProcessEnv = process.env,
53+
): boolean {
54+
return resolveEnvHttpProxyUrl(protocol, env) !== undefined;
55+
}

‎src/infra/net/proxy-fetch.test.ts‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,7 @@ describe("resolveProxyFetchFromEnv", () => {
7373
});
7474

7575
it("returns proxy fetch using EnvHttpProxyAgent when HTTPS_PROXY is set", async () => {
76-
// Stub empty vars first — on Windows, process.env is case-insensitive so
77-
// HTTPS_PROXY and https_proxy share the same slot. Value must be set LAST.
7876
vi.stubEnv("HTTP_PROXY", "");
79-
vi.stubEnv("https_proxy", "");
80-
vi.stubEnv("http_proxy", "");
8177
vi.stubEnv("HTTPS_PROXY", "http://proxy.test:8080");
8278
undiciFetch.mockResolvedValue({ ok: true });
8379

@@ -94,8 +90,6 @@ describe("resolveProxyFetchFromEnv", () => {
9490

9591
it("returns proxy fetch when HTTP_PROXY is set", () => {
9692
vi.stubEnv("HTTPS_PROXY", "");
97-
vi.stubEnv("https_proxy", "");
98-
vi.stubEnv("http_proxy", "");
9993
vi.stubEnv("HTTP_PROXY", "http://fallback.test:3128");
10094

10195
const fetchFn = resolveProxyFetchFromEnv();

‎src/infra/net/proxy-fetch.ts‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici";
22
import { logWarn } from "../../logger.js";
3+
import { hasEnvHttpProxyConfigured } from "./proxy-env.js";
34

45
export const PROXY_FETCH_PROXY_URL = Symbol.for("openclaw.proxyFetch.proxyUrl");
56
type ProxyFetchWithMetadata = typeof fetch & {
@@ -51,12 +52,7 @@ export function getProxyUrlFromFetch(fetchImpl?: typeof fetch): string | undefin
5152
* Gracefully returns undefined if the proxy URL is malformed.
5253
*/
5354
export function resolveProxyFetchFromEnv(): typeof fetch | undefined {
54-
const proxyUrl =
55-
process.env.HTTPS_PROXY ||
56-
process.env.HTTP_PROXY ||
57-
process.env.https_proxy ||
58-
process.env.http_proxy;
59-
if (!proxyUrl?.trim()) {
55+
if (!hasEnvHttpProxyConfigured("https")) {
6056
return undefined;
6157
}
6258
try {

‎src/infra/net/undici-global-dispatcher.test.ts‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,8 +57,14 @@ vi.mock("node:net", () => ({
5757
getDefaultAutoSelectFamily,
5858
}));
5959

60+
vi.mock("./proxy-env.js", () => ({
61+
hasEnvHttpProxyConfigured: vi.fn(() => false),
62+
}));
63+
64+
import { hasEnvHttpProxyConfigured } from "./proxy-env.js";
6065
import {
6166
DEFAULT_UNDICI_STREAM_TIMEOUT_MS,
67+
ensureGlobalUndiciEnvProxyDispatcher,
6268
ensureGlobalUndiciStreamTimeouts,
6369
resetGlobalUndiciStreamTimeoutsForTests,
6470
} from "./undici-global-dispatcher.js";
@@ -69,6 +75,7 @@ describe("ensureGlobalUndiciStreamTimeouts", () => {
6975
resetGlobalUndiciStreamTimeoutsForTests();
7076
setCurrentDispatcher(new Agent());
7177
getDefaultAutoSelectFamily.mockReturnValue(undefined);
78+
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(false);
7279
});
7380

7481
it("replaces default Agent dispatcher with extended stream timeouts", () => {
@@ -136,3 +143,66 @@ describe("ensureGlobalUndiciStreamTimeouts", () => {
136143
});
137144
});
138145
});
146+
147+
describe("ensureGlobalUndiciEnvProxyDispatcher", () => {
148+
beforeEach(() => {
149+
vi.clearAllMocks();
150+
resetGlobalUndiciStreamTimeoutsForTests();
151+
setCurrentDispatcher(new Agent());
152+
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(false);
153+
});
154+
155+
it("installs EnvHttpProxyAgent when env HTTP proxy is configured on a default Agent", () => {
156+
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
157+
158+
ensureGlobalUndiciEnvProxyDispatcher();
159+
160+
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
161+
expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent);
162+
});
163+
164+
it("does not override unsupported custom proxy dispatcher types", () => {
165+
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
166+
setCurrentDispatcher(new ProxyAgent("http://proxy.test:8080"));
167+
168+
ensureGlobalUndiciEnvProxyDispatcher();
169+
170+
expect(setGlobalDispatcher).not.toHaveBeenCalled();
171+
});
172+
173+
it("retries proxy bootstrap after an unsupported dispatcher later becomes a default Agent", () => {
174+
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
175+
setCurrentDispatcher(new ProxyAgent("http://proxy.test:8080"));
176+
177+
ensureGlobalUndiciEnvProxyDispatcher();
178+
expect(setGlobalDispatcher).not.toHaveBeenCalled();
179+
180+
setCurrentDispatcher(new Agent());
181+
ensureGlobalUndiciEnvProxyDispatcher();
182+
183+
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
184+
expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent);
185+
});
186+
187+
it("is idempotent after proxy bootstrap succeeds", () => {
188+
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
189+
190+
ensureGlobalUndiciEnvProxyDispatcher();
191+
ensureGlobalUndiciEnvProxyDispatcher();
192+
193+
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
194+
});
195+
196+
it("reinstalls env proxy if an external change later reverts the dispatcher to Agent", () => {
197+
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
198+
199+
ensureGlobalUndiciEnvProxyDispatcher();
200+
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
201+
202+
setCurrentDispatcher(new Agent());
203+
ensureGlobalUndiciEnvProxyDispatcher();
204+
205+
expect(setGlobalDispatcher).toHaveBeenCalledTimes(2);
206+
expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent);
207+
});
208+
});

‎src/infra/net/undici-global-dispatcher.ts‎

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
import * as net from "node:net";
22
import { Agent, EnvHttpProxyAgent, getGlobalDispatcher, setGlobalDispatcher } from "undici";
3+
import { hasEnvHttpProxyConfigured } from "./proxy-env.js";
34

45
export const DEFAULT_UNDICI_STREAM_TIMEOUT_MS = 30 * 60 * 1000;
56

67
const AUTO_SELECT_FAMILY_ATTEMPT_TIMEOUT_MS = 300;
78

8-
let lastAppliedDispatcherKey: string | null = null;
9+
let lastAppliedTimeoutKey: string | null = null;
10+
let lastAppliedProxyBootstrap = false;
911

1012
type DispatcherKind = "agent" | "env-proxy" | "unsupported";
1113

@@ -59,28 +61,59 @@ function resolveDispatcherKey(params: {
5961
return `${params.kind}:${params.timeoutMs}:${autoSelectToken}`;
6062
}
6163

62-
export function ensureGlobalUndiciStreamTimeouts(opts?: { timeoutMs?: number }): void {
63-
const timeoutMsRaw = opts?.timeoutMs ?? DEFAULT_UNDICI_STREAM_TIMEOUT_MS;
64-
const timeoutMs = Math.max(1, Math.floor(timeoutMsRaw));
65-
if (!Number.isFinite(timeoutMsRaw)) {
66-
return;
67-
}
68-
64+
function resolveCurrentDispatcherKind(): DispatcherKind | null {
6965
let dispatcher: unknown;
7066
try {
7167
dispatcher = getGlobalDispatcher();
7268
} catch {
69+
return null;
70+
}
71+
72+
const currentKind = resolveDispatcherKind(dispatcher);
73+
return currentKind === "unsupported" ? null : currentKind;
74+
}
75+
76+
export function ensureGlobalUndiciEnvProxyDispatcher(): void {
77+
const shouldUseEnvProxy = hasEnvHttpProxyConfigured("https");
78+
if (!shouldUseEnvProxy) {
7379
return;
7480
}
81+
if (lastAppliedProxyBootstrap) {
82+
if (resolveCurrentDispatcherKind() === "env-proxy") {
83+
return;
84+
}
85+
lastAppliedProxyBootstrap = false;
86+
}
87+
const currentKind = resolveCurrentDispatcherKind();
88+
if (currentKind === null) {
89+
return;
90+
}
91+
if (currentKind === "env-proxy") {
92+
lastAppliedProxyBootstrap = true;
93+
return;
94+
}
95+
try {
96+
setGlobalDispatcher(new EnvHttpProxyAgent());
97+
lastAppliedProxyBootstrap = true;
98+
} catch {
99+
// Best-effort bootstrap only.
100+
}
101+
}
75102

76-
const kind = resolveDispatcherKind(dispatcher);
77-
if (kind === "unsupported") {
103+
export function ensureGlobalUndiciStreamTimeouts(opts?: { timeoutMs?: number }): void {
104+
const timeoutMsRaw = opts?.timeoutMs ?? DEFAULT_UNDICI_STREAM_TIMEOUT_MS;
105+
const timeoutMs = Math.max(1, Math.floor(timeoutMsRaw));
106+
if (!Number.isFinite(timeoutMsRaw)) {
107+
return;
108+
}
109+
const kind = resolveCurrentDispatcherKind();
110+
if (kind === null) {
78111
return;
79112
}
80113

81114
const autoSelectFamily = resolveAutoSelectFamily();
82115
const nextKey = resolveDispatcherKey({ kind, timeoutMs, autoSelectFamily });
83-
if (lastAppliedDispatcherKey === nextKey) {
116+
if (lastAppliedTimeoutKey === nextKey) {
84117
return;
85118
}
86119

@@ -102,12 +135,13 @@ export function ensureGlobalUndiciStreamTimeouts(opts?: { timeoutMs?: number }):
102135
}),
103136
);
104137
}
105-
lastAppliedDispatcherKey = nextKey;
138+
lastAppliedTimeoutKey = nextKey;
106139
} catch {
107140
// Best-effort hardening only.
108141
}
109142
}
110143

111144
export function resetGlobalUndiciStreamTimeoutsForTests(): void {
112-
lastAppliedDispatcherKey = null;
145+
lastAppliedTimeoutKey = null;
146+
lastAppliedProxyBootstrap = false;
113147
}

‎src/telegram/fetch.ts‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import * as dns from "node:dns";
22
import { Agent, EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici";
33
import type { TelegramNetworkConfig } from "../config/types.telegram.js";
44
import { resolveFetch } from "../infra/fetch.js";
5+
import { hasEnvHttpProxyConfigured } from "../infra/net/proxy-env.js";
56
import { createSubsystemLogger } from "../logging/subsystem.js";
67
import {
78
resolveTelegramAutoSelectFamilyDecision,
@@ -177,13 +178,7 @@ function shouldBypassEnvProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env
177178
}
178179

179180
function hasEnvHttpProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env): boolean {
180-
// Match EnvHttpProxyAgent behavior (undici) for HTTPS requests:
181-
// - lower-case env vars take precedence over upper-case
182-
// - HTTPS requests use https_proxy/HTTPS_PROXY first, then fall back to http_proxy/HTTP_PROXY
183-
// - ALL_PROXY is ignored by EnvHttpProxyAgent
184-
const httpProxy = env.http_proxy ?? env.HTTP_PROXY;
185-
const httpsProxy = env.https_proxy ?? env.HTTPS_PROXY;
186-
return Boolean(httpProxy) || Boolean(httpsProxy);
181+
return hasEnvHttpProxyConfigured("https", env);
187182
}
188183

189184
function createTelegramDispatcher(params: {

0 commit comments

Comments
 (0)