From 84faea8e65d0a80fdcc574390f27839d4be93133 Mon Sep 17 00:00:00 2001 From: Daniel Bankhead Date: Thu, 16 May 2024 13:02:51 -0700 Subject: [PATCH 1/3] 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 5235b9e..0eb2c4a 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 8da69dd..9e17a8d 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 a18ddef..7ec7763 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 d52aacb..9ccfad8 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 01e3b7f..f9afcb3 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 2/3] 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 0eb2c4a..6d9f221 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 9e17a8d..d583268 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 f9afcb3..85b1ed3 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 3/3] 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 6d9f221..e6fd144 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 d583268..9b59e8e 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 7ec7763..c563eac 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 85b1ed3..442cf95 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);