fix: catch network-level fetch errors in web_fetch to prevent gateway crash
Fixes #4392. When fetch() fails at the network level (TypeError: fetch failed), the error was not caught by the web_fetch tool, causing an unhandled promise rejection that crashed the gateway process via process.exit(1) in infra/unhandled-rejections.js. Changes: - Wrap execute handler in try-catch to return error results instead of throwing, preventing unhandled rejections from network failures - Add try-catch around fetchFirecrawlContent's fetch() call for clearer error messages on network failures - Fix floating promise from res.body?.cancel() that could cause unhandled rejections - Update tests to verify errors are returned as results, add test for network-level TypeError
This commit is contained in:
parent
ee50fb8528
commit
b062131ae9
@ -227,7 +227,7 @@ async function fetchWithRedirects(params: {
|
||||
throw new Error("Redirect loop detected");
|
||||
}
|
||||
visited.add(nextUrl);
|
||||
void res.body?.cancel();
|
||||
res.body?.cancel().catch(() => {});
|
||||
await closeDispatcher(dispatcher);
|
||||
currentUrl = nextUrl;
|
||||
continue;
|
||||
@ -282,15 +282,21 @@ export async function fetchFirecrawlContent(params: {
|
||||
storeInCache: params.storeInCache,
|
||||
};
|
||||
|
||||
const res = await fetch(endpoint, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
Authorization: `Bearer ${params.apiKey}`,
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
signal: withTimeout(undefined, params.timeoutSeconds * 1000),
|
||||
});
|
||||
let res: Response;
|
||||
try {
|
||||
res = await fetch(endpoint, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
Authorization: `Bearer ${params.apiKey}`,
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
signal: withTimeout(undefined, params.timeoutSeconds * 1000),
|
||||
});
|
||||
} catch (err) {
|
||||
const detail = err instanceof Error ? err.message : String(err);
|
||||
throw new Error(`Firecrawl fetch failed (network): ${detail}`);
|
||||
}
|
||||
|
||||
const payload = (await res.json()) as {
|
||||
success?: boolean;
|
||||
@ -600,25 +606,36 @@ export function createWebFetchTool(options?: {
|
||||
const url = readStringParam(params, "url", { required: true });
|
||||
const extractMode = readStringParam(params, "extractMode") === "text" ? "text" : "markdown";
|
||||
const maxChars = readNumberParam(params, "maxChars", { integer: true });
|
||||
const result = await runWebFetch({
|
||||
url,
|
||||
extractMode,
|
||||
maxChars: resolveMaxChars(maxChars ?? fetch?.maxChars, DEFAULT_FETCH_MAX_CHARS),
|
||||
maxRedirects: resolveMaxRedirects(fetch?.maxRedirects, DEFAULT_FETCH_MAX_REDIRECTS),
|
||||
timeoutSeconds: resolveTimeoutSeconds(fetch?.timeoutSeconds, DEFAULT_TIMEOUT_SECONDS),
|
||||
cacheTtlMs: resolveCacheTtlMs(fetch?.cacheTtlMinutes, DEFAULT_CACHE_TTL_MINUTES),
|
||||
userAgent,
|
||||
readabilityEnabled,
|
||||
firecrawlEnabled,
|
||||
firecrawlApiKey,
|
||||
firecrawlBaseUrl,
|
||||
firecrawlOnlyMainContent,
|
||||
firecrawlMaxAgeMs,
|
||||
firecrawlProxy: "auto",
|
||||
firecrawlStoreInCache: true,
|
||||
firecrawlTimeoutSeconds,
|
||||
});
|
||||
return jsonResult(result);
|
||||
try {
|
||||
const result = await runWebFetch({
|
||||
url,
|
||||
extractMode,
|
||||
maxChars: resolveMaxChars(maxChars ?? fetch?.maxChars, DEFAULT_FETCH_MAX_CHARS),
|
||||
maxRedirects: resolveMaxRedirects(fetch?.maxRedirects, DEFAULT_FETCH_MAX_REDIRECTS),
|
||||
timeoutSeconds: resolveTimeoutSeconds(fetch?.timeoutSeconds, DEFAULT_TIMEOUT_SECONDS),
|
||||
cacheTtlMs: resolveCacheTtlMs(fetch?.cacheTtlMinutes, DEFAULT_CACHE_TTL_MINUTES),
|
||||
userAgent,
|
||||
readabilityEnabled,
|
||||
firecrawlEnabled,
|
||||
firecrawlApiKey,
|
||||
firecrawlBaseUrl,
|
||||
firecrawlOnlyMainContent,
|
||||
firecrawlMaxAgeMs,
|
||||
firecrawlProxy: "auto",
|
||||
firecrawlStoreInCache: true,
|
||||
firecrawlTimeoutSeconds,
|
||||
});
|
||||
return jsonResult(result);
|
||||
} catch (err) {
|
||||
const message =
|
||||
err instanceof Error ? err.message : typeof err === "string" ? err : "Unknown error";
|
||||
return jsonResult({
|
||||
status: "error",
|
||||
tool: "web_fetch",
|
||||
url,
|
||||
error: `Web fetch failed: ${message}`,
|
||||
});
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@ -125,7 +125,7 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
expect(details.text).toContain("firecrawl content");
|
||||
});
|
||||
|
||||
it("throws when readability is disabled and firecrawl is unavailable", async () => {
|
||||
it("returns error result when readability is disabled and firecrawl is unavailable", async () => {
|
||||
const mockFetch = vi.fn((input: RequestInfo) =>
|
||||
Promise.resolve(htmlResponse("<html><body>hi</body></html>", requestUrl(input))),
|
||||
);
|
||||
@ -143,12 +143,15 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
sandboxed: false,
|
||||
});
|
||||
|
||||
await expect(
|
||||
tool?.execute?.("call", { url: "https://example.com/readability-off" }),
|
||||
).rejects.toThrow("Readability disabled");
|
||||
const result = await tool?.execute?.("call", {
|
||||
url: "https://example.com/readability-off",
|
||||
});
|
||||
const details = result?.details as { status?: string; error?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toContain("Readability disabled");
|
||||
});
|
||||
|
||||
it("throws when readability is empty and firecrawl fails", async () => {
|
||||
it("returns error result when readability is empty and firecrawl fails", async () => {
|
||||
const mockFetch = vi.fn((input: RequestInfo) => {
|
||||
const url = requestUrl(input);
|
||||
if (url.includes("api.firecrawl.dev")) {
|
||||
@ -172,9 +175,12 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
sandboxed: false,
|
||||
});
|
||||
|
||||
await expect(
|
||||
tool?.execute?.("call", { url: "https://example.com/readability-empty" }),
|
||||
).rejects.toThrow("Readability and Firecrawl returned no content");
|
||||
const result = await tool?.execute?.("call", {
|
||||
url: "https://example.com/readability-empty",
|
||||
});
|
||||
const details = result?.details as { status?: string; error?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toContain("Readability and Firecrawl returned no content");
|
||||
});
|
||||
|
||||
it("uses firecrawl when direct fetch fails", async () => {
|
||||
@ -232,17 +238,14 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
sandboxed: false,
|
||||
});
|
||||
|
||||
let message = "";
|
||||
try {
|
||||
await tool?.execute?.("call", { url: "https://example.com/missing" });
|
||||
} catch (error) {
|
||||
message = (error as Error).message;
|
||||
}
|
||||
|
||||
expect(message).toContain("Web fetch failed (404):");
|
||||
expect(message).toContain("Not Found");
|
||||
expect(message).not.toContain("<html");
|
||||
expect(message.length).toBeLessThan(5_000);
|
||||
const result = await tool?.execute?.("call", { url: "https://example.com/missing" });
|
||||
const details = result?.details as { status?: string; error?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toContain("Web fetch failed");
|
||||
expect(details.error).toContain("404");
|
||||
expect(details.error).toContain("Not Found");
|
||||
expect(details.error).not.toContain("<html");
|
||||
expect(details.error!.length).toBeLessThan(5_000);
|
||||
});
|
||||
|
||||
it("strips HTML errors when content-type is missing", async () => {
|
||||
@ -265,8 +268,32 @@ describe("web_fetch extraction fallbacks", () => {
|
||||
sandboxed: false,
|
||||
});
|
||||
|
||||
await expect(tool?.execute?.("call", { url: "https://example.com/oops" })).rejects.toThrow(
|
||||
/Web fetch failed \(500\):.*Oops/,
|
||||
);
|
||||
const result = await tool?.execute?.("call", { url: "https://example.com/oops" });
|
||||
const details = result?.details as { status?: string; error?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toMatch(/Web fetch failed.*500.*Oops/);
|
||||
});
|
||||
|
||||
it("returns error result for network-level TypeError instead of crashing", async () => {
|
||||
const mockFetch = vi.fn(() => Promise.reject(new TypeError("fetch failed")));
|
||||
// @ts-expect-error mock fetch
|
||||
global.fetch = mockFetch;
|
||||
|
||||
const tool = createWebFetchTool({
|
||||
config: {
|
||||
tools: {
|
||||
web: {
|
||||
fetch: { cacheTtlMinutes: 0, firecrawl: { enabled: false } },
|
||||
},
|
||||
},
|
||||
},
|
||||
sandboxed: false,
|
||||
});
|
||||
|
||||
const result = await tool?.execute?.("call", { url: "https://unreachable.example.com/" });
|
||||
const details = result?.details as { status?: string; error?: string; url?: string };
|
||||
expect(details.status).toBe("error");
|
||||
expect(details.error).toContain("fetch failed");
|
||||
expect(details.url).toBe("https://unreachable.example.com/");
|
||||
});
|
||||
});
|
||||
|
||||
Loading…
Reference in New Issue
Block a user