From 58c7dc2785277ebf32bbc5bb99f0d5c5897e839c Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 16 Apr 2024 15:18:42 -0700 Subject: [PATCH] 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",