From 001a6bc0c0238b46d0fd805019c3b9e79181beb7 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 2 Jan 2024 11:23:33 -0800 Subject: [PATCH 1/6] feat: Lazily load `https-proxy-agent` --- src/gaxios.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/gaxios.ts b/src/gaxios.ts index a0eba80..48b9cd5 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -30,9 +30,6 @@ import { } from './common'; import {getRetryConfig} from './retry'; import {Stream} from 'stream'; -import {HttpsProxyAgent as httpsProxyAgent} from 'https-proxy-agent'; - -/* eslint-disable @typescript-eslint/no-explicit-any */ const fetch = hasFetch() ? window.fetch : nodeFetch; @@ -71,10 +68,14 @@ function loadProxy() { process?.env?.HTTP_PROXY || process?.env?.http_proxy; if (proxy) { + // selectively require, for browser compatibility purposes + const {HttpsProxyAgent: httpsProxyAgent} = require('https-proxy-agent'); HttpsProxyAgent = httpsProxyAgent; } + return proxy; } + loadProxy(); function skipProxy(url: string) { From 1e3d0441eb3e419b46105918cac484d71b484f13 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Tue, 2 Jan 2024 11:24:10 -0800 Subject: [PATCH 2/6] feat: Lazily load `https-proxy-agent` --- src/gaxios.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/gaxios.ts b/src/gaxios.ts index a0eba80..48b9cd5 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -30,9 +30,6 @@ import { } from './common'; import {getRetryConfig} from './retry'; import {Stream} from 'stream'; -import {HttpsProxyAgent as httpsProxyAgent} from 'https-proxy-agent'; - -/* eslint-disable @typescript-eslint/no-explicit-any */ const fetch = hasFetch() ? window.fetch : nodeFetch; @@ -71,10 +68,14 @@ function loadProxy() { process?.env?.HTTP_PROXY || process?.env?.http_proxy; if (proxy) { + // selectively require, for browser compatibility purposes + const {HttpsProxyAgent: httpsProxyAgent} = require('https-proxy-agent'); HttpsProxyAgent = httpsProxyAgent; } + return proxy; } + loadProxy(); function skipProxy(url: string) { From 26813adc2b74fbaaf1b29fdc5c579b4268229e9c Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 31 Jan 2024 10:15:28 -0800 Subject: [PATCH 3/6] fix: Do Not Mutate Config for Redacted Retries --- src/common.ts | 35 +++++++++++++++++------------------ src/gaxios.ts | 7 ++++++- test/test.getch.ts | 9 ++++++++- 3 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/common.ts b/src/common.ts index de944c7..0e144d2 100644 --- a/src/common.ts +++ b/src/common.ts @@ -15,6 +15,7 @@ import {Agent} from 'http'; import {URL} from 'url'; import {pkg} from './util'; +import extend from 'extend'; /** * Support `instanceof` operator for `GaxiosError`s in different versions of this library. @@ -81,9 +82,19 @@ export class GaxiosError extends Error { ) { super(message); + // deep-copy config as we do not want to mutate + // the existing config for future retries/use + this.config = extend(true, {}, config); + if (this.response) { + this.response.config = extend(true, {}, this.response.config); + } + if (this.response) { try { - this.response.data = translateData(config.responseType, response?.data); + this.response.data = translateData( + this.config.responseType, + this.response?.data + ); } catch { // best effort - don't throw an error within an error // we could set `this.response.config.responseType = 'unknown'`, but @@ -98,22 +109,10 @@ export class GaxiosError extends Error { } if (config.errorRedactor) { - const errorRedactor = config.errorRedactor; - - // shallow-copy config for redaction as we do not want - // future requests to have redacted information - this.config = {...config}; - if (this.response) { - // copy response's config, as it may be recursively redacted - this.response = {...this.response, config: {...this.response.config}}; - } - - const results = errorRedactor({config, response}); - this.config = {...config, ...results.config}; - - if (this.response) { - this.response = {...this.response, ...results.response, config}; - } + config.errorRedactor({ + config: this.config, + response: this.response, + }); } } } @@ -148,7 +147,7 @@ export interface GaxiosOptions { options: GaxiosOptions, defaultAdapter: (options: GaxiosOptions) => GaxiosPromise ) => GaxiosPromise; - url?: string; + url?: string; // TODO: | URL baseUrl?: string; // deprecated baseURL?: string; method?: diff --git a/src/gaxios.ts b/src/gaxios.ts index 48b9cd5..054b712 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -187,7 +187,12 @@ export class Gaxios { if (shouldRetry && config) { err.config.retryConfig!.currentRetryAttempt = config.retryConfig!.currentRetryAttempt; - return this._request(err.config); + + // The error's config could be redacted - therefore we only want to + // copy the retry state over to the existing config + opts.retryConfig = err.config?.retryConfig; + + return this._request(opts); } throw err; } diff --git a/test/test.getch.ts b/test/test.getch.ts index f61eeea..30fcc8c 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -702,6 +702,7 @@ describe('🎏 data handling', () => { body: 'grant_type=somesensitivedata&assertion=somesensitivedata', }; + // simulate JSON response const responseHeaders = { ...config.headers, 'content-type': 'application/json', @@ -714,9 +715,14 @@ describe('🎏 data handling', () => { .reply(404, response, responseHeaders); const instance = new Gaxios(JSON.parse(JSON.stringify(config))); + const requestConfig: GaxiosOptions = { + url: customURL.toString(), + method: 'POST', + }; + const requestConfigCopy = JSON.parse(JSON.stringify({...requestConfig})); try { - await instance.request({url: customURL.toString(), method: 'POST'}); + await instance.request(requestConfig); throw new Error('Expected a GaxiosError'); } catch (e) { @@ -724,6 +730,7 @@ describe('🎏 data handling', () => { // config should not be mutated assert.deepStrictEqual(instance.defaults, config); + assert.deepStrictEqual(requestConfig, requestConfigCopy); assert.notStrictEqual(e.config, config); // config redactions - headers From 8e325dde66017046c6710fcb7b49178ce6e1c363 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 31 Jan 2024 10:17:01 -0800 Subject: [PATCH 4/6] fix: Do Not Mutate Config for Redacted Retries --- src/common.ts | 33 ++++++++++++++++----------------- src/gaxios.ts | 7 ++++++- test/test.getch.ts | 9 ++++++++- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/src/common.ts b/src/common.ts index de944c7..07f52f5 100644 --- a/src/common.ts +++ b/src/common.ts @@ -15,6 +15,7 @@ import {Agent} from 'http'; import {URL} from 'url'; import {pkg} from './util'; +import extend from 'extend'; /** * Support `instanceof` operator for `GaxiosError`s in different versions of this library. @@ -81,9 +82,19 @@ export class GaxiosError extends Error { ) { super(message); + // deep-copy config as we do not want to mutate + // the existing config for future retries/use + this.config = extend(true, {}, config); + if (this.response) { + this.response.config = extend(true, {}, this.response.config); + } + if (this.response) { try { - this.response.data = translateData(config.responseType, response?.data); + this.response.data = translateData( + this.config.responseType, + this.response?.data + ); } catch { // best effort - don't throw an error within an error // we could set `this.response.config.responseType = 'unknown'`, but @@ -98,22 +109,10 @@ export class GaxiosError extends Error { } if (config.errorRedactor) { - const errorRedactor = config.errorRedactor; - - // shallow-copy config for redaction as we do not want - // future requests to have redacted information - this.config = {...config}; - if (this.response) { - // copy response's config, as it may be recursively redacted - this.response = {...this.response, config: {...this.response.config}}; - } - - const results = errorRedactor({config, response}); - this.config = {...config, ...results.config}; - - if (this.response) { - this.response = {...this.response, ...results.response, config}; - } + config.errorRedactor({ + config: this.config, + response: this.response, + }); } } } diff --git a/src/gaxios.ts b/src/gaxios.ts index 48b9cd5..054b712 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -187,7 +187,12 @@ export class Gaxios { if (shouldRetry && config) { err.config.retryConfig!.currentRetryAttempt = config.retryConfig!.currentRetryAttempt; - return this._request(err.config); + + // The error's config could be redacted - therefore we only want to + // copy the retry state over to the existing config + opts.retryConfig = err.config?.retryConfig; + + return this._request(opts); } throw err; } diff --git a/test/test.getch.ts b/test/test.getch.ts index f61eeea..30fcc8c 100644 --- a/test/test.getch.ts +++ b/test/test.getch.ts @@ -702,6 +702,7 @@ describe('🎏 data handling', () => { body: 'grant_type=somesensitivedata&assertion=somesensitivedata', }; + // simulate JSON response const responseHeaders = { ...config.headers, 'content-type': 'application/json', @@ -714,9 +715,14 @@ describe('🎏 data handling', () => { .reply(404, response, responseHeaders); const instance = new Gaxios(JSON.parse(JSON.stringify(config))); + const requestConfig: GaxiosOptions = { + url: customURL.toString(), + method: 'POST', + }; + const requestConfigCopy = JSON.parse(JSON.stringify({...requestConfig})); try { - await instance.request({url: customURL.toString(), method: 'POST'}); + await instance.request(requestConfig); throw new Error('Expected a GaxiosError'); } catch (e) { @@ -724,6 +730,7 @@ describe('🎏 data handling', () => { // config should not be mutated assert.deepStrictEqual(instance.defaults, config); + assert.deepStrictEqual(requestConfig, requestConfigCopy); assert.notStrictEqual(e.config, config); // config redactions - headers From 809c6d17d8d1d3e6e9bd7c333e706b2d61508097 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 31 Jan 2024 10:17:47 -0800 Subject: [PATCH 5/6] chore: remove note --- src/common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common.ts b/src/common.ts index 0e144d2..07f52f5 100644 --- a/src/common.ts +++ b/src/common.ts @@ -147,7 +147,7 @@ export interface GaxiosOptions { options: GaxiosOptions, defaultAdapter: (options: GaxiosOptions) => GaxiosPromise ) => GaxiosPromise; - url?: string; // TODO: | URL + url?: string; baseUrl?: string; // deprecated baseURL?: string; method?: From 2bed79b6775b3c13f54ef97c0557a23f9c118bbd Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Wed, 31 Jan 2024 10:20:27 -0800 Subject: [PATCH 6/6] chore: Remove extraneous --- src/gaxios.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/gaxios.ts b/src/gaxios.ts index 054b712..9a8af19 100644 --- a/src/gaxios.ts +++ b/src/gaxios.ts @@ -30,6 +30,9 @@ import { } from './common'; import {getRetryConfig} from './retry'; import {Stream} from 'stream'; +import {HttpsProxyAgent as httpsProxyAgent} from 'https-proxy-agent'; + +/* eslint-disable @typescript-eslint/no-explicit-any */ const fetch = hasFetch() ? window.fetch : nodeFetch; @@ -68,8 +71,6 @@ function loadProxy() { process?.env?.HTTP_PROXY || process?.env?.http_proxy; if (proxy) { - // selectively require, for browser compatibility purposes - const {HttpsProxyAgent: httpsProxyAgent} = require('https-proxy-agent'); HttpsProxyAgent = httpsProxyAgent; }