Skip to content

Commit

Permalink
fix: Do Not Mutate Config for Redacted Retries (#597)
Browse files Browse the repository at this point in the history
* feat: Lazily load `https-proxy-agent`

* feat: Lazily load `https-proxy-agent`

* fix: Do Not Mutate Config for Redacted Retries

* fix: Do Not Mutate Config for Redacted Retries

* chore: remove note

* chore: Remove extraneous
  • Loading branch information
danielbankhead committed Jan 31, 2024
1 parent 3cc1c76 commit 4d1a551
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 19 deletions.
33 changes: 16 additions & 17 deletions src/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -81,9 +82,19 @@ export class GaxiosError<T = any> 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
Expand All @@ -98,22 +109,10 @@ export class GaxiosError<T = any> extends Error {
}

if (config.errorRedactor) {
const errorRedactor = config.errorRedactor<T>;

// 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<T>({
config: this.config,
response: this.response,
});
}
}
}
Expand Down
9 changes: 8 additions & 1 deletion src/gaxios.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,10 @@ function loadProxy() {
if (proxy) {
HttpsProxyAgent = httpsProxyAgent;
}

return proxy;
}

loadProxy();

function skipProxy(url: string) {
Expand Down Expand Up @@ -186,7 +188,12 @@ export class Gaxios {
if (shouldRetry && config) {
err.config.retryConfig!.currentRetryAttempt =
config.retryConfig!.currentRetryAttempt;
return this._request<T>(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<T>(opts);
}
throw err;
}
Expand Down
9 changes: 8 additions & 1 deletion test/test.getch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,7 @@ describe('🎏 data handling', () => {
body: 'grant_type=somesensitivedata&assertion=somesensitivedata',
};

// simulate JSON response
const responseHeaders = {
...config.headers,
'content-type': 'application/json',
Expand All @@ -725,16 +726,22 @@ 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) {
assert(e instanceof GaxiosError);

// config should not be mutated
assert.deepStrictEqual(instance.defaults, config);
assert.deepStrictEqual(requestConfig, requestConfigCopy);
assert.notStrictEqual(e.config, config);

// config redactions - headers
Expand Down

0 comments on commit 4d1a551

Please sign in to comment.