From 6e963d0637c8473f91d89c2f5aef859735c4cc5f Mon Sep 17 00:00:00 2001 From: minpeter Date: Wed, 26 Aug 2026 20:57:22 +0900 Subject: [PATCH 1/3] fix(webfetch): handle bodies without dump --- src/webfetch/fetcher.test.ts | 58 ++++++++++++++++++++++++++ src/webfetch/fetcher.ts | 16 ++++++- test/webfetch-explicit-article.test.ts | 15 +------ test/webfetch.test.ts | 27 ++++++++++++ 4 files changed, 101 insertions(+), 15 deletions(-) create mode 100644 src/webfetch/fetcher.test.ts diff --git a/src/webfetch/fetcher.test.ts b/src/webfetch/fetcher.test.ts new file mode 100644 index 0000000..1d7732b --- /dev/null +++ b/src/webfetch/fetcher.test.ts @@ -0,0 +1,58 @@ +import { createServer, type Server } from "node:http"; +import { afterEach, describe, expect, it } from "vitest"; + +import { fetchUrl } from "./fetcher.js"; + +const servers: Server[] = []; + +async function createFixtureServer(): Promise { + const server = createServer((request, response) => { + if (request.url === "/start") { + response.writeHead(302, { + location: "/final", + "content-type": "text/plain; charset=utf-8", + }); + response.end("redirecting"); + return; + } + + response.writeHead(200, { "content-type": "text/plain; charset=utf-8" }); + response.end("ready"); + }); + await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve)); + const address = server.address(); + if (typeof address !== "object" || address === null) { + throw new Error("Expected TCP server address"); + } + servers.push(server); + return `http://127.0.0.1:${address.port}`; +} + +afterEach(async () => { + await Promise.all( + servers.splice(0).map( + (server) => + new Promise((resolve, reject) => { + server.close((error) => (error ? reject(error) : resolve())); + }), + ), + ); +}); + +describe("fetchUrl", () => { + it("discards a response body without dump", async () => { + // given + const baseUrl = await createFixtureServer(); + + // when + const result = await fetchUrl({ + url: `${baseUrl}/start`, + format: "text", + timeoutSeconds: 1, + }); + + // then + expect(new TextDecoder().decode(result.body)).toBe("ready"); + expect(result.url).toBe(`${baseUrl}/final`); + }); +}); diff --git a/src/webfetch/fetcher.ts b/src/webfetch/fetcher.ts index 8f878b5..664273c 100644 --- a/src/webfetch/fetcher.ts +++ b/src/webfetch/fetcher.ts @@ -47,7 +47,8 @@ interface HttpResponse { interface ResponseBodyStream extends AsyncIterable { destroy(error?: Error): void; - dump(options?: { limit: number; signal?: AbortSignal }): Promise; + dump?(options?: { limit: number; signal?: AbortSignal }): Promise; + once(event: "error", listener: (error: Error) => void): unknown; } export async function fetchUrl(options: FetchOptions): Promise { @@ -209,17 +210,28 @@ function getHeader(headers: IncomingHttpHeaders, name: string): string { } async function discardBody(body: ResponseBodyStream): Promise { + if (typeof body.dump !== "function") { + destroyDiscardedBody(body); + return; + } + try { await body.dump({ limit: 1024 }); } catch (error) { if (error instanceof Error) { - body.destroy(error); + destroyDiscardedBody(body); return; } throw error; } } +function destroyDiscardedBody(body: ResponseBodyStream): void { + // Deliberate teardown can emit an error after the body no longer has a consumer. + body.once("error", () => undefined); + body.destroy(); +} + async function readResponseBody(response: HttpResponse, signal: AbortSignal): Promise { const chunks: Uint8Array[] = []; let total = 0; diff --git a/test/webfetch-explicit-article.test.ts b/test/webfetch-explicit-article.test.ts index dc84bb0..3bbc12f 100644 --- a/test/webfetch-explicit-article.test.ts +++ b/test/webfetch-explicit-article.test.ts @@ -1,15 +1,9 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; - -const readability = vi.hoisted(() => ({ - parse: vi.fn(() => { - throw new Error("Readability should not run for explicit article matches"); - }), -})); +import { describe, expect, it, vi } from "vitest"; vi.mock("@mozilla/readability", () => ({ Readability: class { parse(): unknown { - return readability.parse(); + throw new Error("Readability should not run for explicit article matches"); } }, })); @@ -29,10 +23,6 @@ function explicitArticleHtml(): string { } describe("webfetch explicit article extraction", () => { - beforeEach(() => { - readability.parse.mockClear(); - }); - it("#given an explicit article container #when converting markdown #then skips Readability fallback parsing", () => { // given / when const markdown = htmlToMarkdown(explicitArticleHtml(), "https://example.test/post"); @@ -40,6 +30,5 @@ describe("webfetch explicit article extraction", () => { // then expect(markdown).toContain("# Explicit Article"); expect(markdown).toContain("Explicit article body"); - expect(readability.parse).not.toHaveBeenCalled(); }); }); diff --git a/test/webfetch.test.ts b/test/webfetch.test.ts index 367357a..76ad3ef 100644 --- a/test/webfetch.test.ts +++ b/test/webfetch.test.ts @@ -563,6 +563,33 @@ describe("webfetch", () => { expect(headerValue(challengeHeaders, "sec-ch-ua-platform")).toBe('"Windows"'); }); + it("#given one redirect #when fetching #then returns the final response body", async () => { + // given + const visitedPaths: string[] = []; + const server = await createFixtureServer((request, response) => { + visitedPaths.push(request.url ?? ""); + if (request.url === "/start") { + response.writeHead(302, { + location: "/final", + "content-type": "text/plain; charset=utf-8", + }); + response.end("redirecting"); + return; + } + + response.writeHead(200, { "content-type": "text/html; charset=utf-8" }); + response.end("

Redirect Complete

Final page.

"); + }); + + // when + const result = await executeWebfetch({ url: `${server.baseUrl}/start`, format: "markdown" }); + + // then + expect(textContent(result)).toContain("# Redirect Complete"); + expect(textContent(result)).toContain("Final page."); + expect(visitedPaths).toEqual(["/start", "/final"]); + }); + it("#given too many redirects #when fetching #then returns the final redirect response body", async () => { // given const server = await createFixtureServer((_request, response) => { From 313d476d47c04fdd0e89d09f16fe636b1a011f1e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 27 Aug 2026 00:31:33 +0900 Subject: [PATCH 2/3] test: make explicit-article regression fail when Readability runs --- test/webfetch-explicit-article.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/test/webfetch-explicit-article.test.ts b/test/webfetch-explicit-article.test.ts index 3bbc12f..765a830 100644 --- a/test/webfetch-explicit-article.test.ts +++ b/test/webfetch-explicit-article.test.ts @@ -1,9 +1,12 @@ import { describe, expect, it, vi } from "vitest"; +let readabilityParseCalls = 0; + vi.mock("@mozilla/readability", () => ({ Readability: class { parse(): unknown { - throw new Error("Readability should not run for explicit article matches"); + readabilityParseCalls += 1; + return null; } }, })); @@ -30,5 +33,6 @@ describe("webfetch explicit article extraction", () => { // then expect(markdown).toContain("# Explicit Article"); expect(markdown).toContain("Explicit article body"); + expect(readabilityParseCalls).toBe(0); }); }); From 649d19013c5ed628150471e4ecc860ef3b9ebb9e Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Thu, 27 Aug 2026 01:19:16 +0900 Subject: [PATCH 3/3] ci: retrigger checks