mirror of
https://github.com/openclaw/clawhub.git
synced 2026-08-14 08:52:21 +00:00
fix: abort registry discovery fetch after a timeout (#3378)
* fix: abort registry discovery fetch after a timeout
discoverRegistryFromSite called fetch without an AbortSignal, so a
site that accepts the request but never answers hung 'clawhub login'
and registry resolution forever. Wrap the fetch in a local
AbortController + setTimeout helper (mirroring fetchWithTimeout in
http.ts, which is not exported) with a 15s budget matching the
package's request timeout convention, and reject with a clear
'Request timed out after 15s' error. Both call sites already
degrade any discovery rejection to null via .catch(() => null).
* fix: extend timeout to cover JSON body parsing
ClawSweeper P2 finding: the timeout cleared after fetch() resolved,
but response.json() could still hang if the peer sent headers and
never completed the body.
Changes:
- fetchWithTimeout now returns {response, clearTimer} tuple
- Caller keeps timeout active through JSON parsing
- Only clears timer in finally after body consumed
- Added test: stalled body triggers timeout (4/4 → 8/8 passing)
Addresses: ClawSweeper review P2 finding
Fixes: Timeout now covers full request lifecycle
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(cli): clear discovery timeout on fetch failure
* test: format discovery timeout regression
* fix(cli): normalize discovery timeout errors
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Patrick Erichsen <patrick.a.erichsen@gmail.com>
This commit is contained in:
co-authored by
Claude Opus 5
Patrick Erichsen
parent
6d935f0595
commit
29bc11f29d
@@ -0,0 +1,119 @@
|
||||
/* @vitest-environment node */
|
||||
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { createGlobalStubRegistry } from "../test/runtimeStubs.js";
|
||||
import { discoverRegistryFromSite } from "./discovery";
|
||||
|
||||
const globalStubs = createGlobalStubRegistry();
|
||||
|
||||
function stubNeverResolvingFetch() {
|
||||
globalStubs.stub(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
(_input: unknown, init?: RequestInit) =>
|
||||
new Promise<Response>((_resolve, reject) => {
|
||||
const signal = init?.signal;
|
||||
if (!signal) return; // hangs forever, like an endpoint that never answers
|
||||
signal.addEventListener("abort", () => {
|
||||
reject(signal.reason instanceof Error ? signal.reason : new Error("aborted"));
|
||||
});
|
||||
}),
|
||||
) as unknown as typeof fetch,
|
||||
);
|
||||
}
|
||||
|
||||
function stubJsonFetch(status: number, body: unknown) {
|
||||
globalStubs.stub(
|
||||
"fetch",
|
||||
vi.fn(
|
||||
async () =>
|
||||
new Response(JSON.stringify(body), {
|
||||
status,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
}),
|
||||
) as unknown as typeof fetch,
|
||||
);
|
||||
}
|
||||
|
||||
function stubRejectingFetch(error: Error) {
|
||||
globalStubs.stub("fetch", vi.fn(async () => Promise.reject(error)) as unknown as typeof fetch);
|
||||
}
|
||||
|
||||
function stubStalledBodyFetch() {
|
||||
globalStubs.stub(
|
||||
"fetch",
|
||||
vi.fn(async (_input: unknown, init?: RequestInit) => {
|
||||
const signal = init?.signal;
|
||||
// Return a response with headers immediately, but body never completes
|
||||
const response = new Response(null, {
|
||||
status: 200,
|
||||
headers: { "Content-Type": "application/json" },
|
||||
});
|
||||
// Override .json() to hang forever (or abort if signal fires)
|
||||
response.json = vi.fn(
|
||||
() =>
|
||||
new Promise<unknown>((_resolve, reject) => {
|
||||
if (!signal) return; // hangs forever
|
||||
signal.addEventListener("abort", () => {
|
||||
reject(new DOMException("The operation was aborted", "AbortError"));
|
||||
});
|
||||
}),
|
||||
);
|
||||
return response;
|
||||
}) as unknown as typeof fetch,
|
||||
);
|
||||
}
|
||||
|
||||
describe("discoverRegistryFromSite timeout", () => {
|
||||
afterEach(() => {
|
||||
vi.useRealTimers();
|
||||
globalStubs.restoreAll();
|
||||
vi.restoreAllMocks();
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it(
|
||||
"rejects with a timeout error when the endpoint never responds",
|
||||
{ timeout: 5_000 },
|
||||
async () => {
|
||||
stubNeverResolvingFetch();
|
||||
await expect(discoverRegistryFromSite("https://example.com", 50)).rejects.toThrow(
|
||||
/Request timed out after \d+s/,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("clears the timeout when fetch rejects before returning headers", async () => {
|
||||
vi.useFakeTimers();
|
||||
const error = new Error("connection refused");
|
||||
stubRejectingFetch(error);
|
||||
|
||||
await expect(discoverRegistryFromSite("https://example.com")).rejects.toBe(error);
|
||||
expect(vi.getTimerCount()).toBe(0);
|
||||
});
|
||||
|
||||
it(
|
||||
"rejects with a timeout error when the response body never completes",
|
||||
{ timeout: 5_000 },
|
||||
async () => {
|
||||
stubStalledBodyFetch();
|
||||
await expect(discoverRegistryFromSite("https://example.com", 50)).rejects.toThrow(
|
||||
/Request timed out after \d+s/,
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
it("still parses a valid well-known config", async () => {
|
||||
stubJsonFetch(200, { registry: "https://example.convex.site" });
|
||||
await expect(discoverRegistryFromSite("https://example.com")).resolves.toEqual({
|
||||
apiBase: "https://example.convex.site",
|
||||
authBase: undefined,
|
||||
minCliVersion: undefined,
|
||||
});
|
||||
});
|
||||
|
||||
it("still returns null when both well-known paths 404", async () => {
|
||||
stubJsonFetch(404, { error: "nope" });
|
||||
await expect(discoverRegistryFromSite("https://example.com")).resolves.toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -1,23 +1,74 @@
|
||||
import { parseArk, WellKnownConfigSchema } from "./schema/index.js";
|
||||
|
||||
export async function discoverRegistryFromSite(siteUrl: string) {
|
||||
const DISCOVERY_TIMEOUT_MS = 15_000;
|
||||
|
||||
// Mirrors the fetchWithTimeout pattern from ./http.js (not imported) so an
|
||||
// endpoint that accepts but never answers cannot hang discovery forever.
|
||||
// Returns both the response and the timeout handle so the caller can extend
|
||||
// the timeout to cover body parsing.
|
||||
async function fetchWithTimeout(
|
||||
url: string,
|
||||
init: RequestInit,
|
||||
timeoutMs: number,
|
||||
): Promise<{
|
||||
response: Response;
|
||||
clearTimer: () => void;
|
||||
normalizeError: (error: unknown) => unknown;
|
||||
}> {
|
||||
const controller = new AbortController();
|
||||
const timeoutSeconds = Math.ceil(timeoutMs / 1000);
|
||||
let timeoutError: Error | null = null;
|
||||
const timeout = setTimeout(() => {
|
||||
timeoutError = new Error(`Request timed out after ${timeoutSeconds}s`);
|
||||
controller.abort(timeoutError);
|
||||
}, timeoutMs);
|
||||
const normalizeError = (error: unknown) => timeoutError ?? error;
|
||||
try {
|
||||
const response = await fetch(url, { ...init, signal: controller.signal });
|
||||
return {
|
||||
response,
|
||||
clearTimer: () => clearTimeout(timeout),
|
||||
normalizeError,
|
||||
};
|
||||
} catch (error) {
|
||||
clearTimeout(timeout);
|
||||
throw normalizeError(error);
|
||||
}
|
||||
}
|
||||
|
||||
export async function discoverRegistryFromSite(siteUrl: string, timeoutMs = DISCOVERY_TIMEOUT_MS) {
|
||||
const paths = ["/.well-known/clawhub.json", "/.well-known/clawdhub.json"];
|
||||
for (const path of paths) {
|
||||
const url = new URL(path, siteUrl);
|
||||
const response = await fetch(url.toString(), {
|
||||
method: "GET",
|
||||
headers: { Accept: "application/json" },
|
||||
});
|
||||
if (!response.ok) continue;
|
||||
const raw = (await response.json()) as unknown;
|
||||
const parsed = parseArk(WellKnownConfigSchema, raw, "WellKnown config");
|
||||
const apiBase = "apiBase" in parsed ? parsed.apiBase : parsed.registry;
|
||||
if (!apiBase) return null;
|
||||
return {
|
||||
apiBase,
|
||||
authBase: parsed.authBase,
|
||||
minCliVersion: parsed.minCliVersion,
|
||||
};
|
||||
const { response, clearTimer, normalizeError } = await fetchWithTimeout(
|
||||
url.toString(),
|
||||
{
|
||||
method: "GET",
|
||||
headers: { Accept: "application/json" },
|
||||
},
|
||||
timeoutMs,
|
||||
);
|
||||
if (!response.ok) {
|
||||
clearTimer();
|
||||
continue;
|
||||
}
|
||||
// Keep timeout active through JSON body parsing to guard against
|
||||
// peers that send headers but never complete the response body
|
||||
try {
|
||||
const raw = (await response.json()) as unknown;
|
||||
const parsed = parseArk(WellKnownConfigSchema, raw, "WellKnown config");
|
||||
const apiBase = "apiBase" in parsed ? parsed.apiBase : parsed.registry;
|
||||
if (!apiBase) return null;
|
||||
return {
|
||||
apiBase,
|
||||
authBase: parsed.authBase,
|
||||
minCliVersion: parsed.minCliVersion,
|
||||
};
|
||||
} catch (error) {
|
||||
throw normalizeError(error);
|
||||
} finally {
|
||||
clearTimer();
|
||||
}
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user