From 58c7dc2785277ebf32bbc5bb99f0d5c5897e839c Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 16 Apr 2024 15:18:42 -0700 Subject: [PATCH 01/28] feat!: Upgrade to `node-fetch` v3 --- README.md | 5 +-- package.json | 5 +-- src/common.ts | 5 +-- src/gaxios.ts | 106 +++++++++++++++++++++++++-------------------- src/retry.ts | 2 +- test/test.getch.ts | 46 ++++++++++++++++---- test/test.retry.ts | 1 - tsconfig.json | 6 ++- 8 files changed, 107 insertions(+), 69 deletions(-) diff --git a/README.md b/README.md index 7cf8b6b8..de0c07c6 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ interface GaxiosOptions = { headers: { 'some': 'header' }, // The data to send in the body of the request. Data objects will be - // serialized as JSON. + // serialized as JSON, except for `FormData`. // // Note: if you would like to provide a Content-Type header other than // application/json you you must provide a string or readable stream, rather @@ -151,8 +151,7 @@ interface GaxiosOptions = { // Enables default configuration for retries. retry: boolean, - // Cancelling a request requires the `abort-controller` library. - // See https://github.com/bitinn/node-fetch#request-cancellation-with-abortsignal + // Enables aborting via AbortController signal?: AbortSignal /** diff --git a/package.json b/package.json index 313fdc6d..34329cc0 100644 --- a/package.json +++ b/package.json @@ -47,18 +47,15 @@ "@types/mv": "^2.1.0", "@types/ncp": "^2.0.1", "@types/node": "^20.0.0", - "@types/node-fetch": "^2.5.7", "@types/sinon": "^17.0.0", "@types/tmp": "0.2.6", "@types/uuid": "^9.0.0", - "abort-controller": "^3.0.0", "assert": "^2.0.0", "browserify": "^17.0.0", "c8": "^8.0.0", "cors": "^2.8.5", "execa": "^5.0.0", "express": "^4.16.4", - "form-data": "^4.0.0", "gts": "^5.0.0", "is-docker": "^2.0.0", "karma": "^6.0.0", @@ -89,7 +86,7 @@ "extend": "^3.0.2", "https-proxy-agent": "^7.0.1", "is-stream": "^2.0.0", - "node-fetch": "^2.6.9", + "node-fetch": "^3.3.2", "uuid": "^9.0.1" } } diff --git a/src/common.ts b/src/common.ts index d4a2678a..b8cae2e3 100644 --- a/src/common.ts +++ b/src/common.ts @@ -203,9 +203,8 @@ export interface GaxiosOptions { validateStatus?: (status: number) => boolean; retryConfig?: RetryConfig; retry?: boolean; - // Should be instance of https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal - // interface. Left as 'any' due to incompatibility between spec and abort-controller. - signal?: any; + // Enables aborting via AbortController + signal?: AbortSignal; size?: number; /** * Implementation of `fetch` to use when making the API call. By default, diff --git a/src/gaxios.ts b/src/gaxios.ts index a0daeec5..df5fe64d 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -14,11 +14,12 @@ import extend from 'extend'; import {Agent} from 'http'; import {Agent as HTTPSAgent} from 'https'; -import nodeFetch from 'node-fetch'; import qs from 'querystring'; import isStream from 'is-stream'; import {URL} from 'url'; +import type nodeFetch from 'node-fetch' with {'resolution-mode': 'import'}; + import { FetchResponse, GaxiosMultipartOptions, @@ -30,21 +31,11 @@ import { defaultErrorRedactor, } from './common'; import {getRetryConfig} from './retry'; -import {PassThrough, Stream, pipeline} from 'stream'; +import {Readable} from 'stream'; import {v4} from 'uuid'; /* eslint-disable @typescript-eslint/no-explicit-any */ -const fetch = hasFetch() ? window.fetch : nodeFetch; - -function hasWindow() { - return typeof window !== 'undefined' && !!window; -} - -function hasFetch() { - return hasWindow() && !!window.fetch; -} - function hasBuffer() { return typeof Buffer !== 'undefined'; } @@ -94,8 +85,14 @@ export class Gaxios { private async _defaultAdapter( opts: GaxiosOptions ): Promise> { - const fetchImpl = opts.fetchImplementation || fetch; - const res = (await fetchImpl(opts.url, opts)) as FetchResponse; + const fetchImpl = opts.fetchImplementation || (await Gaxios.#getFetch()); + + // node-fetch v3 warns when `data` is present + // https://github.com/node-fetch/node-fetch/issues/1000 + const preparedOpts = {...opts}; + delete preparedOpts.data; + + const res = await fetchImpl(opts.url, preparedOpts); const data = await this.getResponseData(opts, res); return this.translateResponse(opts, res, data); } @@ -122,10 +119,10 @@ export class Gaxios { if (opts.responseType === 'stream') { let response = ''; await new Promise(resolve => { - (translatedResponse?.data as Stream).on('data', chunk => { + (translatedResponse?.data as Readable).on('data', chunk => { response += chunk; }); - (translatedResponse?.data as Stream).on('end', resolve); + (translatedResponse?.data as Readable).on('end', resolve); }); translatedResponse.data = response as T; } @@ -164,15 +161,13 @@ export class Gaxios { switch (opts.responseType) { case 'stream': return res.body; - case 'json': { - let data = await res.text(); + case 'json': try { - data = JSON.parse(data); + return await res.json(); } catch { - // continue + // fallback to returning text + return await res.text(); } - return data as {}; - } case 'arraybuffer': return res.arrayBuffer(); case 'blob': @@ -267,11 +262,17 @@ export class Gaxios { } opts.headers = opts.headers || {}; + + // FormData is available in Node.js versions 18.0.0+, however there is a runtime + // warning until 18.13.0. Additionally, `node-fetch` v3 only supports the official + // `FormData` or its own exported `FormData` class: + // - https://nodejs.org/api/globals.html#class-formdata + // - https://nodejs.org/en/blog/release/v18.13.0 + // - https://github.com/node-fetch/node-fetch/issues/1167 + // const isFormData = opts?.data instanceof FormData; + const isFormData = opts.data?.constructor?.name === 'FormData'; + if (opts.multipart === undefined && opts.data) { - const isFormData = - typeof FormData === 'undefined' - ? false - : opts?.data instanceof FormData; if (isStream.readable(opts.data)) { opts.body = opts.data; } else if (hasBuffer() && Buffer.isBuffer(opts.data)) { @@ -280,22 +281,19 @@ export class Gaxios { if (!hasHeader(opts, 'Content-Type')) { opts.headers['Content-Type'] = 'application/json'; } - } else if (typeof opts.data === 'object') { - // If www-form-urlencoded content type has been set, but data is - // provided as an object, serialize the content using querystring: - if (!isFormData) { - if ( - getHeader(opts, 'content-type') === - 'application/x-www-form-urlencoded' - ) { - opts.body = opts.paramsSerializer(opts.data); - } else { - // } else if (!(opts.data instanceof FormData)) { - if (!hasHeader(opts, 'Content-Type')) { - opts.headers['Content-Type'] = 'application/json'; - } - opts.body = JSON.stringify(opts.data); + } else if (typeof opts.data === 'object' && !isFormData) { + if ( + getHeader(opts, 'content-type') === + 'application/x-www-form-urlencoded' + ) { + // If www-form-urlencoded content type has been set, but data is + // provided as an object, serialize the content + opts.body = opts.paramsSerializer(opts.data); + } else { + if (!hasHeader(opts, 'Content-Type')) { + opts.headers['Content-Type'] = 'application/json'; } + opts.body = JSON.stringify(opts.data); } } else { opts.body = opts.data; @@ -306,12 +304,8 @@ export class Gaxios { // and the dependency on UUID removed const boundary = v4(); opts.headers['Content-Type'] = `multipart/related; boundary=${boundary}`; - const bodyStream = new PassThrough(); - opts.body = bodyStream; - pipeline( - this.getMultipartRequest(opts.multipart, boundary), - bodyStream, - () => {} + opts.body = Readable.from( + this.getMultipartRequest(opts.multipart, boundary) ); } @@ -475,6 +469,14 @@ export class Gaxios { // using `import` to dynamically import the types here static #proxyAgent?: typeof import('https-proxy-agent').HttpsProxyAgent; + /** + * A cache for the lazily-loaded fetch library. + * + * Should use {@link Gaxios[#getFetch]} to retrieve. + */ + // + static #fetch?: typeof nodeFetch | typeof fetch; + /** * Imports, caches, and returns a proxy agent - if not already imported * @@ -485,4 +487,14 @@ export class Gaxios { return this.#proxyAgent; } + + static async #getFetch() { + const hasWindow = typeof window !== 'undefined' && !!window; + + this.#fetch ||= hasWindow + ? window.fetch + : (await import('node-fetch')).default; + + return this.#fetch; + } } diff --git a/src/retry.ts b/src/retry.ts index 28ad6d7e..6fc90f56 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -68,7 +68,7 @@ export async function getRetryConfig(err: GaxiosError) { const delay = retryDelay + ((Math.pow(2, config.currentRetryAttempt) - 1) / 2) * 1000; - // We're going to retry! Incremenent the counter. + // We're going to retry! Increment the counter. err.config.retryConfig!.currentRetryAttempt! += 1; // Create a promise that invokes the retry after the backOffDelay diff --git a/test/test.getch.ts b/test/test.getch.ts index 353ac961..562d5417 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -16,7 +16,6 @@ import nock from 'nock'; import sinon from 'sinon'; import stream, {Readable} from 'stream'; import {describe, it, afterEach} from 'mocha'; -import fetch from 'node-fetch'; import {HttpsProxyAgent} from 'https-proxy-agent'; import { Gaxios, @@ -30,8 +29,6 @@ import {GAXIOS_ERROR_SYMBOL, Headers} from '../src/common'; import {pkg} from '../src/util'; import qs from 'querystring'; import fs from 'fs'; -import {Blob} from 'node-fetch'; -global.FormData = require('form-data'); nock.disableNetConnect(); @@ -604,10 +601,33 @@ describe('🥁 configuration options', () => { }); it('should not stringify the data if it is appended by a form', async () => { + const FormData = (await import('node-fetch')).FormData; const formData = new FormData(); formData.append('test', '123'); - // I don't think matching formdata is supported in nock, so skipping: https://github.com/nock/nock/issues/887 - const scope = nock(url).post('/').reply(200); + + const scope = nock(url) + .post('/', body => { + /** + * Sample from native `node-fetch` + * body: '------3785545705014550845559551617\r\n' + + * 'Content-Disposition: form-data; name="test"\r\n' + + * '\r\n' + + * '123\r\n' + + * '------3785545705014550845559551617--', + */ + + /** + * Sample from native `fetch` + * body: '------formdata-undici-0.39470493152687736\r\n' + + * 'Content-Disposition: form-data; name="test"\r\n' + + * '\r\n' + + * '123\r\n' + + * '------formdata-undici-0.39470493152687736--', + */ + + return body.match('Content-Disposition: form-data;'); + }) + .reply(200); const res = await request({ url, method: 'POST', @@ -615,14 +635,22 @@ describe('🥁 configuration options', () => { }); scope.done(); assert.deepStrictEqual(res.config.data, formData); + assert.ok(res.config.body instanceof FormData); assert.ok(res.config.data instanceof FormData); - assert.deepEqual(res.config.body, undefined); }); it('should allow explicitly setting the fetch implementation to node-fetch', async () => { + let customFetchCalled = false; + const nodeFetch = (await import('node-fetch')).default; + const myFetch = (...args: Parameters) => { + customFetchCalled = true; + return nodeFetch(...args); + }; + const scope = nock(url).get('/').reply(200); - const res = await request({url, fetchImplementation: fetch}); + const res = await request({url, fetchImplementation: myFetch}); scope.done(); + assert(customFetchCalled); assert.deepStrictEqual(res.status, 200); }); @@ -777,7 +805,9 @@ describe('🎏 data handling', () => { const res = await request({url}); scope.done(); assert.ok(res.data); - assert.strictEqual(res.statusText, 'OK'); + // node-fetch and native fetch specs differ... + // https://github.com/node-fetch/node-fetch/issues/1066 + assert.strictEqual(typeof res.statusText, 'string'); }); it('should return JSON when response Content-Type=application/json', async () => { diff --git a/test/test.retry.ts b/test/test.retry.ts index 53a07737..e2801d69 100644 --- a/test/test.retry.ts +++ b/test/test.retry.ts @@ -11,7 +11,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {AbortController} from 'abort-controller'; import assert from 'assert'; import nock from 'nock'; import {describe, it, afterEach} from 'mocha'; diff --git a/tsconfig.json b/tsconfig.json index dac05e8b..e10edbe0 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -1,10 +1,12 @@ { "extends": "./node_modules/gts/tsconfig-google.json", "compilerOptions": { - "lib": ["es2015", "dom"], + "lib": ["es2020", "dom"], "rootDir": ".", "outDir": "build", - "esModuleInterop": true + "esModuleInterop": true, + "module": "Node16", + "moduleResolution": "Node16", }, "include": [ "src/*.ts", From 7fd2a9589ba1579aa7721407c827648360772350 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 17 Apr 2024 10:01:35 -0700 Subject: [PATCH 02/28] refactor: Use native `FormData` in testing --- test/test.getch.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index 562d5417..0a5fe18e 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -601,7 +601,8 @@ describe('🥁 configuration options', () => { }); it('should not stringify the data if it is appended by a form', async () => { - const FormData = (await import('node-fetch')).FormData; + // Optional, can use `node-fetch`'s FormData to avoid warnings in Node < 18.13 + // const FormData = (await import('node-fetch')).FormData; const formData = new FormData(); formData.append('test', '123'); From 23e76260f54139adb4f5594db2a930315ac1ceda Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 19 Apr 2024 12:13:13 -0700 Subject: [PATCH 03/28] feat: `fetch` (POC) --- README.md | 12 +-- package.json | 4 +- src/common.ts | 133 ++++++++++++++---------------- src/gaxios.ts | 200 ++++++++++++++++++--------------------------- src/retry.ts | 10 ++- test/test.getch.ts | 82 ++++++++----------- 6 files changed, 186 insertions(+), 255 deletions(-) diff --git a/README.md b/README.md index de0c07c6..fb29fe1a 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![codecov](https://codecov.io/gh/googleapis/gaxios/branch/master/graph/badge.svg)](https://codecov.io/gh/googleapis/gaxios) [![Code Style: Google](https://img.shields.io/badge/code%20style-google-blueviolet.svg)](https://github.com/google/gts) -> An HTTP request client that provides an `axios` like interface over top of `node-fetch`. +> An HTTP request client that provides an `axios` like interface over top of `fetch`. ## Install @@ -71,10 +71,6 @@ interface GaxiosOptions = { // Defaults to `0`, which is the same as unset. maxContentLength: 2000, - // The max number of HTTP redirects to follow. - // Defaults to 100. - maxRedirects: 100, - // The querystring parameters that will be encoded using `qs` and // appended to the url params: { @@ -114,9 +110,9 @@ interface GaxiosOptions = { // status code. Defaults to (>= 200 && < 300) validateStatus: (status: number) => true, - // Implementation of `fetch` to use when making the API call. By default, - // will use the browser context if available, and fall back to `node-fetch` - // in node.js otherwise. + /** + * Implementation of `fetch` to use when making the API call. Will use `fetch` by default. + */ fetchImplementation?: typeof fetch; // Configuration for retrying of requests. diff --git a/package.json b/package.json index 9cd1baab..39c07b0b 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "multiparty": "^4.2.1", "mv": "^2.1.1", "ncp": "^2.0.0", - "nock": "^13.0.0", + "nock": "^14.0.0-beta.5", "null-loader": "^4.0.0", "puppeteer": "^21.0.0", "sinon": "^17.0.0", @@ -85,8 +85,6 @@ "dependencies": { "extend": "^3.0.2", "https-proxy-agent": "^7.0.1", - "is-stream": "^2.0.0", - "node-fetch": "^3.3.2", "uuid": "^9.0.1" } } diff --git a/src/common.ts b/src/common.ts index b8cae2e3..5f42aa3e 100644 --- a/src/common.ts +++ b/src/common.ts @@ -118,22 +118,17 @@ export class GaxiosError extends Error { } } +/** + * @deprecated - use native {@link globalThis.Headers}. + */ export interface Headers { [index: string]: any; } export type GaxiosPromise = Promise>; -export interface GaxiosXMLHttpRequest { - responseURL: string; -} - -export interface GaxiosResponse { +export interface GaxiosResponse extends Response { config: GaxiosOptions; data: T; - status: number; - statusText: string; - headers: Headers; - request: GaxiosXMLHttpRequest; } export interface GaxiosMultipartOptions { @@ -144,10 +139,12 @@ export interface GaxiosMultipartOptions { /** * Request options that are used to form the request. */ -export interface GaxiosOptions { +export interface GaxiosOptions extends Omit { /** * Optional method to override making the actual HTTP request. Useful * for writing tests. + * + * @deprecated - Use {@link GaxiosOptions.fetchImplementation} instead. */ adapter?: ( options: GaxiosOptions, @@ -169,20 +166,38 @@ export interface GaxiosOptions { | 'OPTIONS' | 'TRACE' | 'PATCH'; + /** + * Recommended: Provide a native {@link globalThis.Headers Headers} object. + * + * @privateRemarks + * + * This type does not have the native {@link globalThis.Headers Headers} in + * its signature as it would break customers looking to modify headers before + * providing to this library. + */ headers?: Headers; + /** + * The data to send in the {@link RequestInit.body} of the request. Data objects will be + * serialized as JSON, except for `FormData`. + * + * Note: if you would like to provide a Content-Type header other than + * application/json you you must provide a string or readable stream, rather + * than an object: + * + * @example + * + * {data: JSON.stringify({some: 'data'})} + * {data: fs.readFile('./some-data.jpeg')} + */ data?: any; - body?: any; /** * The maximum size of the http response content in bytes allowed. */ maxContentLength?: number; - /** - * The maximum number of redirects to follow. Defaults to 20. - */ - maxRedirects?: number; - follow?: number; /** * A collection of parts to send as a `Content-Type: multipart/related` request. + * + * This is passed to {@link RequestInit.body}. */ multipart?: GaxiosMultipartOptions[]; params?: any; @@ -203,15 +218,24 @@ export interface GaxiosOptions { validateStatus?: (status: number) => boolean; retryConfig?: RetryConfig; retry?: boolean; - // Enables aborting via AbortController + /** + * Enables aborting via {@link AbortController} + */ signal?: AbortSignal; - size?: number; /** - * Implementation of `fetch` to use when making the API call. By default, - * will use the browser context if available, and fall back to `node-fetch` - * in node.js otherwise. + * Implementation of `fetch` to use when making the API call. Will use `fetch` by default. + * + * @example + * + * let customFetchCalled = false; + * const myFetch = (...args: Parameters) => { + * customFetchCalled = true; + * return fetch(...args); + * }; + * + * {fetchImplementation: myFetch}; */ - fetchImplementation?: FetchImplementation; + fetchImplementation?: typeof fetch; // Configure client to use mTLS: cert?: string; key?: string; @@ -258,7 +282,7 @@ export interface GaxiosOptions { errorRedactor?: typeof defaultErrorRedactor | false; } /** - * A partial object of `GaxiosResponse` with only redactable keys + * A partial object of `GaxiosOptions` with only redactable keys * * @experimental */ @@ -330,41 +354,6 @@ export interface RetryConfig { retryBackoff?: (err: GaxiosError, defaultBackoffMs: number) => Promise; } -export type FetchImplementation = ( - input: FetchRequestInfo, - init?: FetchRequestInit -) => Promise; - -export type FetchRequestInfo = any; - -export interface FetchResponse { - readonly status: number; - readonly statusText: string; - readonly url: string; - readonly body: unknown | null; - arrayBuffer(): Promise; - blob(): Promise; - readonly headers: FetchHeaders; - json(): Promise; - text(): Promise; -} - -export interface FetchRequestInit { - method?: string; -} - -export interface FetchHeaders { - append(name: string, value: string): void; - delete(name: string): void; - get(name: string): string | null; - has(name: string): boolean; - set(name: string, value: string): void; - forEach( - callbackfn: (value: string, key: string) => void, - thisArg?: any - ): void; -} - function translateData(responseType: string | undefined, data: any) { switch (responseType) { case 'stream': @@ -395,23 +384,27 @@ export function defaultErrorRedactor(data: { const REDACT = '< - See `errorRedactor` option in `gaxios` for configuration>.'; - function redactHeaders(headers?: Headers) { + function redactHeaders(headers?: Headers | globalThis.Headers) { if (!headers) return; - for (const key of Object.keys(headers)) { - // any casing of `Authentication` - if (/^authentication$/i.test(key)) { - headers[key] = REDACT; - } + const source: string[] = + headers instanceof Headers + ? // TS is missing Headers#keys at the time of writing + (headers as {} as {keys(): string[]}).keys() + : Object.keys(headers); + for (const key of source) { + // any casing of `Authentication` // any casing of `Authorization` - if (/^authorization$/i.test(key)) { - headers[key] = REDACT; - } - // anything containing secret, such as 'client secret' - if (/secret/i.test(key)) { - headers[key] = REDACT; + if ( + /^authentication$/i.test(key) || + /^authorization$/i.test(key) || + /secret/i.test(key) + ) { + headers instanceof Headers + ? headers.set(key, REDACT) + : (headers[key] = REDACT); } } } diff --git a/src/gaxios.ts b/src/gaxios.ts index df5fe64d..6fed14e5 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -15,13 +15,9 @@ import extend from 'extend'; import {Agent} from 'http'; import {Agent as HTTPSAgent} from 'https'; import qs from 'querystring'; -import isStream from 'is-stream'; import {URL} from 'url'; -import type nodeFetch from 'node-fetch' with {'resolution-mode': 'import'}; - import { - FetchResponse, GaxiosMultipartOptions, GaxiosError, GaxiosOptions, @@ -36,24 +32,6 @@ import {v4} from 'uuid'; /* eslint-disable @typescript-eslint/no-explicit-any */ -function hasBuffer() { - return typeof Buffer !== 'undefined'; -} - -function hasHeader(options: GaxiosOptions, header: string) { - return !!getHeader(options, header); -} - -function getHeader(options: GaxiosOptions, header: string): string | undefined { - header = header.toLowerCase(); - for (const key of Object.keys(options?.headers || {})) { - if (header === key.toLowerCase()) { - return options.headers![key]; - } - } - return undefined; -} - export class Gaxios { protected agentCache = new Map< string | URL, @@ -78,23 +56,19 @@ export class Gaxios { * @param opts Set of HTTP options that will be used for this HTTP request. */ async request(opts: GaxiosOptions = {}): GaxiosPromise { - opts = await this.#prepareRequest(opts); - return this._request(opts); + const prepared = await this.#prepareRequest(opts); + return this._request(prepared); } private async _defaultAdapter( - opts: GaxiosOptions + config: GaxiosOptions ): Promise> { - const fetchImpl = opts.fetchImplementation || (await Gaxios.#getFetch()); + const fetchImpl = config.fetchImplementation || fetch; - // node-fetch v3 warns when `data` is present - // https://github.com/node-fetch/node-fetch/issues/1000 - const preparedOpts = {...opts}; - delete preparedOpts.data; + const res = await fetchImpl(config.url!, config); + const data = await this.getResponseData(config, res); - const res = await fetchImpl(opts.url, preparedOpts); - const data = await this.getResponseData(opts, res); - return this.translateResponse(opts, res, data); + return Object.assign(res, {config, data}) as GaxiosResponse; } /** @@ -117,13 +91,12 @@ export class Gaxios { if (!opts.validateStatus!(translatedResponse.status)) { if (opts.responseType === 'stream') { - let response = ''; - await new Promise(resolve => { - (translatedResponse?.data as Readable).on('data', chunk => { - response += chunk; - }); - (translatedResponse?.data as Readable).on('end', resolve); - }); + const response = []; + + for await (const chunk of opts.data) { + response.push(chunk); + } + translatedResponse.data = response as T; } throw new GaxiosError( @@ -156,18 +129,26 @@ export class Gaxios { private async getResponseData( opts: GaxiosOptions, - res: FetchResponse + res: Response ): Promise { + if ( + opts.maxContentLength && + res.headers.has('content-length') && + opts.maxContentLength < + Number.parseInt(res.headers?.get('content-length') || '') + ) { + throw new GaxiosError( + "Response's `Content-Length` is over the limit.", + opts, + Object.assign(res, {config: opts, data: null}) as GaxiosResponse + ); + } + switch (opts.responseType) { case 'stream': return res.body; case 'json': - try { - return await res.json(); - } catch { - // fallback to returning text - return await res.text(); - } + return res.json(); case 'arraybuffer': return res.arrayBuffer(); case 'blob': @@ -253,45 +234,39 @@ export class Gaxios { opts.url = opts.url + prefix + additionalQueryParams; } - if (typeof options.maxContentLength === 'number') { - opts.size = options.maxContentLength; - } - - if (typeof options.maxRedirects === 'number') { - opts.follow = options.maxRedirects; - } - - opts.headers = opts.headers || {}; + const preparedHeaders = + opts.headers instanceof Headers + ? opts.headers + : new Headers(opts.headers); - // FormData is available in Node.js versions 18.0.0+, however there is a runtime - // warning until 18.13.0. Additionally, `node-fetch` v3 only supports the official - // `FormData` or its own exported `FormData` class: - // - https://nodejs.org/api/globals.html#class-formdata - // - https://nodejs.org/en/blog/release/v18.13.0 - // - https://github.com/node-fetch/node-fetch/issues/1167 - // const isFormData = opts?.data instanceof FormData; - const isFormData = opts.data?.constructor?.name === 'FormData'; + const isFormData = opts?.data instanceof FormData; if (opts.multipart === undefined && opts.data) { - if (isStream.readable(opts.data)) { - opts.body = opts.data; - } else if (hasBuffer() && Buffer.isBuffer(opts.data)) { + if ( + opts.data instanceof ReadableStream || + opts.data instanceof Readable + ) { + opts.body = opts.data as ReadableStream; + } else if ( + opts.data instanceof Blob || + ('Buffer' in globalThis && Buffer.isBuffer(opts.data)) + ) { // Do not attempt to JSON.stringify() a Buffer: opts.body = opts.data; - if (!hasHeader(opts, 'Content-Type')) { - opts.headers['Content-Type'] = 'application/json'; + if (!preparedHeaders.has('content-type')) { + preparedHeaders.set('content-type', 'application/json'); } } else if (typeof opts.data === 'object' && !isFormData) { if ( - getHeader(opts, 'content-type') === + preparedHeaders.get('Content-Type') === 'application/x-www-form-urlencoded' ) { // If www-form-urlencoded content type has been set, but data is // provided as an object, serialize the content opts.body = opts.paramsSerializer(opts.data); } else { - if (!hasHeader(opts, 'Content-Type')) { - opts.headers['Content-Type'] = 'application/json'; + if (!preparedHeaders.has('content-type')) { + preparedHeaders.set('content-type', 'application/json'); } opts.body = JSON.stringify(opts.data); } @@ -303,16 +278,20 @@ export class Gaxios { // this can be replaced with randomUUID() function from crypto // and the dependency on UUID removed const boundary = v4(); - opts.headers['Content-Type'] = `multipart/related; boundary=${boundary}`; + preparedHeaders.set( + 'content-type', + `multipart/related; boundary=${boundary}` + ); + opts.body = Readable.from( this.getMultipartRequest(opts.multipart, boundary) - ); + ) as {} as ReadableStream; } opts.validateStatus = opts.validateStatus || this.validateStatus; opts.responseType = opts.responseType || 'unknown'; - if (!opts.headers['Accept'] && opts.responseType === 'json') { - opts.headers['Accept'] = 'application/json'; + if (!preparedHeaders.has('accept') && opts.responseType === 'json') { + preparedHeaders.set('accept', 'application/json'); } opts.method = opts.method || 'GET'; @@ -359,7 +338,27 @@ export class Gaxios { opts.errorRedactor = defaultErrorRedactor; } - return opts; + if (opts.body && !('duplex' in opts)) { + /** + * required for Node.js and the type isn't available today + * @link https://github.com/nodejs/node/issues/46221 + * @link https://github.com/microsoft/TypeScript-DOM-lib-generator/issues/1483 + */ + (opts as {duplex: string}).duplex = 'half'; + } + + // preserve the original type for auditing later + if (opts.headers instanceof Headers) { + opts.headers = preparedHeaders; + } else { + const headers: Headers = {}; + preparedHeaders.forEach((value, key) => { + headers[key] = value; + }); + opts.headers = headers; + } + + return {...opts}; } /** @@ -378,38 +377,13 @@ export class Gaxios { return qs.stringify(params); } - private translateResponse( - opts: GaxiosOptions, - res: FetchResponse, - data?: T - ): GaxiosResponse { - // headers need to be converted from a map to an obj - const headers = {} as Headers; - res.headers.forEach((value, key) => { - headers[key] = value; - }); - - return { - config: opts, - data: data as T, - headers, - status: res.status, - statusText: res.statusText, - - // XMLHttpRequestLike - request: { - responseURL: res.url, - }, - }; - } - /** * Attempts to parse a response by looking at the Content-Type header. - * @param {FetchResponse} response the HTTP response. + * @param {Response} response the HTTP response. * @returns {Promise} a promise that resolves to the response data. */ private async getResponseDataFromContentType( - response: FetchResponse + response: Response ): Promise { let contentType = response.headers.get('Content-Type'); if (contentType === null) { @@ -469,14 +443,6 @@ export class Gaxios { // using `import` to dynamically import the types here static #proxyAgent?: typeof import('https-proxy-agent').HttpsProxyAgent; - /** - * A cache for the lazily-loaded fetch library. - * - * Should use {@link Gaxios[#getFetch]} to retrieve. - */ - // - static #fetch?: typeof nodeFetch | typeof fetch; - /** * Imports, caches, and returns a proxy agent - if not already imported * @@ -487,14 +453,4 @@ export class Gaxios { return this.#proxyAgent; } - - static async #getFetch() { - const hasWindow = typeof window !== 'undefined' && !!window; - - this.#fetch ||= hasWindow - ? window.fetch - : (await import('node-fetch')).default; - - return this.#fetch; - } } diff --git a/src/retry.ts b/src/retry.ts index 6fc90f56..3f8ab56c 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -95,9 +95,13 @@ export async function getRetryConfig(err: GaxiosError) { function shouldRetryRequest(err: GaxiosError) { const config = getConfig(err); - // node-fetch raises an AbortError if signaled: - // https://github.com/bitinn/node-fetch#request-cancellation-with-abortsignal - if (err.name === 'AbortError' || err.error?.name === 'AbortError') { + // `fetch` raises an AbortError if signaled: + // https://developer.mozilla.org/en-US/docs/Web/API/AbortController/abort + if ( + err.config.signal?.aborted || + err.name === 'AbortError' || + err.error?.name === 'AbortError' + ) { return false; } diff --git a/test/test.getch.ts b/test/test.getch.ts index 0a5fe18e..b889e39e 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -104,7 +104,7 @@ describe('🚙 error handling', () => { it('should not throw an error during a translation error', () => { const notJSON = '.'; - const response: GaxiosResponse = { + const response = { config: { responseType: 'json', }, @@ -112,10 +112,7 @@ describe('🚙 error handling', () => { status: 500, statusText: '', headers: {}, - request: { - responseURL: url, - }, - }; + } as GaxiosResponse; const error = new GaxiosError('translation test', {}, response); @@ -178,9 +175,14 @@ describe('🥁 configuration options', () => { it('should allow setting maxContentLength', async () => { const body = {hello: '🌎'}; - const scope = nock(url).get('/').reply(200, body); + const scope = nock(url) + .get('/') + .reply(200, body, {'content-length': body.toString().length.toString()}); const maxContentLength = 1; - await assert.rejects(request({url, maxContentLength}), /over limit/); + await assert.rejects(request({url, maxContentLength}), (err: Error) => { + return err instanceof GaxiosError && /limit/.test(err.message); + }); + scope.done(); }); @@ -193,27 +195,17 @@ describe('🥁 configuration options', () => { const res = await request({url}); scopes.forEach(x => x.done()); assert.deepStrictEqual(res.data, body); - assert.strictEqual(res.request.responseURL, `${url}/foo`); - }); - - it('should support disabling redirects', async () => { - const scope = nock(url).get('/').reply(302, undefined, {location: '/foo'}); - const maxRedirects = 0; - await assert.rejects(request({url, maxRedirects}), /maximum redirect/); - scope.done(); + assert.strictEqual(res.url, `${url}/foo`); }); it('should allow overriding the adapter', async () => { - const response: GaxiosResponse = { + const response = { data: {hello: '🌎'}, config: {}, status: 200, statusText: 'OK', - headers: {}, - request: { - responseURL: url, - }, - }; + headers: new Headers(), + } as GaxiosResponse; const adapter = () => Promise.resolve(response); const res = await request({url, adapter}); assert.strictEqual(response, res); @@ -601,22 +593,11 @@ describe('🥁 configuration options', () => { }); it('should not stringify the data if it is appended by a form', async () => { - // Optional, can use `node-fetch`'s FormData to avoid warnings in Node < 18.13 - // const FormData = (await import('node-fetch')).FormData; const formData = new FormData(); formData.append('test', '123'); const scope = nock(url) .post('/', body => { - /** - * Sample from native `node-fetch` - * body: '------3785545705014550845559551617\r\n' + - * 'Content-Disposition: form-data; name="test"\r\n' + - * '\r\n' + - * '123\r\n' + - * '------3785545705014550845559551617--', - */ - /** * Sample from native `fetch` * body: '------formdata-undici-0.39470493152687736\r\n' + @@ -640,12 +621,11 @@ describe('🥁 configuration options', () => { assert.ok(res.config.data instanceof FormData); }); - it('should allow explicitly setting the fetch implementation to node-fetch', async () => { + it('should allow explicitly setting the fetch implementation', async () => { let customFetchCalled = false; - const nodeFetch = (await import('node-fetch')).default; - const myFetch = (...args: Parameters) => { + const myFetch = (...args: Parameters) => { customFetchCalled = true; - return nodeFetch(...args); + return fetch(...args); }; const scope = nock(url).get('/').reply(200); @@ -764,9 +744,9 @@ describe('🎏 data handling', () => { it('should return stream if asked nicely', async () => { const body = {hello: '🌎'}; const scope = nock(url).get('/').reply(200, body); - const res = await request({url, responseType: 'stream'}); + const res = await request({url, responseType: 'stream'}); scope.done(); - assert(res.data instanceof stream.Readable); + assert(res.data instanceof ReadableStream); }); it('should return an ArrayBuffer if asked nicely', async () => { @@ -806,9 +786,7 @@ describe('🎏 data handling', () => { const res = await request({url}); scope.done(); assert.ok(res.data); - // node-fetch and native fetch specs differ... - // https://github.com/node-fetch/node-fetch/issues/1066 - assert.strictEqual(typeof res.statusText, 'string'); + assert.strictEqual(res.statusText, 'OK'); }); it('should return JSON when response Content-Type=application/json', async () => { @@ -990,11 +968,16 @@ describe('🎏 data handling', () => { // config redactions - headers assert(e.config.headers); - assert.deepStrictEqual(e.config.headers, { + const expectedRequestHeaders = new Headers({ ...config.headers, // non-redactables should be present Authentication: REDACT, AUTHORIZATION: REDACT, }); + const actualHeaders = new Headers(e.config.headers); + + expectedRequestHeaders.forEach((value, key) => { + assert.equal(actualHeaders.get(key), value); + }); // config redactions - data assert.deepStrictEqual(e.config.data, { @@ -1019,16 +1002,17 @@ describe('🎏 data handling', () => { assert(e.response); assert.deepStrictEqual(e.response.config, e.config); - const expectedHeaders: Headers = { + const expectedResponseHeaders = new Headers({ ...responseHeaders, // non-redactables should be present - authentication: REDACT, - authorization: REDACT, - }; + }); + + expectedResponseHeaders.set('authentication', REDACT); + expectedResponseHeaders.set('authorization', REDACT); - delete expectedHeaders['AUTHORIZATION']; - delete expectedHeaders['Authentication']; + expectedResponseHeaders.forEach((value, key) => { + assert.equal(e.response?.headers.get(key), value); + }); - assert.deepStrictEqual(e.response.headers, expectedHeaders); assert.deepStrictEqual(e.response.data, { ...response, // non-redactables should be present assertion: REDACT, From a61cfa5cbd068d0fd26ebaccbd547ca6431d88cc Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 19 Apr 2024 15:57:01 -0700 Subject: [PATCH 04/28] feat!: Improve spec compliance --- README.md | 12 ++- package.json | 3 +- src/common.ts | 162 +++++++++++++++++++++--------------- src/gaxios.ts | 201 +++++++++++++++++++++++++-------------------- src/retry.ts | 8 +- test/test.getch.ts | 96 +++++++++++++--------- 6 files changed, 271 insertions(+), 211 deletions(-) diff --git a/README.md b/README.md index de0c07c6..19bc888c 100644 --- a/README.md +++ b/README.md @@ -71,10 +71,6 @@ interface GaxiosOptions = { // Defaults to `0`, which is the same as unset. maxContentLength: 2000, - // The max number of HTTP redirects to follow. - // Defaults to 100. - maxRedirects: 100, - // The querystring parameters that will be encoded using `qs` and // appended to the url params: { @@ -105,6 +101,8 @@ interface GaxiosOptions = { // The expected return type of the request. Options are: // json | stream | blob | arraybuffer | text | unknown // Defaults to `unknown`. + // If the `fetchImplementation` is native `fetch`, the + // stream is a `ReadableStream`, otherwise `readable.Stream` responseType: 'unknown', // The node.js http agent to use for the request. @@ -114,9 +112,9 @@ interface GaxiosOptions = { // status code. Defaults to (>= 200 && < 300) validateStatus: (status: number) => true, - // Implementation of `fetch` to use when making the API call. By default, - // will use the browser context if available, and fall back to `node-fetch` - // in node.js otherwise. + /** + * Implementation of `fetch` to use when making the API call. Will use `fetch` by default. + */ fetchImplementation?: typeof fetch; // Configuration for retrying of requests. diff --git a/package.json b/package.json index 9cd1baab..6d2e1961 100644 --- a/package.json +++ b/package.json @@ -71,7 +71,7 @@ "multiparty": "^4.2.1", "mv": "^2.1.1", "ncp": "^2.0.0", - "nock": "^13.0.0", + "nock": "^14.0.0-beta.5", "null-loader": "^4.0.0", "puppeteer": "^21.0.0", "sinon": "^17.0.0", @@ -85,7 +85,6 @@ "dependencies": { "extend": "^3.0.2", "https-proxy-agent": "^7.0.1", - "is-stream": "^2.0.0", "node-fetch": "^3.3.2", "uuid": "^9.0.1" } diff --git a/src/common.ts b/src/common.ts index b8cae2e3..cb584ecc 100644 --- a/src/common.ts +++ b/src/common.ts @@ -94,7 +94,8 @@ export class GaxiosError extends Error { try { this.response.data = translateData( this.config.responseType, - this.response?.data + // workaround for `node-fetch`'s `.data` deprecation... + this.response?.bodyUsed ? this.response?.data : undefined ); } catch { // best effort - don't throw an error within an error @@ -118,22 +119,17 @@ export class GaxiosError extends Error { } } +/** + * @deprecated use native {@link globalThis.Headers}. + */ export interface Headers { [index: string]: any; } export type GaxiosPromise = Promise>; -export interface GaxiosXMLHttpRequest { - responseURL: string; -} - -export interface GaxiosResponse { +export interface GaxiosResponse extends Response { config: GaxiosOptions; data: T; - status: number; - statusText: string; - headers: Headers; - request: GaxiosXMLHttpRequest; } export interface GaxiosMultipartOptions { @@ -144,10 +140,12 @@ export interface GaxiosMultipartOptions { /** * Request options that are used to form the request. */ -export interface GaxiosOptions { +export interface GaxiosOptions extends Omit { /** * Optional method to override making the actual HTTP request. Useful * for writing tests. + * + * @deprecated Use {@link GaxiosOptions.fetchImplementation} instead. */ adapter?: ( options: GaxiosOptions, @@ -169,20 +167,48 @@ export interface GaxiosOptions { | 'OPTIONS' | 'TRACE' | 'PATCH'; + /** + * Recommended: Provide a native {@link globalThis.Headers Headers} object. + * + * @privateRemarks + * + * This type does not have the native {@link globalThis.Headers Headers} in + * its signature as it would break customers looking to modify headers before + * providing to this library (new, unnecessary type checks/guards). + */ headers?: Headers; + /** + * The data to send in the {@link RequestInit.body} of the request. Data objects will be + * serialized as JSON, except for `FormData`. + * + * Note: if you would like to provide a Content-Type header other than + * application/json you you must provide a string or readable stream, rather + * than an object: + * + * ```ts + * {data: JSON.stringify({some: 'data'})} + * {data: fs.readFile('./some-data.jpeg')} + * ``` + */ data?: any; - body?: any; /** - * The maximum size of the http response content in bytes allowed. + * The maximum size of the http response `Content-Length` in bytes allowed. */ maxContentLength?: number; /** * The maximum number of redirects to follow. Defaults to 20. + * + * @deprecated non-spec. Should use `20` if enabled per-spec: https://fetch.spec.whatwg.org/#http-redirect-fetch */ maxRedirects?: number; + /** + * @deprecated non-spec. Should use `20` if enabled per-spec: https://fetch.spec.whatwg.org/#http-redirect-fetch + */ follow?: number; /** * A collection of parts to send as a `Content-Type: multipart/related` request. + * + * This is passed to {@link RequestInit.body}. */ multipart?: GaxiosMultipartOptions[]; params?: any; @@ -192,6 +218,10 @@ export interface GaxiosOptions { * @deprecated ignored */ onUploadProgress?: (progressEvent: any) => void; + /** + * If the `fetchImplementation` is native `fetch`, the + * stream is a `ReadableStream`, otherwise `readable.Stream` + */ responseType?: | 'arraybuffer' | 'blob' @@ -203,15 +233,28 @@ export interface GaxiosOptions { validateStatus?: (status: number) => boolean; retryConfig?: RetryConfig; retry?: boolean; - // Enables aborting via AbortController + /** + * Enables aborting via {@link AbortController}. + */ signal?: AbortSignal; + /** + * @deprecated non-spec. https://github.com/node-fetch/node-fetch/issues/1438 + */ size?: number; /** - * Implementation of `fetch` to use when making the API call. By default, - * will use the browser context if available, and fall back to `node-fetch` - * in node.js otherwise. + * Implementation of `fetch` to use when making the API call. Will use `fetch` by default. + * + * @example + * + * let customFetchCalled = false; + * const myFetch = (...args: Parameters) => { + * customFetchCalled = true; + * return fetch(...args); + * }; + * + * {fetchImplementation: myFetch}; */ - fetchImplementation?: FetchImplementation; + fetchImplementation?: typeof fetch; // Configure client to use mTLS: cert?: string; key?: string; @@ -258,7 +301,7 @@ export interface GaxiosOptions { errorRedactor?: typeof defaultErrorRedactor | false; } /** - * A partial object of `GaxiosResponse` with only redactable keys + * A partial object of `GaxiosOptions` with only redactable keys * * @experimental */ @@ -330,41 +373,6 @@ export interface RetryConfig { retryBackoff?: (err: GaxiosError, defaultBackoffMs: number) => Promise; } -export type FetchImplementation = ( - input: FetchRequestInfo, - init?: FetchRequestInit -) => Promise; - -export type FetchRequestInfo = any; - -export interface FetchResponse { - readonly status: number; - readonly statusText: string; - readonly url: string; - readonly body: unknown | null; - arrayBuffer(): Promise; - blob(): Promise; - readonly headers: FetchHeaders; - json(): Promise; - text(): Promise; -} - -export interface FetchRequestInit { - method?: string; -} - -export interface FetchHeaders { - append(name: string, value: string): void; - delete(name: string): void; - get(name: string): string | null; - has(name: string): boolean; - set(name: string, value: string): void; - forEach( - callbackfn: (value: string, key: string) => void, - thisArg?: any - ): void; -} - function translateData(responseType: string | undefined, data: any) { switch (responseType) { case 'stream': @@ -395,25 +403,40 @@ export function defaultErrorRedactor(data: { const REDACT = '< - See `errorRedactor` option in `gaxios` for configuration>.'; - function redactHeaders(headers?: Headers) { + function redactHeaders(headers?: Headers | globalThis.Headers) { if (!headers) return; - for (const key of Object.keys(headers)) { + function check(key: string) { // any casing of `Authentication` - if (/^authentication$/i.test(key)) { - headers[key] = REDACT; - } - // any casing of `Authorization` - if (/^authorization$/i.test(key)) { - headers[key] = REDACT; - } - // anything containing secret, such as 'client secret' - if (/secret/i.test(key)) { - headers[key] = REDACT; + return ( + /^authentication$/i.test(key) || + /^authorization$/i.test(key) || + /secret/i.test(key) + ); + } + + function redactHeadersObject(headers: Headers) { + for (const key of Object.keys(headers)) { + if (check(key)) headers[key] = REDACT; } } + + function redactHeadersHeaders(headers: globalThis.Headers) { + headers.forEach((value, key) => { + if (check(key)) headers.set(key, REDACT); + }); + + // headers.g + } + + // support `node-fetch` Headers and other third-parties + if (headers instanceof Headers || 'set' in headers) { + redactHeadersHeaders(headers as globalThis.Headers); + } else { + redactHeadersObject(headers); + } } function redactString(obj: GaxiosOptions, key: keyof GaxiosOptions) { @@ -480,8 +503,11 @@ export function defaultErrorRedactor(data: { defaultErrorRedactor({config: data.response.config}); redactHeaders(data.response.headers); - redactString(data.response, 'data'); - redactObject(data.response.data); + // workaround for `node-fetch`'s `.data` deprecation... + if ((data.response as {} as Response).bodyUsed) { + redactString(data.response, 'data'); + redactObject(data.response.data); + } } return data; diff --git a/src/gaxios.ts b/src/gaxios.ts index df5fe64d..fff69932 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -15,13 +15,10 @@ import extend from 'extend'; import {Agent} from 'http'; import {Agent as HTTPSAgent} from 'https'; import qs from 'querystring'; -import isStream from 'is-stream'; import {URL} from 'url'; - import type nodeFetch from 'node-fetch' with {'resolution-mode': 'import'}; import { - FetchResponse, GaxiosMultipartOptions, GaxiosError, GaxiosOptions, @@ -36,24 +33,6 @@ import {v4} from 'uuid'; /* eslint-disable @typescript-eslint/no-explicit-any */ -function hasBuffer() { - return typeof Buffer !== 'undefined'; -} - -function hasHeader(options: GaxiosOptions, header: string) { - return !!getHeader(options, header); -} - -function getHeader(options: GaxiosOptions, header: string): string | undefined { - header = header.toLowerCase(); - for (const key of Object.keys(options?.headers || {})) { - if (header === key.toLowerCase()) { - return options.headers![key]; - } - } - return undefined; -} - export class Gaxios { protected agentCache = new Map< string | URL, @@ -78,23 +57,50 @@ export class Gaxios { * @param opts Set of HTTP options that will be used for this HTTP request. */ async request(opts: GaxiosOptions = {}): GaxiosPromise { - opts = await this.#prepareRequest(opts); - return this._request(opts); + const prepared = await this.#prepareRequest(opts); + return this._request(prepared); } private async _defaultAdapter( - opts: GaxiosOptions + config: GaxiosOptions ): Promise> { - const fetchImpl = opts.fetchImplementation || (await Gaxios.#getFetch()); + const fetchImpl = + config.fetchImplementation || + this.defaults.fetchImplementation || + (await Gaxios.#getFetch()); // node-fetch v3 warns when `data` is present // https://github.com/node-fetch/node-fetch/issues/1000 - const preparedOpts = {...opts}; + const preparedOpts = {...config}; delete preparedOpts.data; - const res = await fetchImpl(opts.url, preparedOpts); - const data = await this.getResponseData(opts, res); - return this.translateResponse(opts, res, data); + const res = (await fetchImpl(config.url!, preparedOpts as {})) as Response; + let data = await this.getResponseData(config, res); + + // `node-fetch`'s data isn't writable. Native `fetch`'s is. + if (Object.getOwnPropertyDescriptor(res, 'data')?.configurable) { + // Keep `Response` as a class + return Object.assign(res, {config, data}); + } else { + Object.assign(res, {config}); + + // best effort for `node-fetch`; `.data` is not writable... + return new Proxy(res, { + get: (target, prop, receiver) => { + if (prop === 'data') return data; + + return Reflect.get(target, prop, receiver); + }, + set(target, prop, newValue, receiver) { + if (prop === 'data') { + data = newValue; + return true; + } else { + return Reflect.set(target, prop, newValue, receiver); + } + }, + }) as GaxiosResponse; + } } /** @@ -117,13 +123,12 @@ export class Gaxios { if (!opts.validateStatus!(translatedResponse.status)) { if (opts.responseType === 'stream') { - let response = ''; - await new Promise(resolve => { - (translatedResponse?.data as Readable).on('data', chunk => { - response += chunk; - }); - (translatedResponse?.data as Readable).on('end', resolve); - }); + const response = []; + + for await (const chunk of opts.data) { + response.push(chunk); + } + translatedResponse.data = response as T; } throw new GaxiosError( @@ -156,18 +161,26 @@ export class Gaxios { private async getResponseData( opts: GaxiosOptions, - res: FetchResponse + res: Response ): Promise { + if ( + opts.maxContentLength && + res.headers.has('content-length') && + opts.maxContentLength < + Number.parseInt(res.headers?.get('content-length') || '') + ) { + throw new GaxiosError( + "Response's `Content-Length` is over the limit.", + opts, + Object.assign(res, {config: opts}) as GaxiosResponse + ); + } + switch (opts.responseType) { case 'stream': return res.body; case 'json': - try { - return await res.json(); - } catch { - // fallback to returning text - return await res.text(); - } + return res.json(); case 'arraybuffer': return res.arrayBuffer(); case 'blob': @@ -261,37 +274,44 @@ export class Gaxios { opts.follow = options.maxRedirects; } - opts.headers = opts.headers || {}; + const preparedHeaders = + opts.headers instanceof Headers + ? opts.headers + : new Headers(opts.headers); - // FormData is available in Node.js versions 18.0.0+, however there is a runtime - // warning until 18.13.0. Additionally, `node-fetch` v3 only supports the official - // `FormData` or its own exported `FormData` class: - // - https://nodejs.org/api/globals.html#class-formdata - // - https://nodejs.org/en/blog/release/v18.13.0 - // - https://github.com/node-fetch/node-fetch/issues/1167 - // const isFormData = opts?.data instanceof FormData; - const isFormData = opts.data?.constructor?.name === 'FormData'; + const isFormData = + opts?.data instanceof FormData || + /** + * @deprecated `node-fetch` or another third-party `FormData` instance + **/ + opts.data?.constructor?.name === 'FormData'; if (opts.multipart === undefined && opts.data) { - if (isStream.readable(opts.data)) { - opts.body = opts.data; - } else if (hasBuffer() && Buffer.isBuffer(opts.data)) { + if ( + opts.data instanceof ReadableStream || + opts.data instanceof Readable + ) { + opts.body = opts.data as ReadableStream; + } else if ( + opts.data instanceof Blob || + ('Buffer' in globalThis && Buffer.isBuffer(opts.data)) + ) { // Do not attempt to JSON.stringify() a Buffer: opts.body = opts.data; - if (!hasHeader(opts, 'Content-Type')) { - opts.headers['Content-Type'] = 'application/json'; + if (!preparedHeaders.has('content-type')) { + preparedHeaders.set('content-type', 'application/json'); } } else if (typeof opts.data === 'object' && !isFormData) { if ( - getHeader(opts, 'content-type') === + preparedHeaders.get('Content-Type') === 'application/x-www-form-urlencoded' ) { // If www-form-urlencoded content type has been set, but data is // provided as an object, serialize the content opts.body = opts.paramsSerializer(opts.data); } else { - if (!hasHeader(opts, 'Content-Type')) { - opts.headers['Content-Type'] = 'application/json'; + if (!preparedHeaders.has('content-type')) { + preparedHeaders.set('content-type', 'application/json'); } opts.body = JSON.stringify(opts.data); } @@ -303,16 +323,20 @@ export class Gaxios { // this can be replaced with randomUUID() function from crypto // and the dependency on UUID removed const boundary = v4(); - opts.headers['Content-Type'] = `multipart/related; boundary=${boundary}`; + preparedHeaders.set( + 'content-type', + `multipart/related; boundary=${boundary}` + ); + opts.body = Readable.from( this.getMultipartRequest(opts.multipart, boundary) - ); + ) as {} as ReadableStream; } opts.validateStatus = opts.validateStatus || this.validateStatus; opts.responseType = opts.responseType || 'unknown'; - if (!opts.headers['Accept'] && opts.responseType === 'json') { - opts.headers['Accept'] = 'application/json'; + if (!preparedHeaders.has('accept') && opts.responseType === 'json') { + preparedHeaders.set('accept', 'application/json'); } opts.method = opts.method || 'GET'; @@ -359,6 +383,26 @@ export class Gaxios { opts.errorRedactor = defaultErrorRedactor; } + if (opts.body && !('duplex' in opts)) { + /** + * required for Node.js and the type isn't available today + * @link https://github.com/nodejs/node/issues/46221 + * @link https://github.com/microsoft/TypeScript-DOM-lib-generator/issues/1483 + */ + (opts as {duplex: string}).duplex = 'half'; + } + + // preserve the original type for auditing later + if (opts.headers instanceof Headers) { + opts.headers = preparedHeaders; + } else { + const headers: Headers = {}; + preparedHeaders.forEach((value, key) => { + headers[key] = value; + }); + opts.headers = headers; + } + return opts; } @@ -378,38 +422,13 @@ export class Gaxios { return qs.stringify(params); } - private translateResponse( - opts: GaxiosOptions, - res: FetchResponse, - data?: T - ): GaxiosResponse { - // headers need to be converted from a map to an obj - const headers = {} as Headers; - res.headers.forEach((value, key) => { - headers[key] = value; - }); - - return { - config: opts, - data: data as T, - headers, - status: res.status, - statusText: res.statusText, - - // XMLHttpRequestLike - request: { - responseURL: res.url, - }, - }; - } - /** * Attempts to parse a response by looking at the Content-Type header. - * @param {FetchResponse} response the HTTP response. + * @param {Response} response the HTTP response. * @returns {Promise} a promise that resolves to the response data. */ private async getResponseDataFromContentType( - response: FetchResponse + response: Response ): Promise { let contentType = response.headers.get('Content-Type'); if (contentType === null) { diff --git a/src/retry.ts b/src/retry.ts index 6fc90f56..9608d1b5 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -95,9 +95,11 @@ export async function getRetryConfig(err: GaxiosError) { function shouldRetryRequest(err: GaxiosError) { const config = getConfig(err); - // node-fetch raises an AbortError if signaled: - // https://github.com/bitinn/node-fetch#request-cancellation-with-abortsignal - if (err.name === 'AbortError' || err.error?.name === 'AbortError') { + if ( + err.config.signal?.aborted || + err.name === 'AbortError' || + err.error?.name === 'AbortError' + ) { return false; } diff --git a/test/test.getch.ts b/test/test.getch.ts index 0a5fe18e..9590a10c 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -104,7 +104,7 @@ describe('🚙 error handling', () => { it('should not throw an error during a translation error', () => { const notJSON = '.'; - const response: GaxiosResponse = { + const response = { config: { responseType: 'json', }, @@ -112,14 +112,13 @@ describe('🚙 error handling', () => { status: 500, statusText: '', headers: {}, - request: { - responseURL: url, - }, - }; + // workaround for `node-fetch`'s `.data` deprecation... + bodyUsed: true, + } as GaxiosResponse; const error = new GaxiosError('translation test', {}, response); - assert(error.response, undefined); + assert(error.response); assert.equal(error.response.data, notJSON); }); @@ -178,9 +177,14 @@ describe('🥁 configuration options', () => { it('should allow setting maxContentLength', async () => { const body = {hello: '🌎'}; - const scope = nock(url).get('/').reply(200, body); + const scope = nock(url) + .get('/') + .reply(200, body, {'content-length': body.toString().length.toString()}); const maxContentLength = 1; - await assert.rejects(request({url, maxContentLength}), /over limit/); + await assert.rejects(request({url, maxContentLength}), (err: Error) => { + return err instanceof GaxiosError && /limit/.test(err.message); + }); + scope.done(); }); @@ -193,27 +197,17 @@ describe('🥁 configuration options', () => { const res = await request({url}); scopes.forEach(x => x.done()); assert.deepStrictEqual(res.data, body); - assert.strictEqual(res.request.responseURL, `${url}/foo`); - }); - - it('should support disabling redirects', async () => { - const scope = nock(url).get('/').reply(302, undefined, {location: '/foo'}); - const maxRedirects = 0; - await assert.rejects(request({url, maxRedirects}), /maximum redirect/); - scope.done(); + assert.strictEqual(res.url, `${url}/foo`); }); it('should allow overriding the adapter', async () => { - const response: GaxiosResponse = { + const response = { data: {hello: '🌎'}, config: {}, status: 200, statusText: 'OK', - headers: {}, - request: { - responseURL: url, - }, - }; + headers: new Headers(), + } as GaxiosResponse; const adapter = () => Promise.resolve(response); const res = await request({url, adapter}); assert.strictEqual(response, res); @@ -601,8 +595,6 @@ describe('🥁 configuration options', () => { }); it('should not stringify the data if it is appended by a form', async () => { - // Optional, can use `node-fetch`'s FormData to avoid warnings in Node < 18.13 - // const FormData = (await import('node-fetch')).FormData; const formData = new FormData(); formData.append('test', '123'); @@ -640,19 +632,24 @@ describe('🥁 configuration options', () => { assert.ok(res.config.data instanceof FormData); }); - it('should allow explicitly setting the fetch implementation to node-fetch', async () => { + it('should allow explicitly setting the fetch implementation', async () => { let customFetchCalled = false; - const nodeFetch = (await import('node-fetch')).default; - const myFetch = (...args: Parameters) => { + const myFetch = (...args: Parameters) => { customFetchCalled = true; - return nodeFetch(...args); + return fetch(...args); }; - const scope = nock(url).get('/').reply(200); - const res = await request({url, fetchImplementation: myFetch}); - scope.done(); + const scope = nock(url).post('/').reply(204); + const res = await request({ + url, + method: 'POST', + fetchImplementation: myFetch, + // This `data` ensures the 'duplex' option has been set + data: {sample: 'data'}, + }); assert(customFetchCalled); - assert.deepStrictEqual(res.status, 200); + assert.equal(res.status, 204); + scope.done(); }); it('should be able to disable the `errorRedactor`', async () => { @@ -769,6 +766,18 @@ describe('🎏 data handling', () => { assert(res.data instanceof stream.Readable); }); + it('should return a `ReadableStream` when `fetch` has been provided ', async () => { + const body = {hello: '🌎'}; + const scope = nock(url).get('/').reply(200, body); + const res = await request({ + url, + responseType: 'stream', + fetchImplementation: fetch, + }); + scope.done(); + assert(res.data instanceof ReadableStream); + }); + it('should return an ArrayBuffer if asked nicely', async () => { const body = {hello: '🌎'}; const scope = nock(url).get('/').reply(200, body); @@ -809,6 +818,7 @@ describe('🎏 data handling', () => { // node-fetch and native fetch specs differ... // https://github.com/node-fetch/node-fetch/issues/1066 assert.strictEqual(typeof res.statusText, 'string'); + // assert.strictEqual(res.statusText, 'OK'); }); it('should return JSON when response Content-Type=application/json', async () => { @@ -990,11 +1000,16 @@ describe('🎏 data handling', () => { // config redactions - headers assert(e.config.headers); - assert.deepStrictEqual(e.config.headers, { + const expectedRequestHeaders = new Headers({ ...config.headers, // non-redactables should be present Authentication: REDACT, AUTHORIZATION: REDACT, }); + const actualHeaders = new Headers(e.config.headers); + + expectedRequestHeaders.forEach((value, key) => { + assert.equal(actualHeaders.get(key), value); + }); // config redactions - data assert.deepStrictEqual(e.config.data, { @@ -1019,16 +1034,17 @@ describe('🎏 data handling', () => { assert(e.response); assert.deepStrictEqual(e.response.config, e.config); - const expectedHeaders: Headers = { + const expectedResponseHeaders = new Headers({ ...responseHeaders, // non-redactables should be present - authentication: REDACT, - authorization: REDACT, - }; + }); - delete expectedHeaders['AUTHORIZATION']; - delete expectedHeaders['Authentication']; + expectedResponseHeaders.set('authentication', REDACT); + expectedResponseHeaders.set('authorization', REDACT); + + expectedResponseHeaders.forEach((value, key) => { + assert.equal(e.response?.headers.get(key), value); + }); - assert.deepStrictEqual(e.response.headers, expectedHeaders); assert.deepStrictEqual(e.response.data, { ...response, // non-redactables should be present assertion: REDACT, From 35a73b1e35ffb2fd1b93ccac2b91fae93ad91d72 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 19 Apr 2024 16:01:16 -0700 Subject: [PATCH 05/28] test(temp): Run 18+ --- .github/workflows/ci.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 711957ba..feefb2c8 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,7 +9,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node: [14, 16, 18, 20] + node: [18, 20] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -32,7 +32,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: 14 + node-version: 18 - run: npm install - run: npm test env: @@ -43,7 +43,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: 14 + node-version: 18 - run: npm install - run: npm run lint docs: @@ -52,7 +52,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: 14 + node-version: 18 - run: npm install - run: npm run docs - uses: JustinBeckwith/linkinator-action@v1 From e7addeb4d539fa90335d0624488ac762115e3264 Mon Sep 17 00:00:00 2001 From: Owl Bot Date: Fri, 19 Apr 2024 23:04:10 +0000 Subject: [PATCH 06/28] =?UTF-8?q?=F0=9F=A6=89=20Updates=20from=20OwlBot=20?= =?UTF-8?q?post-processor?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --- .github/workflows/ci.yaml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index feefb2c8..711957ba 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -9,7 +9,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node: [18, 20] + node: [14, 16, 18, 20] steps: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 @@ -32,7 +32,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: 18 + node-version: 14 - run: npm install - run: npm test env: @@ -43,7 +43,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: 18 + node-version: 14 - run: npm install - run: npm run lint docs: @@ -52,7 +52,7 @@ jobs: - uses: actions/checkout@v3 - uses: actions/setup-node@v3 with: - node-version: 18 + node-version: 14 - run: npm install - run: npm run docs - uses: JustinBeckwith/linkinator-action@v1 From 981622985debc16e5177cf612f55608416cc23ac Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 14 May 2024 16:55:40 -0700 Subject: [PATCH 07/28] feat: Improve types, streamline, and standardize --- README.md | 40 ++--- browser-test/test.browser.ts | 4 +- package.json | 4 +- src/common.ts | 72 +++++++-- src/gaxios.ts | 138 ++++++++++-------- system-test/fixtures/sample/webpack.config.js | 1 - test/test.getch.ts | 39 +++-- webpack-tests.config.js | 1 - webpack.config.js | 1 - 9 files changed, 186 insertions(+), 114 deletions(-) diff --git a/README.md b/README.md index 19bc888c..f17175a6 100644 --- a/README.md +++ b/README.md @@ -53,16 +53,29 @@ interface GaxiosOptions = { baseURL: 'https://example.com'; // The HTTP methods to be sent with the request. - headers: { 'some': 'header' }, - - // The data to send in the body of the request. Data objects will be - // serialized as JSON, except for `FormData`. + headers: { 'some': 'header' } || new Headers(), + + // The data to send in the body of the request. Objects will be serialized as JSON + // except for: + // - `ArrayBuffer` + // - `Blob` + // - `Buffer` (Node.js) + // - `DataView` + // - `File` + // - `FormData` + // - `ReadableStream` + // - `stream.Readable` (Node.js) + // - strings + // - `TypedArray` (e.g. `Uint8Array`, `BigInt64Array`) + // - `URLSearchParams` + // - all other objects where: + // - headers['Content-Type'] === 'application/x-www-form-urlencoded' (serialized as `URLSearchParams`) // - // Note: if you would like to provide a Content-Type header other than - // application/json you you must provide a string or readable stream, rather - // than an object: - // data: JSON.stringify({some: 'data'}) - // data: fs.readFile('./some-data.jpeg') + // In all other cases, if you would like to prevent `application/json` as the + // default `Content-Type` header you must provide a string or readable stream + // rather than an object, e.g.: + // - data: JSON.stringify({some: 'data'}) + // - data: fs.readFile('./some-data.jpeg') data: { some: 'data' }, @@ -71,19 +84,12 @@ interface GaxiosOptions = { // Defaults to `0`, which is the same as unset. maxContentLength: 2000, - // The querystring parameters that will be encoded using `qs` and + // The query parameters that will be encoded using `URLSearchParams` and // appended to the url params: { querystring: 'parameters' }, - // By default, we use the `querystring` package in node core to serialize - // querystring parameters. You can override that and provide your - // own implementation. - paramsSerializer: (params) => { - return qs.stringify(params); - }, - // The timeout for the HTTP request in milliseconds. Defaults to 0. timeout: 1000, diff --git a/browser-test/test.browser.ts b/browser-test/test.browser.ts index 06d926e0..41b2ccb3 100644 --- a/browser-test/test.browser.ts +++ b/browser-test/test.browser.ts @@ -14,7 +14,6 @@ import assert from 'assert'; import {describe, it} from 'mocha'; import {request} from '../src/index'; -import * as uuid from 'uuid'; const port = 7172; // should match the port defined in `webserver.ts` describe('💻 browser tests', () => { @@ -53,7 +52,8 @@ describe('💻 browser tests', () => { body: 'hello world!', }, ]; - const boundary = uuid.v4(); + const boundary = + globalThis?.crypto.randomUUID() || (await import('crypto')).randomUUID(); const finale = `--${boundary}--`; headers['Content-Type'] = `multipart/related; boundary=${boundary}`; diff --git a/package.json b/package.json index 6d2e1961..bea41a60 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "@types/node": "^20.0.0", "@types/sinon": "^17.0.0", "@types/tmp": "0.2.6", - "@types/uuid": "^9.0.0", "assert": "^2.0.0", "browserify": "^17.0.0", "c8": "^8.0.0", @@ -85,7 +84,6 @@ "dependencies": { "extend": "^3.0.2", "https-proxy-agent": "^7.0.1", - "node-fetch": "^3.3.2", - "uuid": "^9.0.1" + "node-fetch": "^3.3.2" } } diff --git a/src/common.ts b/src/common.ts index cb584ecc..5235b9ec 100644 --- a/src/common.ts +++ b/src/common.ts @@ -178,19 +178,46 @@ export interface GaxiosOptions extends Omit { */ headers?: Headers; /** - * The data to send in the {@link RequestInit.body} of the request. Data objects will be - * serialized as JSON, except for `FormData`. + * The data to send in the {@link RequestInit.body} of the request. Objects will be + * serialized as JSON, except for: + * - `ArrayBuffer` + * - `Blob` + * - `Buffer` (Node.js) + * - `DataView` + * - `File` + * - `FormData` + * - `ReadableStream` + * - `stream.Readable` (Node.js) + * - strings + * - `TypedArray` (e.g. `Uint8Array`, `BigInt64Array`) + * - `URLSearchParams` + * - all other objects where: + * - headers['Content-Type'] === 'application/x-www-form-urlencoded' (serialized as `URLSearchParams`) * - * Note: if you would like to provide a Content-Type header other than - * application/json you you must provide a string or readable stream, rather - * than an object: + * In all other cases, if you would like to prevent `application/json` as the + * default `Content-Type` header you must provide a string or readable stream + * rather than an object, e.g.: * * ```ts * {data: JSON.stringify({some: 'data'})} * {data: fs.readFile('./some-data.jpeg')} * ``` */ - data?: any; + data?: + | ArrayBuffer + | Blob + | Buffer + | DataView + | File + | FormData + | ReadableStream + | Readable + | string + | ArrayBufferView + | {buffer: ArrayBufferLike} + | URLSearchParams + | {} + | BodyInit; /** * The maximum size of the http response `Content-Length` in bytes allowed. */ @@ -212,6 +239,9 @@ export interface GaxiosOptions extends Omit { */ multipart?: GaxiosMultipartOptions[]; params?: any; + /** + * @deprecated Use {@link URLSearchParams} instead and pass this directly to {@link GaxiosOptions.data `data`}. + */ paramsSerializer?: (params: {[index: string]: string | number}) => string; timeout?: number; /** @@ -427,8 +457,6 @@ export function defaultErrorRedactor(data: { headers.forEach((value, key) => { if (check(key)) headers.set(key, REDACT); }); - - // headers.g } // support `node-fetch` Headers and other third-parties @@ -439,26 +467,44 @@ export function defaultErrorRedactor(data: { } } - function redactString(obj: GaxiosOptions, key: keyof GaxiosOptions) { + function redactString( + obj: T, + key: keyof T + ) { if ( typeof obj === 'object' && obj !== null && typeof obj[key] === 'string' ) { - const text = obj[key]; + const text = obj[key] as string; if ( /grant_type=/i.test(text) || /assertion=/i.test(text) || /secret/i.test(text) ) { - obj[key] = REDACT; + (obj[key] as {}) = REDACT; } } } - function redactObject(obj: T) { - if (typeof obj === 'object' && obj !== null) { + function redactObject( + obj: T | null + ) { + if (!obj) { + return; + } else if ( + obj instanceof FormData || + obj instanceof URLSearchParams || + // support `node-fetch` FormData/URLSearchParams + ('forEach' in obj && 'set' in obj) + ) { + (obj as FormData | URLSearchParams).forEach((_, key) => { + if (['grant_type', 'assertion'].includes(key) || /secret/.test(key)) { + (obj as FormData | URLSearchParams).set(key, REDACT); + } + }); + } else { if ('grant_type' in obj) { obj['grant_type'] = REDACT; } diff --git a/src/gaxios.ts b/src/gaxios.ts index fff69932..62cf2812 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -14,7 +14,6 @@ import extend from 'extend'; import {Agent} from 'http'; import {Agent as HTTPSAgent} from 'https'; -import qs from 'querystring'; import {URL} from 'url'; import type nodeFetch from 'node-fetch' with {'resolution-mode': 'import'}; @@ -29,10 +28,12 @@ import { } from './common'; import {getRetryConfig} from './retry'; import {Readable} from 'stream'; -import {v4} from 'uuid'; /* eslint-disable @typescript-eslint/no-explicit-any */ +const randomUUID = async () => + globalThis.crypto?.randomUUID() || (await import('crypto')).randomUUID(); + export class Gaxios { protected agentCache = new Map< string | URL, @@ -125,7 +126,7 @@ export class Gaxios { if (opts.responseType === 'stream') { const response = []; - for await (const chunk of opts.data) { + for await (const chunk of opts.data as Readable) { response.push(chunk); } @@ -245,7 +246,7 @@ export class Gaxios { * @returns Prepared options, ready to make a request */ async #prepareRequest(options: GaxiosOptions): Promise { - const opts = extend(true, {}, this.defaults, options); + const opts: GaxiosOptions = extend(true, {}, this.defaults, options); if (!opts.url) { throw new Error('URL is required.'); } @@ -256,14 +257,27 @@ export class Gaxios { opts.url = baseUrl.toString() + opts.url; } - opts.paramsSerializer = opts.paramsSerializer || this.paramsSerializer; - if (opts.params && Object.keys(opts.params).length > 0) { - let additionalQueryParams = opts.paramsSerializer(opts.params); - if (additionalQueryParams.startsWith('?')) { - additionalQueryParams = additionalQueryParams.slice(1); + // don't modify the properties of a default or provided URL + opts.url = new URL(opts.url); + + if (opts.params) { + if (opts.paramsSerializer) { + let additionalQueryParams = opts.paramsSerializer(opts.params); + + if (additionalQueryParams.startsWith('?')) { + additionalQueryParams = additionalQueryParams.slice(1); + } + const prefix = opts.url.toString().includes('?') ? '&' : '?'; + opts.url = opts.url + prefix + additionalQueryParams; + } else { + const url = opts.url instanceof URL ? opts.url : new URL(opts.url); + + for (const [key, value] of new URLSearchParams(opts.params)) { + url.searchParams.append(key, value); + } + + opts.url = url; } - const prefix = opts.url.toString().includes('?') ? '&' : '?'; - opts.url = opts.url + prefix + additionalQueryParams; } if (typeof options.maxContentLength === 'number') { @@ -279,50 +293,25 @@ export class Gaxios { ? opts.headers : new Headers(opts.headers); - const isFormData = - opts?.data instanceof FormData || + const shouldDirectlyPassData = + typeof opts.data === 'string' || + opts.data instanceof ArrayBuffer || + opts.data instanceof Blob || + opts.data instanceof File || + opts.data instanceof FormData || + opts.data instanceof Readable || + opts.data instanceof ReadableStream || + opts.data instanceof String || + opts.data instanceof URLSearchParams || + ArrayBuffer.isView(opts.data) || // `Buffer` (Node.js), `DataView`, `TypedArray` /** - * @deprecated `node-fetch` or another third-party `FormData` instance - **/ - opts.data?.constructor?.name === 'FormData'; + * @deprecated `node-fetch` or another third-party's request types + */ + ['Blob', 'File', 'FormData'].includes(opts.data?.constructor?.name || ''); + + if (opts.multipart?.length) { + const boundary = await randomUUID(); - if (opts.multipart === undefined && opts.data) { - if ( - opts.data instanceof ReadableStream || - opts.data instanceof Readable - ) { - opts.body = opts.data as ReadableStream; - } else if ( - opts.data instanceof Blob || - ('Buffer' in globalThis && Buffer.isBuffer(opts.data)) - ) { - // Do not attempt to JSON.stringify() a Buffer: - opts.body = opts.data; - if (!preparedHeaders.has('content-type')) { - preparedHeaders.set('content-type', 'application/json'); - } - } else if (typeof opts.data === 'object' && !isFormData) { - if ( - preparedHeaders.get('Content-Type') === - 'application/x-www-form-urlencoded' - ) { - // If www-form-urlencoded content type has been set, but data is - // provided as an object, serialize the content - opts.body = opts.paramsSerializer(opts.data); - } else { - if (!preparedHeaders.has('content-type')) { - preparedHeaders.set('content-type', 'application/json'); - } - opts.body = JSON.stringify(opts.data); - } - } else { - opts.body = opts.data; - } - } else if (opts.multipart && opts.multipart.length > 0) { - // note: once the minimum version reaches Node 16, - // this can be replaced with randomUUID() function from crypto - // and the dependency on UUID removed - const boundary = v4(); preparedHeaders.set( 'content-type', `multipart/related; boundary=${boundary}` @@ -331,6 +320,38 @@ export class Gaxios { opts.body = Readable.from( this.getMultipartRequest(opts.multipart, boundary) ) as {} as ReadableStream; + } else if (shouldDirectlyPassData) { + opts.body = opts.data as BodyInit; + + /** + * Used for backwards-compatibility. + * + * @deprecated we shouldn't infer Buffers as JSON + */ + if ('Buffer' in globalThis && Buffer.isBuffer(opts.data)) { + if (!preparedHeaders.has('content-type')) { + preparedHeaders.set('content-type', 'application/json'); + } + } + } else if (typeof opts.data === 'object') { + if ( + preparedHeaders.get('Content-Type') === + 'application/x-www-form-urlencoded' + ) { + // If www-form-urlencoded content type has been set, but data is + // provided as an object, serialize the content + opts.body = opts.paramsSerializer + ? opts.paramsSerializer(opts.data as {}) + : new URLSearchParams(opts.data as {}); + } else { + if (!preparedHeaders.has('content-type')) { + preparedHeaders.set('content-type', 'application/json'); + } + + opts.body = JSON.stringify(opts.data); + } + } else if (opts.data) { + opts.body = opts.data as BodyInit; } opts.validateStatus = opts.validateStatus || this.validateStatus; @@ -346,11 +367,10 @@ export class Gaxios { process?.env?.https_proxy || process?.env?.HTTP_PROXY || process?.env?.http_proxy; - const urlMayUseProxy = this.#urlMayUseProxy(opts.url, opts.noProxy); if (opts.agent) { // don't do any of the following options - use the user-provided agent. - } else if (proxy && urlMayUseProxy) { + } else if (proxy && this.#urlMayUseProxy(opts.url, opts.noProxy)) { const HttpsProxyAgent = await Gaxios.#getProxyAgent(); if (this.agentCache.has(proxy)) { @@ -414,14 +434,6 @@ export class Gaxios { return status >= 200 && status < 300; } - /** - * Encode a set of key/value pars into a querystring format (?foo=bar&baz=boo) - * @param params key value pars to encode - */ - private paramsSerializer(params: {[index: string]: string | number}) { - return qs.stringify(params); - } - /** * Attempts to parse a response by looking at the Content-Type header. * @param {Response} response the HTTP response. diff --git a/system-test/fixtures/sample/webpack.config.js b/system-test/fixtures/sample/webpack.config.js index 1a65d5a4..f83630cc 100644 --- a/system-test/fixtures/sample/webpack.config.js +++ b/system-test/fixtures/sample/webpack.config.js @@ -32,7 +32,6 @@ module.exports = { buffer: 'browserify', process: false, os: false, - querystring: false, path: false, stream: 'stream-browserify', url: false, diff --git a/test/test.getch.ts b/test/test.getch.ts index 9590a10c..bf53dbf7 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -27,7 +27,6 @@ import { } from '../src'; import {GAXIOS_ERROR_SYMBOL, Headers} from '../src/common'; import {pkg} from '../src/util'; -import qs from 'querystring'; import fs from 'fs'; nock.disableNetConnect(); @@ -247,7 +246,7 @@ describe('🥁 configuration options', () => { const scope = nock(url).get(path).reply(200, {}); const res = await request(opts); assert.strictEqual(res.status, 200); - assert.strictEqual(res.config.url, url + path); + assert.strictEqual(res.config.url?.toString(), url + path); scope.done(); }); @@ -257,7 +256,7 @@ describe('🥁 configuration options', () => { const scope = nock(url).get(path).reply(200, {}); const res = await request(opts); assert.strictEqual(res.status, 200); - assert.strictEqual(res.config.url, url + path); + assert.strictEqual(res.config.url?.toString(), url + path); scope.done(); }); @@ -278,7 +277,10 @@ describe('🥁 configuration options', () => { const scope = nock(url).get(path).reply(200, {}); const res = await request(opts); assert.strictEqual(res.status, 200); - assert.strictEqual(res.config.url, url + qs); + assert.strictEqual( + res.config.url?.toString(), + new URL(url + qs).toString() + ); scope.done(); }); @@ -291,7 +293,7 @@ describe('🥁 configuration options', () => { const scope = nock(url).get(path).reply(200, {}); const res = await request(opts); assert.strictEqual(res.status, 200); - assert.strictEqual(res.config.url, url + path); + assert.strictEqual(res.config.url?.toString(), url + path); scope.done(); }); @@ -309,7 +311,7 @@ describe('🥁 configuration options', () => { const scope = nock(url).get(`/${qs}`).reply(200, {}); const res = await request(opts); assert.strictEqual(res.status, 200); - assert.strictEqual(res.config.url, url + qs); + assert.strictEqual(res.config.url, new URL(url + qs).toString()); scope.done(); }); @@ -692,10 +694,10 @@ describe('🎏 data handling', () => { it('should accept a string in the request data', async () => { const body = {hello: '🌎'}; - const encoded = qs.stringify(body); + const encoded = new URLSearchParams(body); const scope = nock(url) .matchHeader('content-type', 'application/x-www-form-urlencoded') - .post('/', encoded) + .post('/', encoded.toString()) .reply(200, {}); const res = await request({ url, @@ -744,7 +746,7 @@ describe('🎏 data handling', () => { const body = {hello: '🌎'}; const scope = nock(url) .matchHeader('Content-Type', 'application/x-www-form-urlencoded') - .post('/', qs.stringify(body)) + .post('/', new URLSearchParams(body).toString()) .reply(200, {}); const res = await request({ url, @@ -948,7 +950,7 @@ describe('🎏 data handling', () => { customURL.searchParams.append('client_secret', 'data'); customURL.searchParams.append('random', 'non-sensitive'); - const config: GaxiosOptions = { + const config = { headers: { Authentication: 'My Auth', /** @@ -965,7 +967,7 @@ describe('🎏 data handling', () => { client_secret: 'data', }, body: 'grant_type=somesensitivedata&assertion=somesensitivedata&client_secret=data', - }; + } as const; // simulate JSON response const responseHeaders = { @@ -1019,8 +1021,19 @@ describe('🎏 data handling', () => { client_secret: REDACT, }); - // config redactions - body - assert.deepStrictEqual(e.config.body, REDACT); + assert.deepStrictEqual( + Object.fromEntries(e.config.body as URLSearchParams), + { + ...config.data, // non-redactables should be present + grant_type: REDACT, + assertion: REDACT, + client_secret: REDACT, + } + ); + + expectedRequestHeaders.forEach((value, key) => { + assert.equal(actualHeaders.get(key), value); + }); // config redactions - url assert(e.config.url); diff --git a/webpack-tests.config.js b/webpack-tests.config.js index 2590ab03..e29599a2 100644 --- a/webpack-tests.config.js +++ b/webpack-tests.config.js @@ -32,7 +32,6 @@ module.exports = { buffer: 'browserify', process: false, os: false, - querystring: false, path: false, stream: 'stream-browserify', url: false, diff --git a/webpack.config.js b/webpack.config.js index bbb1b492..0621b6dd 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -32,7 +32,6 @@ module.exports = { buffer: 'browserify', process: false, os: false, - querystring: false, path: false, stream: 'stream-browserify', url: false, From 7315442e88af71b0fdec6f50555c1f54559dcd5e Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 14 May 2024 17:06:29 -0700 Subject: [PATCH 08/28] feat!: Do Not Treat Buffers as JSON By Default --- README.md | 5 +---- src/common.ts | 8 +------- src/gaxios.ts | 11 ----------- test/test.getch.ts | 16 ---------------- 4 files changed, 2 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index f17175a6..efceec6c 100644 --- a/README.md +++ b/README.md @@ -72,10 +72,7 @@ interface GaxiosOptions = { // - headers['Content-Type'] === 'application/x-www-form-urlencoded' (serialized as `URLSearchParams`) // // In all other cases, if you would like to prevent `application/json` as the - // default `Content-Type` header you must provide a string or readable stream - // rather than an object, e.g.: - // - data: JSON.stringify({some: 'data'}) - // - data: fs.readFile('./some-data.jpeg') + // default `Content-Type` header you must set the `Content-Type`. data: { some: 'data' }, diff --git a/src/common.ts b/src/common.ts index 5235b9ec..ae40389b 100644 --- a/src/common.ts +++ b/src/common.ts @@ -195,13 +195,7 @@ export interface GaxiosOptions extends Omit { * - headers['Content-Type'] === 'application/x-www-form-urlencoded' (serialized as `URLSearchParams`) * * In all other cases, if you would like to prevent `application/json` as the - * default `Content-Type` header you must provide a string or readable stream - * rather than an object, e.g.: - * - * ```ts - * {data: JSON.stringify({some: 'data'})} - * {data: fs.readFile('./some-data.jpeg')} - * ``` + * default `Content-Type` header you must set the `Content-Type`. */ data?: | ArrayBuffer diff --git a/src/gaxios.ts b/src/gaxios.ts index 62cf2812..923d1af3 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -322,17 +322,6 @@ export class Gaxios { ) as {} as ReadableStream; } else if (shouldDirectlyPassData) { opts.body = opts.data as BodyInit; - - /** - * Used for backwards-compatibility. - * - * @deprecated we shouldn't infer Buffers as JSON - */ - if ('Buffer' in globalThis && Buffer.isBuffer(opts.data)) { - if (!preparedHeaders.has('content-type')) { - preparedHeaders.set('content-type', 'application/json'); - } - } } else if (typeof opts.data === 'object') { if ( preparedHeaders.get('Content-Type') === diff --git a/test/test.getch.ts b/test/test.getch.ts index bf53dbf7..4d1d71b8 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -1102,22 +1102,6 @@ describe('🍂 defaults & instances', () => { assert.deepStrictEqual(res.data, {}); }); - it('should set content-type to application/json by default, for buffer', async () => { - const pkg = fs.readFileSync('./package.json'); - const pkgJson = JSON.parse(pkg.toString('utf8')); - const scope = nock(url) - .matchHeader('content-type', 'application/json') - .post('/', pkgJson) - .reply(200, {}); - const res = await request({ - url, - method: 'POST', - data: pkg, - }); - scope.done(); - assert.deepStrictEqual(res.data, {}); - }); - describe('mtls', () => { class GaxiosAssertAgentCache extends Gaxios { getAgentCache() { From 9164b87c110465bdf695f2f98561d4c7d93dad9e Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 15 May 2024 13:31:35 -0700 Subject: [PATCH 09/28] test: Adopt `Headers` type from merge --- test/test.getch.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index 0926d98e..01e3b7fb 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -1290,13 +1290,13 @@ describe('interceptors', () => { const instance = new Gaxios(); instance.interceptors.response.add({ resolved(response) { - response.headers['hello'] = 'world'; + response.headers.set('hello', 'world'); return Promise.resolve(response); }, }); const resp = await instance.request({url}); scope.done(); - assert.strictEqual(resp.headers['hello'], 'world'); + assert.strictEqual(resp.headers.get('hello'), 'world'); }); it('should not invoke a response interceptor after it is removed', async () => { @@ -1325,30 +1325,30 @@ describe('interceptors', () => { const instance = new Gaxios(); instance.interceptors.response.add({ resolved: response => { - response.headers!['foo'] = 'bar'; + response.headers.set('foo', 'bar'); return Promise.resolve(response); }, }); instance.interceptors.response.add({ resolved: response => { - assert.strictEqual(response.headers!['foo'], 'bar'); - response.headers!['bar'] = 'baz'; + assert.strictEqual(response.headers.get('foo'), 'bar'); + response.headers.set('bar', 'baz'); return Promise.resolve(response); }, }); instance.interceptors.response.add({ resolved: response => { - assert.strictEqual(response.headers!['foo'], 'bar'); - assert.strictEqual(response.headers!['bar'], 'baz'); - response.headers!['baz'] = 'buzz'; + assert.strictEqual(response.headers.get('foo'), 'bar'); + assert.strictEqual(response.headers.get('bar'), 'baz'); + response.headers.set('baz', 'buzz'); return Promise.resolve(response); }, }); const resp = await instance.request({url, headers: {}}); scope.done(); - assert.strictEqual(resp.headers['foo'], 'bar'); - assert.strictEqual(resp.headers['bar'], 'baz'); - assert.strictEqual(resp.headers['baz'], 'buzz'); + assert.strictEqual(resp.headers.get('foo'), 'bar'); + assert.strictEqual(resp.headers.get('bar'), 'baz'); + assert.strictEqual(resp.headers.get('baz'), 'buzz'); }); it('should not invoke a any response interceptors after they are removed', async () => { From 84faea8e65d0a80fdcc574390f27839d4be93133 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 16 May 2024 13:02:51 -0700 Subject: [PATCH 10/28] feat: Introduce `GaxiosOptionsPrepared` for improved type guarantees --- src/common.ts | 14 ++++++++++---- src/gaxios.ts | 34 ++++++++++++++-------------------- src/index.ts | 1 + src/interceptor.ts | 8 +++++--- test/test.getch.ts | 36 ++++++++++++++++++++---------------- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/src/common.ts b/src/common.ts index 5235b9ec..0eb2c4a9 100644 --- a/src/common.ts +++ b/src/common.ts @@ -128,7 +128,7 @@ export interface Headers { export type GaxiosPromise = Promise>; export interface GaxiosResponse extends Response { - config: GaxiosOptions; + config: GaxiosOptionsPrepared; data: T; } @@ -148,8 +148,8 @@ export interface GaxiosOptions extends Omit { * @deprecated Use {@link GaxiosOptions.fetchImplementation} instead. */ adapter?: ( - options: GaxiosOptions, - defaultAdapter: (options: GaxiosOptions) => GaxiosPromise + options: GaxiosOptionsPrepared, + defaultAdapter: (options: GaxiosOptionsPrepared) => GaxiosPromise ) => GaxiosPromise; url?: string | URL; /** @@ -330,13 +330,19 @@ export interface GaxiosOptions extends Omit { */ errorRedactor?: typeof defaultErrorRedactor | false; } + +export interface GaxiosOptionsPrepared extends GaxiosOptions { + headers: globalThis.Headers; + url: NonNullable; +} + /** * A partial object of `GaxiosOptions` with only redactable keys * * @experimental */ export type RedactableGaxiosOptions = Pick< - GaxiosOptions, + GaxiosOptions | GaxiosOptionsPrepared, 'body' | 'data' | 'headers' | 'url' >; /** diff --git a/src/gaxios.ts b/src/gaxios.ts index 8da69ddb..9e17a8dd 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -21,6 +21,7 @@ import { GaxiosMultipartOptions, GaxiosError, GaxiosOptions, + GaxiosOptionsPrepared, GaxiosPromise, GaxiosResponse, Headers, @@ -50,7 +51,7 @@ export class Gaxios { * Interceptors */ interceptors: { - request: GaxiosInterceptorManager; + request: GaxiosInterceptorManager; response: GaxiosInterceptorManager; }; @@ -77,7 +78,7 @@ export class Gaxios { } private async _defaultAdapter( - config: GaxiosOptions + config: GaxiosOptionsPrepared ): Promise> { const fetchImpl = config.fetchImplementation || @@ -89,7 +90,7 @@ export class Gaxios { const preparedOpts = {...config}; delete preparedOpts.data; - const res = (await fetchImpl(config.url!, preparedOpts as {})) as Response; + const res = (await fetchImpl(config.url, preparedOpts as {})) as Response; let data = await this.getResponseData(config, res); // `node-fetch`'s data isn't writable. Native `fetch`'s is. @@ -123,7 +124,7 @@ export class Gaxios { * @param opts Set of HTTP options that will be used for this HTTP request. */ protected async _request( - opts: GaxiosOptions = {} + opts: GaxiosOptionsPrepared ): GaxiosPromise { try { let translatedResponse: GaxiosResponse; @@ -175,7 +176,7 @@ export class Gaxios { } private async getResponseData( - opts: GaxiosOptions, + opts: GaxiosOptionsPrepared, res: Response ): Promise { if ( @@ -262,8 +263,8 @@ export class Gaxios { * @returns {Promise} Promise that resolves to the set of options or response after interceptors are applied. */ async #applyRequestInterceptors( - options: GaxiosOptions - ): Promise { + options: GaxiosOptionsPrepared + ): Promise { let promiseChain = Promise.resolve(options); for (const interceptor of this.interceptors.request.values()) { @@ -271,7 +272,7 @@ export class Gaxios { promiseChain = promiseChain.then( interceptor.resolved, interceptor.rejected - ) as Promise; + ) as Promise; } } @@ -309,7 +310,9 @@ export class Gaxios { * @param options The original options passed from the client. * @returns Prepared options, ready to make a request */ - async #prepareRequest(options: GaxiosOptions): Promise { + async #prepareRequest( + options: GaxiosOptions + ): Promise { const opts: GaxiosOptions = extend(true, {}, this.defaults, options); if (!opts.url) { throw new Error('URL is required.'); @@ -476,18 +479,9 @@ export class Gaxios { (opts as {duplex: string}).duplex = 'half'; } - // preserve the original type for auditing later - if (opts.headers instanceof Headers) { - opts.headers = preparedHeaders; - } else { - const headers: Headers = {}; - preparedHeaders.forEach((value, key) => { - headers[key] = value; - }); - opts.headers = headers; - } + opts.headers = preparedHeaders; - return opts; + return opts as GaxiosOptionsPrepared; } /** diff --git a/src/index.ts b/src/index.ts index a18ddef3..7ec77637 100644 --- a/src/index.ts +++ b/src/index.ts @@ -18,6 +18,7 @@ export { GaxiosError, GaxiosPromise, GaxiosResponse, + GaxiosOptionsPrepared as PreparedGaxiosOptions, Headers, RetryConfig, } from './common'; diff --git a/src/interceptor.ts b/src/interceptor.ts index d52aacbb..9ccfad86 100644 --- a/src/interceptor.ts +++ b/src/interceptor.ts @@ -11,12 +11,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {GaxiosError, GaxiosOptions, GaxiosResponse} from './common'; +import {GaxiosError, GaxiosOptionsPrepared, GaxiosResponse} from './common'; /** * Interceptors that can be run for requests or responses. These interceptors run asynchronously. */ -export interface GaxiosInterceptor { +export interface GaxiosInterceptor< + T extends GaxiosOptionsPrepared | GaxiosResponse, +> { /** * Function to be run when applying an interceptor. * @@ -37,5 +39,5 @@ export interface GaxiosInterceptor { * Class to manage collections of GaxiosInterceptors for both requests and responses. */ export class GaxiosInterceptorManager< - T extends GaxiosOptions | GaxiosResponse, + T extends GaxiosOptionsPrepared | GaxiosResponse, > extends Set | null> {} diff --git a/test/test.getch.ts b/test/test.getch.ts index 01e3b7fb..f9afcb38 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -25,7 +25,11 @@ import { GaxiosResponse, GaxiosPromise, } from '../src'; -import {GAXIOS_ERROR_SYMBOL, Headers} from '../src/common'; +import { + GAXIOS_ERROR_SYMBOL, + GaxiosOptionsPrepared, + Headers, +} from '../src/common'; import {pkg} from '../src/util'; import fs from 'fs'; @@ -155,8 +159,8 @@ describe('🥁 configuration options', () => { const inst = new Gaxios({headers: {apple: 'juice'}}); const res = await inst.request({url, headers: {figgy: 'pudding'}}); scope.done(); - assert.strictEqual(res.config.headers!.apple, 'juice'); - assert.strictEqual(res.config.headers!.figgy, 'pudding'); + assert.strictEqual(res.config.headers.get('apple'), 'juice'); + assert.strictEqual(res.config.headers.get('figgy'), 'pudding'); }); it('should allow setting a base url in the options', async () => { @@ -1125,7 +1129,7 @@ describe('🍂 defaults & instances', () => { } // eslint-disable-next-line @typescript-eslint/no-explicit-any protected async _request( - opts: GaxiosOptions = {} + opts: GaxiosOptionsPrepared ): GaxiosPromise { assert(opts.agent); return super._request(opts); @@ -1141,8 +1145,8 @@ describe('🍂 defaults & instances', () => { }); const res = await inst.request({url, headers: {figgy: 'pudding'}}); scope.done(); - assert.strictEqual(res.config.headers!.apple, 'juice'); - assert.strictEqual(res.config.headers!.figgy, 'pudding'); + assert.strictEqual(res.config.headers.get('apple'), 'juice'); + assert.strictEqual(res.config.headers.get('figgy'), 'pudding'); const agentCache = inst.getAgentCache(); assert(agentCache.get(key)); }); @@ -1173,7 +1177,7 @@ describe('interceptors', () => { const instance = new Gaxios(); instance.interceptors.request.add({ resolved: config => { - config.headers = {hello: 'world'}; + config.headers.set('hello', 'world'); return Promise.resolve(config); }, }); @@ -1190,7 +1194,7 @@ describe('interceptors', () => { validateStatus: () => { return true; }, - }) as unknown as Promise + }) as unknown as Promise ); const instance = new Gaxios(); const interceptor = {resolved: spyFunc}; @@ -1212,22 +1216,22 @@ describe('interceptors', () => { const instance = new Gaxios(); instance.interceptors.request.add({ resolved: config => { - config.headers!['foo'] = 'bar'; + config.headers.set('foo', 'bar'); return Promise.resolve(config); }, }); instance.interceptors.request.add({ resolved: config => { - assert.strictEqual(config.headers!['foo'], 'bar'); - config.headers!['bar'] = 'baz'; + assert.strictEqual(config.headers.get('foo'), 'bar'); + config.headers.set('bar', 'baz'); return Promise.resolve(config); }, }); instance.interceptors.request.add({ resolved: config => { - assert.strictEqual(config.headers!['foo'], 'bar'); - assert.strictEqual(config.headers!['bar'], 'baz'); - config.headers!['baz'] = 'buzz'; + assert.strictEqual(config.headers.get('foo'), 'bar'); + assert.strictEqual(config.headers.get('bar'), 'baz'); + config.headers.set('baz', 'buzz'); return Promise.resolve(config); }, }); @@ -1244,7 +1248,7 @@ describe('interceptors', () => { validateStatus: () => { return true; }, - }) as unknown as Promise + }) as unknown as Promise ); const instance = new Gaxios(); instance.interceptors.request.add({ @@ -1272,7 +1276,7 @@ describe('interceptors', () => { }); instance.interceptors.request.add({ resolved: config => { - config.headers = {hello: 'world'}; + config.headers.set('hello', 'world'); return Promise.resolve(config); }, rejected: err => { From 4e6de5f836c43ee582f6febd1a99d965fa29ad8a Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 16 May 2024 13:11:07 -0700 Subject: [PATCH 11/28] feat: Ensure `GaxiosOptionsPrepared.url` is always a `URL` --- src/common.ts | 2 +- src/gaxios.ts | 7 ++++--- test/test.getch.ts | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/common.ts b/src/common.ts index 0eb2c4a9..6d9f2216 100644 --- a/src/common.ts +++ b/src/common.ts @@ -333,7 +333,7 @@ export interface GaxiosOptions extends Omit { export interface GaxiosOptionsPrepared extends GaxiosOptions { headers: globalThis.Headers; - url: NonNullable; + url: URL; } /** diff --git a/src/gaxios.ts b/src/gaxios.ts index 9e17a8dd..d5832682 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -479,9 +479,10 @@ export class Gaxios { (opts as {duplex: string}).duplex = 'half'; } - opts.headers = preparedHeaders; - - return opts as GaxiosOptionsPrepared; + return Object.assign(opts, { + headers: preparedHeaders, + url: opts.url instanceof URL ? opts.url : new URL(opts.url), + }); } /** diff --git a/test/test.getch.ts b/test/test.getch.ts index f9afcb38..85b1ed3a 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -315,7 +315,7 @@ describe('🥁 configuration options', () => { const scope = nock(url).get(`/${qs}`).reply(200, {}); const res = await request(opts); assert.strictEqual(res.status, 200); - assert.strictEqual(res.config.url, new URL(url + qs).toString()); + assert.strictEqual(res.config.url.toString(), new URL(url + qs).toString()); scope.done(); }); From e6c2371c826753e365ab3f679d2f49467356f06d Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 16 May 2024 14:19:52 -0700 Subject: [PATCH 12/28] refactor: Clean-up Types & Resolve lint warnings --- src/common.ts | 113 +++++++++++++-------------------------------- src/gaxios.ts | 19 ++++---- src/index.ts | 3 +- test/test.getch.ts | 14 +++--- 4 files changed, 50 insertions(+), 99 deletions(-) diff --git a/src/common.ts b/src/common.ts index 6d9f2216..e6fd1442 100644 --- a/src/common.ts +++ b/src/common.ts @@ -77,7 +77,7 @@ export class GaxiosError extends Error { constructor( message: string, - public config: GaxiosOptions, + public config: GaxiosOptionsPrepared, public response?: GaxiosResponse, public error?: Error | NodeJS.ErrnoException ) { @@ -111,7 +111,7 @@ export class GaxiosError extends Error { } if (config.errorRedactor) { - config.errorRedactor({ + config.errorRedactor({ config: this.config, response: this.response, }); @@ -119,35 +119,33 @@ export class GaxiosError extends Error { } } -/** - * @deprecated use native {@link globalThis.Headers}. - */ -export interface Headers { - [index: string]: any; -} -export type GaxiosPromise = Promise>; +type GaxiosResponseData = + | ReturnType + | GaxiosOptionsPrepared['data']; + +export type GaxiosPromise = Promise>; -export interface GaxiosResponse extends Response { +export interface GaxiosResponse extends Response { config: GaxiosOptionsPrepared; data: T; } export interface GaxiosMultipartOptions { - headers: Headers; + headers: HeadersInit; content: string | Readable; } /** * Request options that are used to form the request. */ -export interface GaxiosOptions extends Omit { +export interface GaxiosOptions extends RequestInit { /** * Optional method to override making the actual HTTP request. Useful * for writing tests. * * @deprecated Use {@link GaxiosOptions.fetchImplementation} instead. */ - adapter?: ( + adapter?: ( options: GaxiosOptionsPrepared, defaultAdapter: (options: GaxiosOptionsPrepared) => GaxiosPromise ) => GaxiosPromise; @@ -167,16 +165,6 @@ export interface GaxiosOptions extends Omit { | 'OPTIONS' | 'TRACE' | 'PATCH'; - /** - * Recommended: Provide a native {@link globalThis.Headers Headers} object. - * - * @privateRemarks - * - * This type does not have the native {@link globalThis.Headers Headers} in - * its signature as it would break customers looking to modify headers before - * providing to this library (new, unnecessary type checks/guards). - */ - headers?: Headers; /** * The data to send in the {@link RequestInit.body} of the request. Objects will be * serialized as JSON, except for: @@ -238,7 +226,7 @@ export interface GaxiosOptions extends Omit { * This is passed to {@link RequestInit.body}. */ multipart?: GaxiosMultipartOptions[]; - params?: any; + params?: GaxiosResponseData; /** * @deprecated Use {@link URLSearchParams} instead and pass this directly to {@link GaxiosOptions.data `data`}. */ @@ -247,7 +235,7 @@ export interface GaxiosOptions extends Omit { /** * @deprecated ignored */ - onUploadProgress?: (progressEvent: any) => void; + onUploadProgress?: (progressEvent: GaxiosResponseData) => void; /** * If the `fetchImplementation` is native `fetch`, the * stream is a `ReadableStream`, otherwise `readable.Stream` @@ -336,25 +324,6 @@ export interface GaxiosOptionsPrepared extends GaxiosOptions { url: URL; } -/** - * A partial object of `GaxiosOptions` with only redactable keys - * - * @experimental - */ -export type RedactableGaxiosOptions = Pick< - GaxiosOptions | GaxiosOptionsPrepared, - 'body' | 'data' | 'headers' | 'url' ->; -/** - * A partial object of `GaxiosResponse` with only redactable keys - * - * @experimental - */ -export type RedactableGaxiosResponse = Pick< - GaxiosResponse, - 'config' | 'data' | 'headers' ->; - /** * Configuration for the Gaxios `request` method. */ @@ -409,7 +378,10 @@ export interface RetryConfig { retryBackoff?: (err: GaxiosError, defaultBackoffMs: number) => Promise; } -function translateData(responseType: string | undefined, data: any) { +function translateData( + responseType: string | undefined, + data: GaxiosResponseData +) { switch (responseType) { case 'stream': return data; @@ -432,51 +404,30 @@ function translateData(responseType: string | undefined, data: any) { * * @experimental */ -export function defaultErrorRedactor(data: { - config?: RedactableGaxiosOptions; - response?: RedactableGaxiosResponse; -}) { +export function defaultErrorRedactor< + O extends GaxiosOptionsPrepared, + R extends GaxiosResponse, +>(data: {config?: O; response?: R}) { const REDACT = '< - See `errorRedactor` option in `gaxios` for configuration>.'; - function redactHeaders(headers?: Headers | globalThis.Headers) { + function redactHeaders(headers?: Headers) { if (!headers) return; - function check(key: string) { + headers.forEach((_, key) => { // any casing of `Authentication` // any casing of `Authorization` // anything containing secret, such as 'client secret' - return ( + if ( /^authentication$/i.test(key) || /^authorization$/i.test(key) || /secret/i.test(key) - ); - } - - function redactHeadersObject(headers: Headers) { - for (const key of Object.keys(headers)) { - if (check(key)) headers[key] = REDACT; - } - } - - function redactHeadersHeaders(headers: globalThis.Headers) { - headers.forEach((value, key) => { - if (check(key)) headers.set(key, REDACT); - }); - } - - // support `node-fetch` Headers and other third-parties - if (headers instanceof Headers || 'set' in headers) { - redactHeadersHeaders(headers as globalThis.Headers); - } else { - redactHeadersObject(headers); - } + ) + headers.set(key, REDACT); + }); } - function redactString( - obj: T, - key: keyof T - ) { + function redactString(obj: T, key: keyof T) { if ( typeof obj === 'object' && obj !== null && @@ -494,9 +445,7 @@ export function defaultErrorRedactor(data: { } } - function redactObject( - obj: T | null - ) { + function redactObject(obj: T | null) { if (!obj) { return; } else if ( @@ -535,7 +484,7 @@ export function defaultErrorRedactor(data: { redactObject(data.config.body); try { - const url = new URL('', data.config.url); + const url = data.config.url; if (url.searchParams.has('token')) { url.searchParams.set('token', REDACT); @@ -545,7 +494,7 @@ export function defaultErrorRedactor(data: { url.searchParams.set('client_secret', REDACT); } - data.config.url = url.toString(); + data.config.url = url; } catch { // ignore error - no need to parse an invalid URL } diff --git a/src/gaxios.ts b/src/gaxios.ts index d5832682..9b59e8e5 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -24,7 +24,6 @@ import { GaxiosOptionsPrepared, GaxiosPromise, GaxiosResponse, - Headers, defaultErrorRedactor, } from './common'; import {getRetryConfig} from './retry'; @@ -210,7 +209,7 @@ export class Gaxios { #urlMayUseProxy( url: string | URL, - noProxy: GaxiosOptions['noProxy'] = [] + noProxy: GaxiosOptionsPrepared['noProxy'] = [] ): boolean { const candidate = new URL(url); const noProxyList = [...noProxy]; @@ -258,9 +257,9 @@ export class Gaxios { * Applies the request interceptors. The request interceptors are applied after the * call to prepareRequest is completed. * - * @param {GaxiosOptions} options The current set of options. + * @param {GaxiosOptionsPrepared} options The current set of options. * - * @returns {Promise} Promise that resolves to the set of options or response after interceptors are applied. + * @returns {Promise} Promise that resolves to the set of options or response after interceptors are applied. */ async #applyRequestInterceptors( options: GaxiosOptionsPrepared @@ -283,9 +282,9 @@ export class Gaxios { * Applies the response interceptors. The response interceptors are applied after the * call to request is made. * - * @param {GaxiosOptions} options The current set of options. + * @param {GaxiosOptionsPrepared} options The current set of options. * - * @returns {Promise} Promise that resolves to the set of options or response after interceptors are applied. + * @returns {Promise} Promise that resolves to the set of options or response after interceptors are applied. */ async #applyResponseInterceptors( response: GaxiosResponse | Promise @@ -313,7 +312,7 @@ export class Gaxios { async #prepareRequest( options: GaxiosOptions ): Promise { - const opts: GaxiosOptions = extend(true, {}, this.defaults, options); + const opts = extend(true, {}, this.defaults, options); if (!opts.url) { throw new Error('URL is required.'); } @@ -537,8 +536,12 @@ export class Gaxios { ) { const finale = `--${boundary}--`; for (const currentPart of multipartOptions) { + const headers = + currentPart.headers instanceof Headers + ? currentPart.headers + : new Headers(currentPart.headers); const partContentType = - currentPart.headers['Content-Type'] || 'application/octet-stream'; + headers.get('Content-Type') || 'application/octet-stream'; const preamble = `--${boundary}\r\nContent-Type: ${partContentType}\r\n\r\n`; yield preamble; if (typeof currentPart.content === 'string') { diff --git a/src/index.ts b/src/index.ts index 7ec77637..c563eac6 100644 --- a/src/index.ts +++ b/src/index.ts @@ -18,8 +18,7 @@ export { GaxiosError, GaxiosPromise, GaxiosResponse, - GaxiosOptionsPrepared as PreparedGaxiosOptions, - Headers, + GaxiosOptionsPrepared, RetryConfig, } from './common'; export {Gaxios, GaxiosOptions}; diff --git a/test/test.getch.ts b/test/test.getch.ts index 85b1ed3a..442cf95e 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -25,11 +25,7 @@ import { GaxiosResponse, GaxiosPromise, } from '../src'; -import { - GAXIOS_ERROR_SYMBOL, - GaxiosOptionsPrepared, - Headers, -} from '../src/common'; +import {GAXIOS_ERROR_SYMBOL, GaxiosOptionsPrepared} from '../src/common'; import {pkg} from '../src/util'; import fs from 'fs'; @@ -119,7 +115,11 @@ describe('🚙 error handling', () => { bodyUsed: true, } as GaxiosResponse; - const error = new GaxiosError('translation test', {}, response); + const error = new GaxiosError( + 'translation test', + {} as GaxiosOptionsPrepared, + response + ); assert(error.response); assert.equal(error.response.data, notJSON); @@ -130,7 +130,7 @@ describe('🚙 error handling', () => { const wrongVersion = {[GAXIOS_ERROR_SYMBOL]: '0.0.0'}; const correctVersion = {[GAXIOS_ERROR_SYMBOL]: pkg.version}; - const child = new A('', {}); + const child = new A('', {} as GaxiosOptionsPrepared); assert.equal(wrongVersion instanceof GaxiosError, false); assert.equal(correctVersion instanceof GaxiosError, true); From 01ac8c7153c98c78c63c271e3a656d2e25d6ff2c Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 16 May 2024 14:59:09 -0700 Subject: [PATCH 13/28] chore: merge conflicts --- src/gaxios.ts | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/gaxios.ts b/src/gaxios.ts index e857d1b5..b57b4c76 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -295,22 +295,16 @@ export class Gaxios { opts.url = new URL(opts.url); if (opts.params) { - if (opts.paramsSerializer) { - let additionalQueryParams = opts.paramsSerializer(opts.params); + const url = opts.url instanceof URL ? opts.url : new URL(opts.url); - if (additionalQueryParams.startsWith('?')) { - additionalQueryParams = additionalQueryParams.slice(1); - } - const prefix = opts.url.toString().includes('?') ? '&' : '?'; - opts.url = opts.url + prefix + additionalQueryParams; - } else { - const url = opts.url instanceof URL ? opts.url : new URL(opts.url); - - for (const [key, value] of new URLSearchParams(opts.params)) { - url.searchParams.append(key, value); - } + for (const [key, value] of new URLSearchParams(opts.params)) { + url.searchParams.append(key, value); + } opts.url = url; + opts.url = url; + } + opts.url = url; } } @@ -362,9 +356,7 @@ export class Gaxios { ) { // If www-form-urlencoded content type has been set, but data is // provided as an object, serialize the content - opts.body = opts.paramsSerializer - ? opts.paramsSerializer(opts.data as {}) - : new URLSearchParams(opts.data as {}); + opts.body = new URLSearchParams(opts.data as {}); } else { if (!preparedHeaders.has('content-type')) { preparedHeaders.set('content-type', 'application/json'); From 10d557a94adc62631c3a21b7491947e0bf3be3a9 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 16 May 2024 15:22:57 -0700 Subject: [PATCH 14/28] feat: Support `request(URL)` --- src/gaxios.ts | 25 +++++++++++++++++-------- src/index.ts | 8 ++++---- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/gaxios.ts b/src/gaxios.ts index b57b4c76..f78cbedf 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -69,10 +69,23 @@ export class Gaxios { * Perform an HTTP request with the given options. * @param opts Set of HTTP options that will be used for this HTTP request. */ - async request(opts: GaxiosOptions = {}): GaxiosPromise { - let prepared = await this.#prepareRequest(opts); - prepared = await this.#applyRequestInterceptors(prepared); - return this.#applyResponseInterceptors(this._request(prepared)); + async request( + options: GaxiosOptions | URL | string = {} + ): GaxiosPromise { + const opts = + options instanceof URL || typeof options === 'string' + ? {url: options} + : options; + + const prepared = await this.#prepareRequest(opts); + const preparedWithInterceptors = + await this.#applyRequestInterceptors(prepared); + + const pendingResponse = this._request(preparedWithInterceptors); + const responseWithInterceptors = + this.#applyResponseInterceptors(pendingResponse); + + return responseWithInterceptors; } private async _defaultAdapter( @@ -301,11 +314,7 @@ export class Gaxios { url.searchParams.append(key, value); } - opts.url = url; - opts.url = url; - } opts.url = url; - } } const preparedHeaders = diff --git a/src/index.ts b/src/index.ts index c563eac6..f63cf917 100644 --- a/src/index.ts +++ b/src/index.ts @@ -11,17 +11,17 @@ // See the License for the specific language governing permissions and // limitations under the License. -import {GaxiosOptions} from './common'; import {Gaxios} from './gaxios'; export { GaxiosError, GaxiosPromise, GaxiosResponse, + GaxiosOptions, GaxiosOptionsPrepared, RetryConfig, } from './common'; -export {Gaxios, GaxiosOptions}; +export {Gaxios}; export * from './interceptor'; /** @@ -34,6 +34,6 @@ export const instance = new Gaxios(); * Make an HTTP request using the given options. * @param opts Options for the request */ -export async function request(opts: GaxiosOptions) { - return instance.request(opts); +export async function request(...opts: Parameters) { + return instance.request(...opts); } From 7138ccb89a5bd622887ee321b967a864cdec3e85 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Fri, 17 May 2024 13:45:43 -0700 Subject: [PATCH 15/28] chore: more merge resolutions --- src/common.ts | 16 +++++----------- test/test.getch.ts | 22 ++++++++++++++-------- test/test.retry.ts | 13 ++++++++----- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/common.ts b/src/common.ts index 9db49887..e55c0d54 100644 --- a/src/common.ts +++ b/src/common.ts @@ -94,8 +94,7 @@ export class GaxiosError extends Error { try { this.response.data = translateData( this.config.responseType, - // workaround for `node-fetch`'s `.data` deprecation... - this.response?.bodyUsed ? this.response?.data : undefined + this.response?.data ); } catch { // best effort - don't throw an error within an error @@ -424,17 +423,12 @@ export function defaultErrorRedactor< } function redactObject(obj: T | null) { - if (!obj) { + if (!obj || typeof obj !== 'object') { return; - } else if ( - obj instanceof FormData || - obj instanceof URLSearchParams || - // support `node-fetch` FormData/URLSearchParams - ('forEach' in obj && 'set' in obj) - ) { - (obj as FormData | URLSearchParams).forEach((_, key) => { + } else if (obj instanceof FormData || obj instanceof URLSearchParams) { + obj.forEach((_, key) => { if (['grant_type', 'assertion'].includes(key) || /secret/.test(key)) { - (obj as FormData | URLSearchParams).set(key, REDACT); + obj.set(key, REDACT); } }); } else { diff --git a/test/test.getch.ts b/test/test.getch.ts index 02032042..0f90593c 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -120,7 +120,7 @@ describe('🚙 error handling', () => { ); assert(error.response); - assert.equal(error.response.data, notJSON); + assert.equal(error.response.data, '.'); }); it('should support `instanceof` for GaxiosErrors of the same version', () => { @@ -191,12 +191,17 @@ describe('🥁 configuration options', () => { it('should support redirects by default', async () => { const body = {hello: '🌎'}; - const scopes = [ - nock(url).get('/foo').reply(200, body), - nock(url).get('/').reply(302, undefined, {location: '/foo'}), - ]; - const res = await request({url}); - scopes.forEach(x => x.done()); + const url = new URL('https://example.com/foo/'); + + nock.enableNetConnect(); + const scope = nock(url.origin) + .get('/foo/') + .reply(302, undefined, {location: '/redirect/'}) + .get('/redirect/') + .reply(200, body); + + const res = await request(url); + scope.done(); assert.deepStrictEqual(res.data, body); assert.strictEqual(res.url, `${url}/foo`); }); @@ -317,7 +322,8 @@ describe('🥁 configuration options', () => { assert.deepStrictEqual(res.data, {}); }); - describe('proxying', () => { + // TODO: Should update with `fetch` compatible proxy agent first + describe.skip('proxying', () => { const url = 'https://domain.example.com/with-path'; const proxy = 'https://fake.proxy/'; let gaxios: Gaxios; diff --git a/test/test.retry.ts b/test/test.retry.ts index e2801d69..51b17fa7 100644 --- a/test/test.retry.ts +++ b/test/test.retry.ts @@ -93,9 +93,12 @@ describe('🛸 retry & exponential backoff', () => { it('should not retry if user aborted request', async () => { const ac = new AbortController(); + + // Note, no redirect target as it shouldn't be reached + nock(url).get('/').reply(302, undefined, {location: '/foo'}); + const config: GaxiosOptions = { - method: 'GET', - url: 'https://google.com', + url, signal: ac.signal, retryConfig: {retry: 10, noResponseRetries: 10}, }; @@ -104,10 +107,10 @@ describe('🛸 retry & exponential backoff', () => { try { await req; throw Error('unreachable'); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (err: any) { + } catch (err) { + assert(err instanceof GaxiosError); assert(err.config); - assert.strictEqual(err.config.retryConfig.currentRetryAttempt, 0); + assert.strictEqual(err.config.retryConfig?.currentRetryAttempt, 0); } }); From 98f7009ad1c48dc6a3b4dcd0696c42cce5c53bd0 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Mon, 20 May 2024 15:27:50 -0700 Subject: [PATCH 16/28] refactor: streamline `.data` for `Response` --- src/gaxios.ts | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/src/gaxios.ts b/src/gaxios.ts index 9b59e8e5..9afb591b 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -90,32 +90,22 @@ export class Gaxios { delete preparedOpts.data; const res = (await fetchImpl(config.url, preparedOpts as {})) as Response; - let data = await this.getResponseData(config, res); + const data = await this.getResponseData(config, res); // `node-fetch`'s data isn't writable. Native `fetch`'s is. - if (Object.getOwnPropertyDescriptor(res, 'data')?.configurable) { - // Keep `Response` as a class - return Object.assign(res, {config, data}); - } else { - Object.assign(res, {config}); - - // best effort for `node-fetch`; `.data` is not writable... - return new Proxy(res, { - get: (target, prop, receiver) => { - if (prop === 'data') return data; - - return Reflect.get(target, prop, receiver); + if (!Object.getOwnPropertyDescriptor(res, 'data')?.configurable) { + Object.defineProperties(res, { + data: { + configurable: true, + writable: true, + enumerable: true, + value: data, }, - set(target, prop, newValue, receiver) { - if (prop === 'data') { - data = newValue; - return true; - } else { - return Reflect.set(target, prop, newValue, receiver); - } - }, - }) as GaxiosResponse; + }); } + + // Keep object as an instance of `Response` + return Object.assign(res, {config, data}); } /** From 4894e39f95deee1792baf128ad64f1feb8b82103 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 22 May 2024 14:11:17 -0700 Subject: [PATCH 17/28] docs: Simplify example --- README.md | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/README.md b/README.md index f17175a6..d49000bd 100644 --- a/README.md +++ b/README.md @@ -16,9 +16,7 @@ $ npm install gaxios ```js const {request} = require('gaxios'); -const res = await request({ - url: 'https://www.googleapis.com/discovery/v1/apis/', -}); +const res = await request('https://www.googleapis.com/discovery/v1/apis/'); ``` ## Setting Defaults From 9a070ef1af75b243cdf5ee91d7d9f9531e7fa3e9 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 22 May 2024 14:12:51 -0700 Subject: [PATCH 18/28] chore: Remove `node-fetch` patch --- src/common.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/common.ts b/src/common.ts index e55c0d54..3b0015aa 100644 --- a/src/common.ts +++ b/src/common.ts @@ -475,12 +475,8 @@ export function defaultErrorRedactor< if (data.response) { defaultErrorRedactor({config: data.response.config}); redactHeaders(data.response.headers); - - // workaround for `node-fetch`'s `.data` deprecation... - if ((data.response as {} as Response).bodyUsed) { - redactString(data.response, 'data'); - redactObject(data.response.data); - } + redactString(data.response, 'data'); + redactObject(data.response.data); } return data; From b030c81c8560e6345da22ea7c40a7591ac73aea4 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 22 May 2024 14:13:16 -0700 Subject: [PATCH 19/28] chore: Update summary --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 98046371..76b6dc0f 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,7 @@ [![codecov](https://codecov.io/gh/googleapis/gaxios/branch/master/graph/badge.svg)](https://codecov.io/gh/googleapis/gaxios) [![Code Style: Google](https://img.shields.io/badge/code%20style-google-blueviolet.svg)](https://github.com/google/gts) -> An HTTP request client that provides an `axios` like interface over top of `fetch`. +> An HTTP request client that provides a lightweight interface on top of `fetch`. ## Install From a7e2e831ef2705fda67c79dc27b87f6c429b3367 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 22 May 2024 14:34:46 -0700 Subject: [PATCH 20/28] refactor: streamline, no error case --- src/common.ts | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/common.ts b/src/common.ts index e6fd1442..4af4f01b 100644 --- a/src/common.ts +++ b/src/common.ts @@ -483,20 +483,12 @@ export function defaultErrorRedactor< redactString(data.config, 'body'); redactObject(data.config.body); - try { - const url = data.config.url; - - if (url.searchParams.has('token')) { - url.searchParams.set('token', REDACT); - } - - if (url.searchParams.has('client_secret')) { - url.searchParams.set('client_secret', REDACT); - } + if (data.config.url.searchParams.has('token')) { + data.config.url.searchParams.set('token', REDACT); + } - data.config.url = url; - } catch { - // ignore error - no need to parse an invalid URL + if (data.config.url.searchParams.has('client_secret')) { + data.config.url.searchParams.set('client_secret', REDACT); } } From b2ec3953628e49fec3581fa38012e5666f6d3610 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Mon, 1 Jul 2024 14:22:16 -0700 Subject: [PATCH 21/28] docs: clarification --- README.md | 2 +- src/common.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index efceec6c..354df74a 100644 --- a/README.md +++ b/README.md @@ -72,7 +72,7 @@ interface GaxiosOptions = { // - headers['Content-Type'] === 'application/x-www-form-urlencoded' (serialized as `URLSearchParams`) // // In all other cases, if you would like to prevent `application/json` as the - // default `Content-Type` header you must set the `Content-Type`. + // default you must set the `Content-Type` header. data: { some: 'data' }, diff --git a/src/common.ts b/src/common.ts index bce66e96..eb3c2b5b 100644 --- a/src/common.ts +++ b/src/common.ts @@ -183,7 +183,7 @@ export interface GaxiosOptions extends RequestInit { * - headers['Content-Type'] === 'application/x-www-form-urlencoded' (serialized as `URLSearchParams`) * * In all other cases, if you would like to prevent `application/json` as the - * default `Content-Type` header you must set the `Content-Type`. + * default you must set the `Content-Type` header. */ data?: | ArrayBuffer From 8413f6848fa14f9238717b8a2c7a17cbc1a57299 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 11 Sep 2024 13:36:18 -0700 Subject: [PATCH 22/28] feat: Basic `GET` Support for `Request` objects `node-fetch` does not yet support webstreams, which is required for `.body` https://github.com/node-fetch/node-fetch/issues/387 --- src/common.ts | 12 +----------- src/gaxios.ts | 1 - src/retry.ts | 5 +++-- test/test.getch.ts | 13 ++++++++++--- 4 files changed, 14 insertions(+), 17 deletions(-) diff --git a/src/common.ts b/src/common.ts index 4af4f01b..ca2cfd98 100644 --- a/src/common.ts +++ b/src/common.ts @@ -155,16 +155,6 @@ export interface GaxiosOptions extends RequestInit { */ baseUrl?: string; baseURL?: string | URL; - method?: - | 'GET' - | 'HEAD' - | 'POST' - | 'DELETE' - | 'PUT' - | 'CONNECT' - | 'OPTIONS' - | 'TRACE' - | 'PATCH'; /** * The data to send in the {@link RequestInit.body} of the request. Objects will be * serialized as JSON, except for: @@ -325,7 +315,7 @@ export interface GaxiosOptionsPrepared extends GaxiosOptions { } /** - * Configuration for the Gaxios `request` method. + * Gaxios retry configuration. */ export interface RetryConfig { /** diff --git a/src/gaxios.ts b/src/gaxios.ts index 9afb591b..a471c74d 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -415,7 +415,6 @@ export class Gaxios { if (!preparedHeaders.has('accept') && opts.responseType === 'json') { preparedHeaders.set('accept', 'application/json'); } - opts.method = opts.method || 'GET'; const proxy = opts.proxy || diff --git a/src/retry.ts b/src/retry.ts index 9608d1b5..f0d42e94 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -118,8 +118,9 @@ function shouldRetryRequest(err: GaxiosError) { // Only retry with configured HttpMethods. if ( - !err.config.method || - config.httpMethodsToRetry!.indexOf(err.config.method.toUpperCase()) < 0 + config.httpMethodsToRetry!.indexOf( + err.config.method?.toUpperCase() || 'GET' + ) < 0 ) { return false; } diff --git a/test/test.getch.ts b/test/test.getch.ts index 442cf95e..9254735d 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -139,11 +139,18 @@ describe('🚙 error handling', () => { }); describe('🥁 configuration options', () => { - it('should accept URL objects', async () => { + it('should accept `URL` objects', async () => { const scope = nock(url).get('/').reply(204); const res = await request({url: new URL(url)}); scope.done(); - assert.strictEqual(res.config.method, 'GET'); + assert.strictEqual(res.status, 204); + }); + + it('should accept `Request` objects', async () => { + const scope = nock(url).get('/').reply(204); + const res = await request(new Request(url)); + scope.done(); + assert.strictEqual(res.status, 204); }); it('should use options passed into the constructor', async () => { @@ -686,7 +693,7 @@ describe('🥁 configuration options', () => { }); describe('🎏 data handling', () => { - it('should accpet a ReadableStream as request data', async () => { + it.only('should accpet a ReadableStream as request data', async () => { const body = fs.createReadStream('package.json'); // eslint-disable-next-line @typescript-eslint/no-var-requires const contents = require('../../package.json'); From e3220b9a1ef64a346ab89c3676e03985131c0660 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 11 Sep 2024 13:41:31 -0700 Subject: [PATCH 23/28] chore: Improve `Request` Support --- test/test.getch.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index a753a17b..1a1fd360 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -145,8 +145,15 @@ describe('🥁 configuration options', () => { }); it('should accept `Request` objects', async () => { - const scope = nock(url).get('/').reply(204); - const res = await request(new Request(url)); + const body = 'abc'; + const scope = nock(url).post('/', body).reply(204); + const res = await request( + new Request(url, { + method: 'POST', + headers: {'content-type': 'application/json'}, + body, + }) + ); scope.done(); assert.strictEqual(res.status, 204); }); @@ -670,7 +677,7 @@ describe('🥁 configuration options', () => { }); describe('🎏 data handling', () => { - it.only('should accpet a ReadableStream as request data', async () => { + it('should accept a ReadableStream as request data', async () => { const body = fs.createReadStream('package.json'); // eslint-disable-next-line @typescript-eslint/no-var-requires const contents = require('../../package.json'); From 04e264dfe7f11d7cae81a47ae767a23cba620922 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 11 Sep 2024 13:42:02 -0700 Subject: [PATCH 24/28] test: remove `.only` --- test/test.getch.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index 9254735d..7e270c81 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -693,7 +693,7 @@ describe('🥁 configuration options', () => { }); describe('🎏 data handling', () => { - it.only('should accpet a ReadableStream as request data', async () => { + it('should accpet a ReadableStream as request data', async () => { const body = fs.createReadStream('package.json'); // eslint-disable-next-line @typescript-eslint/no-var-requires const contents = require('../../package.json'); From d163a907736e08754d7a1b3c53bd3661dd3a7d50 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 11 Sep 2024 14:00:03 -0700 Subject: [PATCH 25/28] refactor: simplify `httpMethodsToRetry` --- src/retry.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/retry.ts b/src/retry.ts index f0d42e94..f31c47db 100644 --- a/src/retry.ts +++ b/src/retry.ts @@ -118,9 +118,10 @@ function shouldRetryRequest(err: GaxiosError) { // Only retry with configured HttpMethods. if ( - config.httpMethodsToRetry!.indexOf( + !config.httpMethodsToRetry || + !config.httpMethodsToRetry.includes( err.config.method?.toUpperCase() || 'GET' - ) < 0 + ) ) { return false; } From 705fad3c57a6c47941cb783a6aaccdf06a7a627f Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 11 Sep 2024 14:30:59 -0700 Subject: [PATCH 26/28] chore: update `nock` --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 55e40f06..132b0126 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,7 @@ "multiparty": "^4.2.1", "mv": "^2.1.1", "ncp": "^2.0.0", - "nock": "^14.0.0-beta.5", + "nock": "^14.0.0-beta.13", "null-loader": "^4.0.0", "puppeteer": "^21.0.0", "sinon": "^18.0.0", From 9e5275530d141420be44a0f70f85c6261ba02bb2 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 11 Sep 2024 14:31:17 -0700 Subject: [PATCH 27/28] test: cleanup --- test/test.getch.ts | 3 +-- test/test.retry.ts | 15 ++++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/test/test.getch.ts b/test/test.getch.ts index 7e270c81..93426f29 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -1012,13 +1012,12 @@ describe('🎏 data handling', () => { assert.notStrictEqual(e.config, config); // config redactions - headers - assert(e.config.headers); const expectedRequestHeaders = new Headers({ ...config.headers, // non-redactables should be present Authentication: REDACT, AUTHORIZATION: REDACT, }); - const actualHeaders = new Headers(e.config.headers); + const actualHeaders = e.config.headers; expectedRequestHeaders.forEach((value, key) => { assert.equal(actualHeaders.get(key), value); diff --git a/test/test.retry.ts b/test/test.retry.ts index e2801d69..1c0d86d6 100644 --- a/test/test.retry.ts +++ b/test/test.retry.ts @@ -248,7 +248,7 @@ describe('🛸 retry & exponential backoff', () => { it('should retry on ENOTFOUND', async () => { const body = {spicy: '🌮'}; const scopes = [ - nock(url).get('/').replyWithError({code: 'ENOTFOUND'}), + nock(url).get('/').reply(500, {code: 'ENOTFOUND'}), nock(url).get('/').reply(200, body), ]; const res = await request({url, retry: true}); @@ -259,7 +259,7 @@ describe('🛸 retry & exponential backoff', () => { it('should retry on ETIMEDOUT', async () => { const body = {sizzling: '🥓'}; const scopes = [ - nock(url).get('/').replyWithError({code: 'ETIMEDOUT'}), + nock(url).get('/').reply(500, {code: 'ETIMEDOUT'}), nock(url).get('/').reply(200, body), ]; const res = await request({url, retry: true}); @@ -268,13 +268,14 @@ describe('🛸 retry & exponential backoff', () => { }); it('should allow configuring noResponseRetries', async () => { - const scope = nock(url).get('/').replyWithError({code: 'ETIMEDOUT'}); + // `nock` is not listening, therefore it should fail const config = {url, retryConfig: {noResponseRetries: 0}}; - await assert.rejects(request(config), (e: Error) => { - const cfg = getConfig(e); - return cfg!.currentRetryAttempt === 0; + await assert.rejects(request(config), (e: GaxiosError) => { + return ( + e.code === 'ENETUNREACH' && + e.config.retryConfig?.currentRetryAttempt === 0 + ); }); - scope.done(); }); it('should delay the initial retry by 100ms by default', async () => { From 50321d65b300cf02e0bd99da6f9cba9c6046796c Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 11 Sep 2024 15:11:32 -0700 Subject: [PATCH 28/28] docs: Document `Request` and `RequestInit` --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 4eca0429..69ce29b1 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,9 @@ over other authentication methods, i.e., application default credentials. ## Request Options +`GaxiosOptions` extends from [`RequestInit`](https://developer.mozilla.org/en-US/docs/Web/API/RequestInit). +`Gaxios#request` also accepts [`Request`](https://developer.mozilla.org/en-US/docs/Web/API/Request) objects as well. + ```ts interface GaxiosOptions = { // The url to which the request should be sent. Required.