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
This commit is contained in:
Tak Hoffman
2026-03-11 10:21:35 -05:00
committed by GitHub
parent 1435fce2de
commit 87876a3e36
9 changed files with 209 additions and 34 deletions

View File

@@ -79,6 +79,7 @@ vi.mock("../../../infra/machine-name.js", () => ({
}));
vi.mock("../../../infra/net/undici-global-dispatcher.js", () => ({
ensureGlobalUndiciEnvProxyDispatcher: () => {},
ensureGlobalUndiciStreamTimeouts: () => {},
}));

View File

@@ -11,7 +11,10 @@ import { resolveHeartbeatPrompt } from "../../../auto-reply/heartbeat.js";
import { resolveChannelCapabilities } from "../../../config/channel-capabilities.js";
import type { OpenClawConfig } from "../../../config/config.js";
import { getMachineDisplayName } from "../../../infra/machine-name.js";
import { ensureGlobalUndiciStreamTimeouts } from "../../../infra/net/undici-global-dispatcher.js";
import {
ensureGlobalUndiciEnvProxyDispatcher,
ensureGlobalUndiciStreamTimeouts,
} from "../../../infra/net/undici-global-dispatcher.js";
import { MAX_IMAGE_BYTES } from "../../../media/constants.js";
import { getGlobalHookRunner } from "../../../plugins/hook-runner-global.js";
import type {
@@ -749,6 +752,9 @@ export async function runEmbeddedAttempt(
const resolvedWorkspace = resolveUserPath(params.workspaceDir);
const prevCwd = process.cwd();
const runAbortController = new AbortController();
// Proxy bootstrap must happen before timeout tuning so the timeouts wrap the
// active EnvHttpProxyAgent instead of being replaced by a bare proxy dispatcher.
ensureGlobalUndiciEnvProxyDispatcher();
ensureGlobalUndiciStreamTimeouts();
log.debug(

View File

@@ -0,0 +1,42 @@
import { describe, expect, it } from "vitest";
import { hasEnvHttpProxyConfigured, resolveEnvHttpProxyUrl } from "./proxy-env.js";
describe("resolveEnvHttpProxyUrl", () => {
it("uses lower-case https_proxy before upper-case HTTPS_PROXY", () => {
const env = {
https_proxy: "http://lower.test:8080",
HTTPS_PROXY: "http://upper.test:8080",
} as NodeJS.ProcessEnv;
expect(resolveEnvHttpProxyUrl("https", env)).toBe("http://lower.test:8080");
});
it("treats empty lower-case https_proxy as authoritative over upper-case HTTPS_PROXY", () => {
const env = {
https_proxy: "",
HTTPS_PROXY: "http://upper.test:8080",
} as NodeJS.ProcessEnv;
expect(resolveEnvHttpProxyUrl("https", env)).toBeUndefined();
expect(hasEnvHttpProxyConfigured("https", env)).toBe(false);
});
it("treats empty lower-case http_proxy as authoritative over upper-case HTTP_PROXY", () => {
const env = {
http_proxy: " ",
HTTP_PROXY: "http://upper-http.test:8080",
} as NodeJS.ProcessEnv;
expect(resolveEnvHttpProxyUrl("http", env)).toBeUndefined();
expect(hasEnvHttpProxyConfigured("http", env)).toBe(false);
});
it("falls back from HTTPS proxy vars to HTTP proxy vars for https requests", () => {
const env = {
HTTP_PROXY: "http://upper-http.test:8080",
} as NodeJS.ProcessEnv;
expect(resolveEnvHttpProxyUrl("https", env)).toBe("http://upper-http.test:8080");
expect(hasEnvHttpProxyConfigured("https", env)).toBe(true);
});
});

View File

@@ -16,3 +16,40 @@ export function hasProxyEnvConfigured(env: NodeJS.ProcessEnv = process.env): boo
}
return false;
}
function normalizeProxyEnvValue(value: string | undefined): string | null | undefined {
if (typeof value !== "string") {
return undefined;
}
const trimmed = value.trim();
return trimmed.length > 0 ? trimmed : null;
}
/**
* Match undici EnvHttpProxyAgent semantics for env-based HTTP/S proxy selection:
* - lower-case vars take precedence over upper-case
* - HTTPS requests prefer https_proxy/HTTPS_PROXY, then fall back to http_proxy/HTTP_PROXY
* - ALL_PROXY is ignored by EnvHttpProxyAgent
*/
export function resolveEnvHttpProxyUrl(
protocol: "http" | "https",
env: NodeJS.ProcessEnv = process.env,
): string | undefined {
const lowerHttpProxy = normalizeProxyEnvValue(env.http_proxy);
const lowerHttpsProxy = normalizeProxyEnvValue(env.https_proxy);
const httpProxy =
lowerHttpProxy !== undefined ? lowerHttpProxy : normalizeProxyEnvValue(env.HTTP_PROXY);
const httpsProxy =
lowerHttpsProxy !== undefined ? lowerHttpsProxy : normalizeProxyEnvValue(env.HTTPS_PROXY);
if (protocol === "https") {
return httpsProxy ?? httpProxy ?? undefined;
}
return httpProxy ?? undefined;
}
export function hasEnvHttpProxyConfigured(
protocol: "http" | "https" = "https",
env: NodeJS.ProcessEnv = process.env,
): boolean {
return resolveEnvHttpProxyUrl(protocol, env) !== undefined;
}

View File

@@ -73,11 +73,7 @@ describe("resolveProxyFetchFromEnv", () => {
});
it("returns proxy fetch using EnvHttpProxyAgent when HTTPS_PROXY is set", async () => {
// Stub empty vars first — on Windows, process.env is case-insensitive so
// HTTPS_PROXY and https_proxy share the same slot. Value must be set LAST.
vi.stubEnv("HTTP_PROXY", "");
vi.stubEnv("https_proxy", "");
vi.stubEnv("http_proxy", "");
vi.stubEnv("HTTPS_PROXY", "http://proxy.test:8080");
undiciFetch.mockResolvedValue({ ok: true });
@@ -94,8 +90,6 @@ describe("resolveProxyFetchFromEnv", () => {
it("returns proxy fetch when HTTP_PROXY is set", () => {
vi.stubEnv("HTTPS_PROXY", "");
vi.stubEnv("https_proxy", "");
vi.stubEnv("http_proxy", "");
vi.stubEnv("HTTP_PROXY", "http://fallback.test:3128");
const fetchFn = resolveProxyFetchFromEnv();

View File

@@ -1,5 +1,6 @@
import { EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici";
import { logWarn } from "../../logger.js";
import { hasEnvHttpProxyConfigured } from "./proxy-env.js";
export const PROXY_FETCH_PROXY_URL = Symbol.for("openclaw.proxyFetch.proxyUrl");
type ProxyFetchWithMetadata = typeof fetch & {
@@ -51,12 +52,7 @@ export function getProxyUrlFromFetch(fetchImpl?: typeof fetch): string | undefin
* Gracefully returns undefined if the proxy URL is malformed.
*/
export function resolveProxyFetchFromEnv(): typeof fetch | undefined {
const proxyUrl =
process.env.HTTPS_PROXY ||
process.env.HTTP_PROXY ||
process.env.https_proxy ||
process.env.http_proxy;
if (!proxyUrl?.trim()) {
if (!hasEnvHttpProxyConfigured("https")) {
return undefined;
}
try {

View File

@@ -57,8 +57,14 @@ vi.mock("node:net", () => ({
getDefaultAutoSelectFamily,
}));
vi.mock("./proxy-env.js", () => ({
hasEnvHttpProxyConfigured: vi.fn(() => false),
}));
import { hasEnvHttpProxyConfigured } from "./proxy-env.js";
import {
DEFAULT_UNDICI_STREAM_TIMEOUT_MS,
ensureGlobalUndiciEnvProxyDispatcher,
ensureGlobalUndiciStreamTimeouts,
resetGlobalUndiciStreamTimeoutsForTests,
} from "./undici-global-dispatcher.js";
@@ -69,6 +75,7 @@ describe("ensureGlobalUndiciStreamTimeouts", () => {
resetGlobalUndiciStreamTimeoutsForTests();
setCurrentDispatcher(new Agent());
getDefaultAutoSelectFamily.mockReturnValue(undefined);
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(false);
});
it("replaces default Agent dispatcher with extended stream timeouts", () => {
@@ -136,3 +143,66 @@ describe("ensureGlobalUndiciStreamTimeouts", () => {
});
});
});
describe("ensureGlobalUndiciEnvProxyDispatcher", () => {
beforeEach(() => {
vi.clearAllMocks();
resetGlobalUndiciStreamTimeoutsForTests();
setCurrentDispatcher(new Agent());
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(false);
});
it("installs EnvHttpProxyAgent when env HTTP proxy is configured on a default Agent", () => {
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
ensureGlobalUndiciEnvProxyDispatcher();
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent);
});
it("does not override unsupported custom proxy dispatcher types", () => {
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
setCurrentDispatcher(new ProxyAgent("http://proxy.test:8080"));
ensureGlobalUndiciEnvProxyDispatcher();
expect(setGlobalDispatcher).not.toHaveBeenCalled();
});
it("retries proxy bootstrap after an unsupported dispatcher later becomes a default Agent", () => {
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
setCurrentDispatcher(new ProxyAgent("http://proxy.test:8080"));
ensureGlobalUndiciEnvProxyDispatcher();
expect(setGlobalDispatcher).not.toHaveBeenCalled();
setCurrentDispatcher(new Agent());
ensureGlobalUndiciEnvProxyDispatcher();
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent);
});
it("is idempotent after proxy bootstrap succeeds", () => {
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
ensureGlobalUndiciEnvProxyDispatcher();
ensureGlobalUndiciEnvProxyDispatcher();
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
});
it("reinstalls env proxy if an external change later reverts the dispatcher to Agent", () => {
vi.mocked(hasEnvHttpProxyConfigured).mockReturnValue(true);
ensureGlobalUndiciEnvProxyDispatcher();
expect(setGlobalDispatcher).toHaveBeenCalledTimes(1);
setCurrentDispatcher(new Agent());
ensureGlobalUndiciEnvProxyDispatcher();
expect(setGlobalDispatcher).toHaveBeenCalledTimes(2);
expect(getCurrentDispatcher()).toBeInstanceOf(EnvHttpProxyAgent);
});
});

View File

@@ -1,11 +1,13 @@
import * as net from "node:net";
import { Agent, EnvHttpProxyAgent, getGlobalDispatcher, setGlobalDispatcher } from "undici";
import { hasEnvHttpProxyConfigured } from "./proxy-env.js";
export const DEFAULT_UNDICI_STREAM_TIMEOUT_MS = 30 * 60 * 1000;
const AUTO_SELECT_FAMILY_ATTEMPT_TIMEOUT_MS = 300;
let lastAppliedDispatcherKey: string | null = null;
let lastAppliedTimeoutKey: string | null = null;
let lastAppliedProxyBootstrap = false;
type DispatcherKind = "agent" | "env-proxy" | "unsupported";
@@ -59,28 +61,59 @@ function resolveDispatcherKey(params: {
return `${params.kind}:${params.timeoutMs}:${autoSelectToken}`;
}
function resolveCurrentDispatcherKind(): DispatcherKind | null {
let dispatcher: unknown;
try {
dispatcher = getGlobalDispatcher();
} catch {
return null;
}
const currentKind = resolveDispatcherKind(dispatcher);
return currentKind === "unsupported" ? null : currentKind;
}
export function ensureGlobalUndiciEnvProxyDispatcher(): void {
const shouldUseEnvProxy = hasEnvHttpProxyConfigured("https");
if (!shouldUseEnvProxy) {
return;
}
if (lastAppliedProxyBootstrap) {
if (resolveCurrentDispatcherKind() === "env-proxy") {
return;
}
lastAppliedProxyBootstrap = false;
}
const currentKind = resolveCurrentDispatcherKind();
if (currentKind === null) {
return;
}
if (currentKind === "env-proxy") {
lastAppliedProxyBootstrap = true;
return;
}
try {
setGlobalDispatcher(new EnvHttpProxyAgent());
lastAppliedProxyBootstrap = true;
} catch {
// Best-effort bootstrap only.
}
}
export function ensureGlobalUndiciStreamTimeouts(opts?: { timeoutMs?: number }): void {
const timeoutMsRaw = opts?.timeoutMs ?? DEFAULT_UNDICI_STREAM_TIMEOUT_MS;
const timeoutMs = Math.max(1, Math.floor(timeoutMsRaw));
if (!Number.isFinite(timeoutMsRaw)) {
return;
}
let dispatcher: unknown;
try {
dispatcher = getGlobalDispatcher();
} catch {
return;
}
const kind = resolveDispatcherKind(dispatcher);
if (kind === "unsupported") {
const kind = resolveCurrentDispatcherKind();
if (kind === null) {
return;
}
const autoSelectFamily = resolveAutoSelectFamily();
const nextKey = resolveDispatcherKey({ kind, timeoutMs, autoSelectFamily });
if (lastAppliedDispatcherKey === nextKey) {
if (lastAppliedTimeoutKey === nextKey) {
return;
}
@@ -102,12 +135,13 @@ export function ensureGlobalUndiciStreamTimeouts(opts?: { timeoutMs?: number }):
}),
);
}
lastAppliedDispatcherKey = nextKey;
lastAppliedTimeoutKey = nextKey;
} catch {
// Best-effort hardening only.
}
}
export function resetGlobalUndiciStreamTimeoutsForTests(): void {
lastAppliedDispatcherKey = null;
lastAppliedTimeoutKey = null;
lastAppliedProxyBootstrap = false;
}

View File

@@ -2,6 +2,7 @@ import * as dns from "node:dns";
import { Agent, EnvHttpProxyAgent, ProxyAgent, fetch as undiciFetch } from "undici";
import type { TelegramNetworkConfig } from "../config/types.telegram.js";
import { resolveFetch } from "../infra/fetch.js";
import { hasEnvHttpProxyConfigured } from "../infra/net/proxy-env.js";
import { createSubsystemLogger } from "../logging/subsystem.js";
import {
resolveTelegramAutoSelectFamilyDecision,
@@ -177,13 +178,7 @@ function shouldBypassEnvProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env
}
function hasEnvHttpProxyForTelegramApi(env: NodeJS.ProcessEnv = process.env): boolean {
// Match EnvHttpProxyAgent behavior (undici) for HTTPS requests:
// - lower-case env vars take precedence over upper-case
// - HTTPS requests use https_proxy/HTTPS_PROXY first, then fall back to http_proxy/HTTP_PROXY
// - ALL_PROXY is ignored by EnvHttpProxyAgent
const httpProxy = env.http_proxy ?? env.HTTP_PROXY;
const httpsProxy = env.https_proxy ?? env.HTTPS_PROXY;
return Boolean(httpProxy) || Boolean(httpsProxy);
return hasEnvHttpProxyConfigured("https", env);
}
function createTelegramDispatcher(params: {