Skip to content

Commit

Permalink
feat!: Upgrade to node-fetch v3
Browse files Browse the repository at this point in the history
  • Loading branch information
danielbankhead committed Apr 16, 2024
1 parent 9331f79 commit 58c7dc2
Show file tree
Hide file tree
Showing 8 changed files with 107 additions and 69 deletions.
5 changes: 2 additions & 3 deletions README.md
Expand Up @@ -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
Expand Down Expand Up @@ -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

/**
Expand Down
5 changes: 1 addition & 4 deletions package.json
Expand Up @@ -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",
Expand Down Expand Up @@ -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"
}
}
5 changes: 2 additions & 3 deletions src/common.ts
Expand Up @@ -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,
Expand Down
106 changes: 59 additions & 47 deletions src/gaxios.ts
Expand Up @@ -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,
Expand All @@ -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';
}
Expand Down Expand Up @@ -94,8 +85,14 @@ export class Gaxios {
private async _defaultAdapter<T>(
opts: GaxiosOptions
): Promise<GaxiosResponse<T>> {
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<T>(opts, res, data);
}
Expand All @@ -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;
}
Expand Down Expand Up @@ -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':
Expand Down Expand Up @@ -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)) {
Expand All @@ -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;
Expand All @@ -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)
);
}

Expand Down Expand Up @@ -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
*
Expand All @@ -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;
}
}
2 changes: 1 addition & 1 deletion src/retry.ts
Expand Up @@ -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
Expand Down
46 changes: 38 additions & 8 deletions test/test.getch.ts
Expand Up @@ -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,
Expand All @@ -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();

Expand Down Expand Up @@ -604,25 +601,56 @@ 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',
data: formData,
});
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<typeof nodeFetch>) => {
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);
});

Expand Down Expand Up @@ -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 () => {
Expand Down
1 change: 0 additions & 1 deletion test/test.retry.ts
Expand Up @@ -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';
Expand Down
6 changes: 4 additions & 2 deletions 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",
Expand Down

0 comments on commit 58c7dc2

Please sign in to comment.