diff --git a/package-lock.json b/package-lock.json index 9233754eb..880105add 100644 --- a/package-lock.json +++ b/package-lock.json @@ -25,6 +25,7 @@ "prettier": "3.3.2", "semantic-release-plugin-update-version-in-files": "^1.0.0", "typescript": "^5.0.0", + "undici": "^6.19.2", "vitest": "^2.0.0" }, "engines": { @@ -2570,6 +2571,15 @@ "node": ">=14.17" } }, + "node_modules/undici": { + "version": "6.19.2", + "resolved": "https://registry.npmjs.org/undici/-/undici-6.19.2.tgz", + "integrity": "sha512-JfjKqIauur3Q6biAtHJ564e3bWa8VvT+7cSiOJHFbX4Erv6CLGDpg8z+Fmg/1OI/47RA+GI2QZaF48SSaLvyBA==", + "dev": true, + "engines": { + "node": ">=18.17" + } + }, "node_modules/undici-types": { "version": "5.25.3", "dev": true, diff --git a/package.json b/package.json index 3f895d4ef..bdedb73fe 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "prettier": "3.3.2", "semantic-release-plugin-update-version-in-files": "^1.0.0", "typescript": "^5.0.0", + "undici": "^6.19.2", "vitest": "^2.0.0" }, "release": { diff --git a/src/fetch-wrapper.ts b/src/fetch-wrapper.ts index 23576cdbb..3db220bf5 100644 --- a/src/fetch-wrapper.ts +++ b/src/fetch-wrapper.ts @@ -1,28 +1,10 @@ import { isPlainObject } from "./is-plain-object.js"; import { RequestError } from "@octokit/request-error"; -import type { EndpointInterface } from "@octokit/types"; +import type { EndpointInterface, OctokitResponse } from "@octokit/types"; -export default function fetchWrapper( +export default async function fetchWrapper( requestOptions: ReturnType, -) { - const log = - requestOptions.request && requestOptions.request.log - ? requestOptions.request.log - : console; - const parseSuccessResponseBody = - requestOptions.request?.parseSuccessResponseBody !== false; - - if ( - isPlainObject(requestOptions.body) || - Array.isArray(requestOptions.body) - ) { - requestOptions.body = JSON.stringify(requestOptions.body); - } - - let headers: { [header: string]: string } = {}; - let status: number; - let url: string; - +): Promise> { const fetch: typeof globalThis.fetch = requestOptions.request?.fetch || globalThis.fetch; @@ -32,109 +14,46 @@ export default function fetchWrapper( ); } - return fetch(requestOptions.url, { - method: requestOptions.method, - body: requestOptions.body, - redirect: requestOptions.request?.redirect, - // Header values must be `string` - headers: Object.fromEntries( - Object.entries(requestOptions.headers).map(([name, value]) => [ - name, - String(value), - ]), - ), - signal: requestOptions.request?.signal, - // duplex must be set if request.body is ReadableStream or Async Iterables. - // See https://fetch.spec.whatwg.org/#dom-requestinit-duplex. - ...(requestOptions.body && { duplex: "half" }), - }) - .then(async (response) => { - url = response.url; - status = response.status; - - for (const keyAndValue of response.headers) { - headers[keyAndValue[0]] = keyAndValue[1]; - } - - if ("deprecation" in headers) { - const matches = - headers.link && headers.link.match(/<([^>]+)>; rel="deprecation"/); - const deprecationLink = matches && matches.pop(); - log.warn( - `[@octokit/request] "${requestOptions.method} ${ - requestOptions.url - }" is deprecated. It is scheduled to be removed on ${headers.sunset}${ - deprecationLink ? `. See ${deprecationLink}` : "" - }`, - ); - } - - if (status === 204 || status === 205) { - return; - } - - // GitHub API returns 200 for HEAD requests - if (requestOptions.method === "HEAD") { - if (status < 400) { - return; - } - - throw new RequestError(response.statusText, status, { - response: { - url, - status, - headers, - data: undefined, - }, - request: requestOptions, - }); - } - - if (status === 304) { - throw new RequestError("Not modified", status, { - response: { - url, - status, - headers, - data: await getResponseData(response), - }, - request: requestOptions, - }); - } - - if (status >= 400) { - const data = await getResponseData(response); - - const error = new RequestError(toErrorMessage(data), status, { - response: { - url, - status, - headers, - data, - }, - request: requestOptions, - }); + const log = requestOptions.request?.log || console; + const parseSuccessResponseBody = + requestOptions.request?.parseSuccessResponseBody !== false; + const body = + isPlainObject(requestOptions.body) || Array.isArray(requestOptions.body) + ? JSON.stringify(requestOptions.body) + : requestOptions.body; + + // Header values must be `string` + const requestHeaders = Object.fromEntries( + Object.entries(requestOptions.headers).map(([name, value]) => [ + name, + String(value), + ]), + ); + + let fetchResponse: Response; + + try { + fetchResponse = await fetch(requestOptions.url, { + method: requestOptions.method, + body, + redirect: requestOptions.request?.redirect, + headers: requestHeaders, + signal: requestOptions.request?.signal, + // duplex must be set if request.body is ReadableStream or Async Iterables. + // See https://fetch.spec.whatwg.org/#dom-requestinit-duplex. + ...(requestOptions.body && { duplex: "half" }), + }); + // wrap fetch errors as RequestError if it is not a AbortError + } catch (error) { + let message = "Unknown Error"; + if (error instanceof Error) { + if (error.name === "AbortError") { + (error as RequestError).status = 500; throw error; } - return parseSuccessResponseBody - ? await getResponseData(response) - : response.body; - }) - .then((data) => { - return { - status, - url, - headers, - data, - }; - }) - .catch((error) => { - if (error instanceof RequestError) throw error; - else if (error.name === "AbortError") throw error; - - let message = error.message; + message = error.message; // undici throws a TypeError for network errors // and puts the error message in `error.cause` @@ -146,14 +65,87 @@ export default function fetchWrapper( message = error.cause; } } + } + + const requestError = new RequestError(message, 500, { + request: requestOptions, + }); + requestError.cause = error; + + throw requestError; + } + + const status = fetchResponse.status; + const url = fetchResponse.url; + const responseHeaders: { [header: string]: string } = {}; + + for (const [key, value] of fetchResponse.headers) { + responseHeaders[key] = value; + } + + const octokitResponse: OctokitResponse = { + url, + status, + headers: responseHeaders, + data: "", + }; + + if ("deprecation" in responseHeaders) { + const matches = + responseHeaders.link && + responseHeaders.link.match(/<([^>]+)>; rel="deprecation"/); + const deprecationLink = matches && matches.pop(); + log.warn( + `[@octokit/request] "${requestOptions.method} ${ + requestOptions.url + }" is deprecated. It is scheduled to be removed on ${responseHeaders.sunset}${ + deprecationLink ? `. See ${deprecationLink}` : "" + }`, + ); + } + + if (status === 204 || status === 205) { + return octokitResponse; + } + + // GitHub API returns 200 for HEAD requests + if (requestOptions.method === "HEAD") { + if (status < 400) { + return octokitResponse; + } + + throw new RequestError(fetchResponse.statusText, status, { + response: octokitResponse, + request: requestOptions, + }); + } + + if (status === 304) { + octokitResponse.data = await getResponseData(fetchResponse); + + throw new RequestError("Not modified", status, { + response: octokitResponse, + request: requestOptions, + }); + } + + if (status >= 400) { + octokitResponse.data = await getResponseData(fetchResponse); - throw new RequestError(message, 500, { - request: requestOptions, - }); + throw new RequestError(toErrorMessage(octokitResponse.data), status, { + response: octokitResponse, + request: requestOptions, }); + } + + octokitResponse.data = parseSuccessResponseBody + ? await getResponseData(fetchResponse) + : fetchResponse.body; + + return octokitResponse; } -async function getResponseData(response: Response) { +async function getResponseData(response: Response): Promise { const contentType = response.headers.get("content-type"); if (/application\/json/.test(contentType!)) { return ( diff --git a/test/mock-request-http-server.ts b/test/mock-request-http-server.ts index 3e5f68e99..eb9231e64 100644 --- a/test/mock-request-http-server.ts +++ b/test/mock-request-http-server.ts @@ -10,6 +10,7 @@ import defaults from "../src/defaults.ts"; export default async function mockRequestHttpServer( requestListener: RequestListener, + fetch = globalThis.fetch, ): Promise< RequestInterface & { closeMockServer: () => void; @@ -25,6 +26,9 @@ export default async function mockRequestHttpServer( const request = withDefaults(endpoint, { ...defaults, baseUrl, + request: { + fetch, + }, }) as RequestInterface & { closeMockServer: () => void; baseUrlMockServer: string; diff --git a/test/request-native-fetch.test.ts b/test/request-native-fetch.test.ts index 26d46d025..0017337ed 100644 --- a/test/request-native-fetch.test.ts +++ b/test/request-native-fetch.test.ts @@ -4,13 +4,12 @@ import { ReadableStream } from "node:stream/web"; import { describe, it, expect, vi } from "vitest"; import { getUserAgent } from "universal-user-agent"; -import fetchMock from "fetch-mock"; import { createAppAuth } from "@octokit/auth-app"; import type { EndpointOptions, RequestInterface } from "@octokit/types"; +import { fetch as undiciFetch, Agent, RequestInit } from "undici"; import bodyParser from "./body-parser.ts"; import mockRequestHttpServer from "./mock-request-http-server.ts"; -import { request } from "../src/index.ts"; const userAgent = `octokit-request.js/0.0.0-development ${getUserAgent()}`; const __filename = new URL(import.meta.url); @@ -324,35 +323,53 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== ).rejects.toHaveProperty("status", 404); }); - it.skip("Binary response with redirect (🤔 unclear how to mock fetch redirect properly)", () => { - const mock = fetchMock - .sandbox() - .get( - "https://codeload.github.com/octokit-fixture-org/get-archive/legacy.tar.gz/master", - { - status: 200, - body: Buffer.from( - "1f8b0800000000000003cb4f2ec9cfce2cd14dcbac28292d4ad5cd2f4ad74d4f2dd14d2c4acec82c4bd53580007d060a0050bfb9b9a90203c428741ac2313436343307222320dbc010a8dc5c81c194124b8905a5c525894540a714e5e797e05347481edd734304e41319ff41ae8e2ebeae7ab92964d801d46f66668227fe0d4d51e3dfc8d0c8d808284f75df6201233cfe951590627ba01d330a46c1281805a3806e000024cb59d6000a0000", - "hex", - ), - headers: { - "content-type": "application/x-gzip", - "content-length": "172", - }, - }, - ); + it("Binary response with redirect", async () => { + expect.assertions(7); - return request("GET /repos/{owner}/{repo}/{archive_format}/{ref}", { - owner: "octokit-fixture-org", - repo: "get-archive", - archive_format: "tarball", - ref: "master", - request: { - fetch: mock, - }, - }).then((response) => { - expect(response.data.length).toEqual(172); + const payload = + "1f8b0800000000000003cb4f2ec9cfce2cd14dcbac28292d4ad5cd2f4ad74d4f2dd14d2c4acec82c4bd53580007d060a0050bfb9b9a90203c428741ac2313436343307222320dbc010a8dc5c81c194124b8905a5c525894540a714e5e797e05347481edd734304e41319ff41ae8e2ebeae7ab92964d801d46f66668227fe0d4d51e3dfc8d0c8d808284f75df6201233cfe951590627ba01d330a46c1281805a3806e000024cb59d6000a0000"; + + const request = await mockRequestHttpServer((req, res) => { + expect(req.method).toBe("GET"); + + switch (req.url) { + case "/octokit-fixture-org/get-archive-1/legacy.tar.gz/master": + { + res.writeHead(301, { + Location: + "/octokit-fixture-org/get-archive-2/legacy.tar.gz/master", + }); + res.end(); + } + break; + case "/octokit-fixture-org/get-archive-2/legacy.tar.gz/master": + { + res.writeHead(200, { + "content-type": "application/x-gzip", + "content-length": "172", + }); + res.end(Buffer.from(payload, "hex")); + } + break; + default: { + res.writeHead(500); + } + } + + res.end(); }); + + const response = await request( + `GET ${request.baseUrlMockServer}/octokit-fixture-org/get-archive-1/legacy.tar.gz/master`, + ); + + expect(response.headers["content-type"]).toEqual("application/x-gzip"); + expect(response.headers["content-length"]).toEqual("172"); + expect(response.status).toEqual(200); + expect(response.data).toBeInstanceOf(ArrayBuffer); + expect(zlib.gunzipSync(Buffer.from(payload, "hex")).buffer).toEqual( + response.data, + ); }); it("Binary response", async () => { @@ -707,6 +724,7 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== method: "GET", request: { hook, + fetch: globalThis.fetch, }, url: "/", }); @@ -921,14 +939,16 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== expect(response.data).toEqual({ id: 123 }); }); - it("404 not found", { skip: true }, async () => { - expect.assertions(3); + it("404 not found", async () => { + expect.assertions(5); const request = await mockRequestHttpServer(async (req, res) => { expect(req.method).toBe("GET"); expect(req.url).toBe("/repos/octocat/unknown"); - res.writeHead(404); + res.writeHead(404, { + "content-type": "application/json", + }); res.end( JSON.stringify({ message: "Not Found", @@ -950,7 +970,7 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== } }); - it("Request timeout", { skip: true }, async () => { + it("Request timeout via an AbortSignal", async () => { expect.assertions(4); const request = await mockRequestHttpServer(async (req, res) => { @@ -967,9 +987,55 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== }); try { - await request("GET /"); + await request("GET /", { + request: { + signal: AbortSignal.timeout(500), + }, + }); + throw new Error("should not resolve"); + } catch (error) { + expect(error.name).toEqual("HttpError"); + expect(error.status).toEqual(500); + } + }); + + it("Request timeout via a Dispatcher", async () => { + expect.assertions(5); + + const fetch = (url: string, options: RequestInit) => { + return undiciFetch(url, { + ...options, + dispatcher: new Agent({ + bodyTimeout: 500, + headersTimeout: 500, + keepAliveTimeout: 500, + keepAliveMaxTimeout: 500, + }), + }); + }; + + const request = await mockRequestHttpServer(async (req, res) => { + expect(req.method).toBe("GET"); + expect(req.url).toBe("/"); + + await new Promise((resolve) => setTimeout(resolve, 3000)); + res.writeHead(200); + res.end( + JSON.stringify({ + message: "OK", + }), + ); + }); + + try { + await request("GET /", { + request: { + fetch, + }, + }); throw new Error("should not resolve"); } catch (error) { + expect(error.message).not.toEqual("should not resolve"); expect(error.name).toEqual("HttpError"); expect(error.status).toEqual(500); } diff --git a/test/request.test.ts b/test/request.test.ts index c36a52807..464f7eb95 100644 --- a/test/request.test.ts +++ b/test/request.test.ts @@ -260,11 +260,21 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== ).rejects.toHaveProperty("status", 404); }); - it.skip("Binary response with redirect (🤔 unclear how to mock fetch redirect properly)", () => { + it.skip("Binary response with redirect (🤔 unclear how to mock fetch redirect properly)", async () => { const mock = fetchMock .sandbox() .get( - "https://codeload.github.com/octokit-fixture-org/get-archive/legacy.tar.gz/master", + "https://codeload.github.com/repos/octokit-fixture-org/get-archive-1/tarball/master", + { + status: 301, + headers: { + location: + "https://codeload.github.com/repos/octokit-fixture-org/get-archive-2/tarball/master", + }, + }, + ) + .get( + "https://codeload.github.com/repos/octokit-fixture-org/get-archive-2/tarball/master", { status: 200, body: Buffer.from( @@ -278,17 +288,20 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== }, ); - return request("GET /repos/{owner}/{repo}/{archive_format}/{ref}", { - owner: "octokit-fixture-org", - repo: "get-archive", - archive_format: "tarball", - ref: "master", - request: { - fetch: mock, + const response = await request( + "GET https://codeload.github.com/repos/{owner}/{repo}/{archive_format}/{ref}", + { + owner: "octokit-fixture-org", + repo: "get-archive-1", + archive_format: "tarball", + ref: "master", + request: { + fetch: mock, + }, }, - }).then((response) => { - expect(response.data.length).toEqual(172); - }); + ); + expect(response.status).toEqual(200); + expect(response.data.length).toEqual(172); }); it("Binary response", async () => { @@ -968,7 +981,7 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== } }); - it("Request timeout", async () => { + it("Request timeout via an AbortSignal", async () => { expect.assertions(3); const delay = (millis = 3000) => { @@ -977,28 +990,25 @@ x//0u+zd/R/QRUzLOw4N72/Hu+UG6MNt5iDZFCtapRaKt6OvSBwy8w== }); }; - const mock = (url: string) => { - expect(url).toEqual("https://api.github.com/"); - return delay().then(() => { - return { - status: 200, - headers: {}, - body: { - message: "OK", - }, - }; - }); - }; + const mock = fetchMock.sandbox().get("https://api.github.com/", () => + delay(3000).then(() => ({ + message: "Not Found", + documentation_url: + "https://docs.github.com/en/rest/reference/repos#get-a-repository", + })), + ); try { await request("GET /", { request: { fetch: mock, + signal: AbortSignal.timeout(500), }, }); throw new Error("should not resolve"); } catch (error) { - expect(error.name).toEqual("HttpError"); + expect(error.name).toEqual("AbortError"); + expect(error.message).toEqual("The operation was aborted."); expect(error.status).toEqual(500); } });