Merge PR #8785: fix(update): surface actionable message for unsupported proxy schemes (@roboomp)
This commit is contained in:
@@ -81,6 +81,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed OpenCode MCP servers 401ing when config used OpenCode's `{env:VAR}`/`{file:path}` substitution (e.g. `Bearer {env:MCP_KEY}` headers); the OpenCode loader now expands those tokens the way OpenCode does instead of only `${VAR}` ([#8778](https://github.com/can1357/oh-my-pi/issues/8778)).
|
||||
### Fixed
|
||||
|
||||
- Fixed `omp update` leaking Bun's raw `fetch()` error ("pass `verbose: true` in the second argument to fetch()") when a proxy environment variable (`HTTPS_PROXY`, `ALL_PROXY`, …) uses an unsupported scheme such as SOCKS; the update check now reports an actionable message naming the offending variable and the http/https proxy requirement ([#8784](https://github.com/can1357/oh-my-pi/issues/8784)).
|
||||
|
||||
## [17.3.5] - 2026-08-16
|
||||
|
||||
|
||||
@@ -15,7 +15,12 @@ import chalk from "@oh-my-pi/pi-utils/chalk";
|
||||
import { withFileLock } from "@oh-my-pi/pi-utils/file-lock";
|
||||
import { $ } from "bun";
|
||||
import { theme } from "../modes/theme/theme";
|
||||
import { isTimeoutError, withTimeoutSignal } from "../utils/fetch-timeout";
|
||||
import {
|
||||
isTimeoutError,
|
||||
isUnsupportedProxyError,
|
||||
unsupportedProxyMessage,
|
||||
withTimeoutSignal,
|
||||
} from "../utils/fetch-timeout";
|
||||
|
||||
const REPO = "can1357/oh-my-pi";
|
||||
const PACKAGE = "@oh-my-pi/pi-coding-agent";
|
||||
@@ -248,6 +253,7 @@ async function getReleaseBinaryAsset(
|
||||
if (isTimeoutError(err)) {
|
||||
throw new Error("Timed out fetching GitHub release metadata after 30s", { cause: err });
|
||||
}
|
||||
if (isUnsupportedProxyError(err)) throw new Error(unsupportedProxyMessage(), { cause: err });
|
||||
throw err;
|
||||
}
|
||||
if ((response.status === 403 && !githubToken) || response.status === 429) {
|
||||
@@ -287,6 +293,7 @@ export async function downloadVerifiedBinary(options: VerifiedBinaryDownloadOpti
|
||||
if (isTimeoutError(err)) {
|
||||
throw new Error("Timed out downloading release binary after 15 minutes", { cause: err });
|
||||
}
|
||||
if (isUnsupportedProxyError(err)) throw new Error(unsupportedProxyMessage(), { cause: err });
|
||||
throw err;
|
||||
}
|
||||
if (!response.ok || !response.body) {
|
||||
@@ -326,6 +333,7 @@ export async function downloadVerifiedBinary(options: VerifiedBinaryDownloadOpti
|
||||
if (isTimeoutError(err)) {
|
||||
throw new Error("Timed out downloading release binary after 15 minutes", { cause: err });
|
||||
}
|
||||
if (isUnsupportedProxyError(err)) throw new Error(unsupportedProxyMessage(), { cause: err });
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
@@ -715,6 +723,7 @@ async function fetchLatestManifest(
|
||||
cause: err,
|
||||
});
|
||||
}
|
||||
if (isUnsupportedProxyError(err)) throw new Error(unsupportedProxyMessage(), { cause: err });
|
||||
throw err;
|
||||
}
|
||||
if (!response.ok) {
|
||||
|
||||
@@ -8,3 +8,36 @@ export function withTimeoutSignal(timeoutMs: number, signal?: AbortSignal): Abor
|
||||
export function isTimeoutError(error: unknown): boolean {
|
||||
return error instanceof Error && error.name === "TimeoutError";
|
||||
}
|
||||
|
||||
/**
|
||||
* Proxy environment variables Bun's `fetch` consults, in the precedence order it
|
||||
* reads them. Used to name the offending entry in {@link unsupportedProxyMessage}.
|
||||
*/
|
||||
const PROXY_ENV_VARS = ["HTTPS_PROXY", "https_proxy", "ALL_PROXY", "all_proxy", "HTTP_PROXY", "http_proxy"] as const;
|
||||
|
||||
/**
|
||||
* Detect Bun's `UnsupportedProxyProtocol` fetch rejection, raised when a proxy
|
||||
* env var (`HTTPS_PROXY`, `ALL_PROXY`, …) points at a scheme it cannot drive —
|
||||
* most commonly a SOCKS proxy (`socks5://`, `socks5h://`). The raw error tells
|
||||
* the caller to "pass `verbose: true` in the second argument to fetch()", which
|
||||
* is meaningless from a CLI, so callers translate it into
|
||||
* {@link unsupportedProxyMessage}.
|
||||
*/
|
||||
export function isUnsupportedProxyError(error: unknown): boolean {
|
||||
return error instanceof Error && error.message.includes("UnsupportedProxyProtocol");
|
||||
}
|
||||
|
||||
/**
|
||||
* Build an actionable CLI message for an {@link isUnsupportedProxyError} failure,
|
||||
* naming any set proxy env var whose scheme is not `http(s)://` so the user can
|
||||
* see exactly which variable to change.
|
||||
*/
|
||||
export function unsupportedProxyMessage(env: Record<string, string | undefined> = process.env): string {
|
||||
const offending: string[] = [];
|
||||
for (const name of PROXY_ENV_VARS) {
|
||||
const value = env[name];
|
||||
if (value && !/^https?:\/\//i.test(value)) offending.push(`${name}=${value}`);
|
||||
}
|
||||
const detail = offending.length > 0 ? ` (offending: ${offending.join(", ")})` : "";
|
||||
return `Proxy configuration uses a scheme Bun's fetch cannot use${detail}. Only http:// and https:// proxies are supported — SOCKS proxies (socks5://, socks5h://) are not. Point HTTP_PROXY/HTTPS_PROXY at an http:// proxy URL or unset the proxy variables, then retry.`;
|
||||
}
|
||||
|
||||
@@ -89,3 +89,35 @@ describe("getLatestRelease rename pointers", () => {
|
||||
expect(release.packages).toEqual({ pkg: "@oh-my-pi/pi-coding-agent", natives: "@oh-my-pi/pi-natives" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("getLatestRelease proxy errors", () => {
|
||||
afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("translates Bun's UnsupportedProxyProtocol fetch failure into an actionable CLI message", async () => {
|
||||
const fetchStub = Object.assign(
|
||||
async () => {
|
||||
throw new Error(
|
||||
'UnsupportedProxyProtocol fetching "https://registry.npmjs.org/@oh-my-pi/pi-coding-agent/latest". ' +
|
||||
"For more information, pass `verbose: true` in the second argument to fetch()",
|
||||
);
|
||||
},
|
||||
{ preconnect: globalThis.fetch.preconnect },
|
||||
);
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(fetchStub);
|
||||
|
||||
const err = await getLatestRelease({ timeoutMs: 5000 }).then(
|
||||
() => null,
|
||||
(e: unknown) => e as Error,
|
||||
);
|
||||
|
||||
expect(err).toBeInstanceOf(Error);
|
||||
// The raw fetch() instruction the CLI user cannot act on must not leak through.
|
||||
expect(err?.message).not.toContain("verbose: true");
|
||||
expect(err?.message).not.toContain("fetch()");
|
||||
// Instead the user gets actionable guidance about supported proxy schemes.
|
||||
expect(err?.message).toMatch(/SOCKS/i);
|
||||
expect(err?.message).toMatch(/https?:\/\//i);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { isUnsupportedProxyError, unsupportedProxyMessage } from "../../src/utils/fetch-timeout";
|
||||
|
||||
describe("isUnsupportedProxyError", () => {
|
||||
it("matches Bun's UnsupportedProxyProtocol rejection and nothing else", () => {
|
||||
expect(isUnsupportedProxyError(new Error('UnsupportedProxyProtocol fetching "https://x"'))).toBe(true);
|
||||
expect(isUnsupportedProxyError(new Error("ConnectionRefused"))).toBe(false);
|
||||
expect(isUnsupportedProxyError("UnsupportedProxyProtocol")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("unsupportedProxyMessage", () => {
|
||||
it("names the proxy env var whose scheme Bun's fetch cannot use", () => {
|
||||
const message = unsupportedProxyMessage({
|
||||
HTTPS_PROXY: "socks5h://127.0.0.1:1080",
|
||||
NO_PROXY: "localhost",
|
||||
});
|
||||
expect(message).toContain("HTTPS_PROXY=socks5h://127.0.0.1:1080");
|
||||
expect(message).toMatch(/http:\/\//);
|
||||
});
|
||||
|
||||
it("does not flag http(s) proxy vars as offending", () => {
|
||||
const message = unsupportedProxyMessage({ HTTP_PROXY: "http://127.0.0.1:8080" });
|
||||
expect(message).not.toContain("offending");
|
||||
expect(message).toContain("Only http:// and https:// proxies are supported");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user