From ba4e6ef8ed525c4f9ef67423cd5b57b6ef25c992 Mon Sep 17 00:00:00 2001 From: Andrew Rumble Date: Tue, 18 Aug 2026 13:02:36 +0100 Subject: [PATCH] Merge pull request #35468 from overleaf/ar-fetch-utils-type-improvements [fetch-utils] type improvements GitOrigin-RevId: 4652054c715c4ddb19bac1f46cdf1fc3916c533d --- libraries/fetch-utils/index.ts | 124 ++++++++++++------ .../fetch-utils/test/unit/FetchUtils.test.js | 60 ++++++++- 2 files changed, 144 insertions(+), 40 deletions(-) diff --git a/libraries/fetch-utils/index.ts b/libraries/fetch-utils/index.ts index 64f0ed92cd..383fb60cd2 100644 --- a/libraries/fetch-utils/index.ts +++ b/libraries/fetch-utils/index.ts @@ -1,8 +1,7 @@ -import _ from 'lodash' import { Readable, Duplex } from 'node:stream' import OError from '@overleaf/o-error' -import fetch from 'node-fetch' -import type { Response } from 'node-fetch' +import fetch, { Headers } from 'node-fetch' +import type { RequestInit, Response } from 'node-fetch' import http from 'node:http' import https from 'node:https' import net from 'node:net' @@ -10,6 +9,23 @@ import tls from 'node:tls' let logger: { warn: (...args: any[]) => void } | undefined +type BasicAuthOptions = { + user: string + password: string +} + +type FetchRequestOptions = RequestInit & { + json?: unknown + basicAuth?: BasicAuthOptions +} + +type ParsedFetchOptions = Omit< + FetchRequestOptions, + 'json' | 'signal' | 'basicAuth' +> & { + signal?: AbortSignal +} + function setLogger(loggerInstance: { warn: (...args: any[]) => void }) { logger = loggerInstance } @@ -19,15 +35,22 @@ function setLogger(loggerInstance: { warn: (...args: any[]) => void }) { * * @throws {RequestFailedError} if the response has a failure status code */ -async function fetchJson(url: string | URL, opts: any = {}) { +async function fetchJson(url: string | URL, opts: FetchRequestOptions = {}) { const { json } = await fetchJsonWithResponse(url, opts) return json } -async function fetchJsonWithResponse(url: string | URL, opts: any = {}) { +async function fetchJsonWithResponse( + url: string | URL, + opts: FetchRequestOptions = {} +) { const { fetchOpts, detachSignal } = parseOpts(opts, url) - fetchOpts.headers = fetchOpts.headers ?? {} - fetchOpts.headers.Accept = fetchOpts.headers.Accept ?? 'application/json' + if (!(fetchOpts.headers instanceof Headers)) { + fetchOpts.headers = new Headers(fetchOpts.headers) + } + if (!fetchOpts.headers.has('Accept')) { + fetchOpts.headers.set('Accept', 'application/json') + } const response = await performRequest(url, fetchOpts, detachSignal) if (!response.ok) { @@ -46,12 +69,15 @@ async function fetchJsonWithResponse(url: string | URL, opts: any = {}) { * * @throws {RequestFailedError} if the response has a failure status code */ -async function fetchStream(url: string | URL, opts: any = {}) { +async function fetchStream(url: string | URL, opts: FetchRequestOptions = {}) { const { stream } = await fetchStreamWithResponse(url, opts) return stream } -async function fetchStreamWithResponse(url: string | URL, opts: any = {}) { +async function fetchStreamWithResponse( + url: string | URL, + opts: FetchRequestOptions = {} +) { const { fetchOpts, abortController, detachSignal } = parseOpts(opts, url) const response = await performRequest(url, fetchOpts, detachSignal) @@ -71,7 +97,7 @@ async function fetchStreamWithResponse(url: string | URL, opts: any = {}) { * * @throws {RequestFailedError} if the response has a failure status code */ -async function fetchNothing(url: string | URL, opts: any = {}) { +async function fetchNothing(url: string | URL, opts: FetchRequestOptions = {}) { const { fetchOpts, detachSignal } = parseOpts(opts, url) const response = await performRequest(url, fetchOpts, detachSignal) if (!response.ok) { @@ -87,7 +113,10 @@ async function fetchNothing(url: string | URL, opts: any = {}) { * * @throws {RequestFailedError} if the response has a non redirect status code or missing Location header */ -async function fetchRedirect(url: string | URL, opts: any = {}) { +async function fetchRedirect( + url: string | URL, + opts: FetchRequestOptions = {} +) { const { location } = await fetchRedirectWithResponse(url, opts) return location } @@ -97,7 +126,10 @@ async function fetchRedirect(url: string | URL, opts: any = {}) { * * @throws {RequestFailedError} if the response has a non redirect status code or missing Location header */ -async function fetchRedirectWithResponse(url: string | URL, opts: any = {}) { +async function fetchRedirectWithResponse( + url: string | URL, + opts: FetchRequestOptions = {} +) { const { fetchOpts, detachSignal } = parseOpts(opts, url) fetchOpts.redirect = 'manual' const response = await performRequest(url, fetchOpts, detachSignal) @@ -123,12 +155,15 @@ async function fetchRedirectWithResponse(url: string | URL, opts: any = {}) { * * @throws {RequestFailedError} if the response has a failure status code */ -async function fetchString(url: string | URL, opts: any = {}) { +async function fetchString(url: string | URL, opts: FetchRequestOptions = {}) { const { body } = await fetchStringWithResponse(url, opts) return body } -async function fetchStringWithResponse(url: string | URL, opts: any = {}) { +async function fetchStringWithResponse( + url: string | URL, + opts: FetchRequestOptions = {} +) { const { fetchOpts, detachSignal } = parseOpts(opts, url) const response = await performRequest(url, fetchOpts, detachSignal) if (!response.ok) { @@ -145,7 +180,7 @@ class RequestFailedError extends OError { constructor( url: string | URL, - opts: any, + opts: Pick, response: Response, body: string | null ) { @@ -163,20 +198,21 @@ class RequestFailedError extends OError { } } -function setupDefaultAgent(fetchOps: any) { +function setupDefaultAgent(fetchOpts: ParsedFetchOptions) { // Provide a function to get the agent for each request as there may be // multiple requests with different protocols due to redirects. - fetchOps.agent = (url: URL) => + fetchOpts.agent = (url: URL) => url.protocol === 'https:' ? httpsAgent : httpAgent } -function parseOpts(opts: any, url: string | URL) { - const fetchOpts = _.omit(opts, ['json', 'signal', 'basicAuth']) - if (opts.json) { - setupJsonBody(fetchOpts, opts.json) +function parseOpts(opts: FetchRequestOptions, url: string | URL) { + const { json, signal, basicAuth, ...rawFetchOpts } = opts + const fetchOpts: ParsedFetchOptions = { ...rawFetchOpts } + if (json) { + setupJsonBody(fetchOpts, json) } - if (opts.basicAuth) { - setupBasicAuth(fetchOpts, opts.basicAuth) + if (basicAuth) { + setupBasicAuth(fetchOpts, basicAuth) } if (!fetchOpts.agent) { setupDefaultAgent(fetchOpts) @@ -184,9 +220,9 @@ function parseOpts(opts: any, url: string | URL) { const abortController = new AbortController() fetchOpts.signal = abortController.signal - let detachSignal - if (opts.signal) { - detachSignal = abortOnSignal(abortController, opts.signal) + let detachSignal: () => void + if (signal) { + detachSignal = abortOnSignal(abortController, signal) } else { let overTimeoutStart: bigint | undefined const stack = new Error().stack @@ -199,7 +235,7 @@ function parseOpts(opts: any, url: string | URL) { logger.warn( { url, - method: opts.method ?? 'GET', + method: fetchOpts.method ?? 'GET', overTimeoutMs: Number(process.hrtime.bigint() - overTimeoutStart) / 1e6, stack, @@ -209,23 +245,32 @@ function parseOpts(opts: any, url: string | URL) { } } } - if (opts.body instanceof Readable) { + if (fetchOpts.body instanceof Readable) { abortOnDestroyedRequest(abortController, fetchOpts.body) } return { fetchOpts, abortController, detachSignal } } -function setupJsonBody(fetchOpts: any, json: any) { +function setupJsonBody(fetchOpts: ParsedFetchOptions, json: unknown) { fetchOpts.body = JSON.stringify(json) - fetchOpts.headers = fetchOpts.headers ?? {} - fetchOpts.headers['Content-Type'] = 'application/json' + if (!(fetchOpts.headers instanceof Headers)) { + fetchOpts.headers = new Headers(fetchOpts.headers) + } + fetchOpts.headers.set('Content-Type', 'application/json') } -function setupBasicAuth(fetchOpts: any, basicAuth: any) { - fetchOpts.headers = fetchOpts.headers ?? {} - fetchOpts.headers.Authorization = +function setupBasicAuth( + fetchOpts: ParsedFetchOptions, + basicAuth: BasicAuthOptions +) { + if (!(fetchOpts.headers instanceof Headers)) { + fetchOpts.headers = new Headers(fetchOpts.headers) + } + fetchOpts.headers.set( + 'Authorization', 'Basic ' + - Buffer.from(`${basicAuth.user}:${basicAuth.password}`).toString('base64') + Buffer.from(`${basicAuth.user}:${basicAuth.password}`).toString('base64') + ) } function abortOnSignal(abortController: AbortController, signal: AbortSignal) { @@ -243,7 +288,7 @@ function abortOnSignal(abortController: AbortController, signal: AbortSignal) { function abortOnDestroyedRequest( abortController: AbortController, - stream: any + stream: Readable ) { stream.on('close', () => { if (!stream.readableEnded) { @@ -265,7 +310,7 @@ function abortOnDestroyedResponse( async function performRequest( url: string | URL, - fetchOpts: any, + fetchOpts: ParsedFetchOptions, detachSignal: () => void ) { let response @@ -283,9 +328,10 @@ async function performRequest( } response.body.on('close', detachSignal) if (fetchOpts.body instanceof Readable) { + const requestBodyStream = fetchOpts.body response.body.on('close', () => { - if (!fetchOpts.body.readableEnded) { - fetchOpts.body.destroy() + if (!requestBodyStream.readableEnded) { + requestBodyStream.destroy() } }) } diff --git a/libraries/fetch-utils/test/unit/FetchUtils.test.js b/libraries/fetch-utils/test/unit/FetchUtils.test.js index 4b81be0079..d4c112447b 100644 --- a/libraries/fetch-utils/test/unit/FetchUtils.test.js +++ b/libraries/fetch-utils/test/unit/FetchUtils.test.js @@ -7,7 +7,7 @@ import fs from 'node:fs' import { pipeline } from 'node:stream/promises' import { Readable } from 'node:stream' import { describe, beforeAll, beforeEach, afterAll, it } from 'vitest' -import { AbortError, FetchError } from 'node-fetch' +import { AbortError, FetchError, Headers } from 'node-fetch' import selfsigned from 'selfsigned' import { TestServer } from './helpers/TestServer.js' import { @@ -128,6 +128,15 @@ describe('fetch-utils', function () { expect(json).to.deep.equal({ key: 'verysecret' }) }) + it('sets an Authorization header for basic auth', async function () { + await fetchJson(url('/json/basic-auth'), { + basicAuth: { user: 'user', password: 'pass' }, + }) + expect(server.lastReq.headers.authorization).to.equal( + 'Basic ' + Buffer.from('user:pass').toString('base64') + ) + }) + it("destroys the request body if it doesn't get consumed", async function () { const stream = Readable.from(infiniteIterator()) await fetchJson(url('/json/ignore-request'), { @@ -136,6 +145,55 @@ describe('fetch-utils', function () { }) expect(stream.destroyed).to.be.true }) + + describe('headers', function () { + it('sets an Accept header of application/json by default', async function () { + await fetchJson(url('/json/hello')) + expect(server.lastReq.headers.accept).to.equal('application/json') + }) + + it('passes provided headers', async function () { + await fetchJson(url('/json/hello'), { + headers: { 'x-some-value': 'value' }, + }) + expect(server.lastReq.headers['x-some-value']).to.equal('value') + }) + + it('respects an explicitly provided Accept header', async function () { + await fetchJson(url('/json/hello'), { + headers: { Accept: 'application/vnd.api+json' }, + }) + expect(server.lastReq.headers.accept).to.equal( + 'application/vnd.api+json' + ) + }) + + it('sets the default Accept header when headers are provided as a Headers instance', async function () { + await fetchJson(url('/json/hello'), { + headers: new Headers({ 'X-Foo': 'bar' }), + }) + expect(server.lastReq.headers.accept).to.equal('application/json') + expect(server.lastReq.headers['x-foo']).to.equal('bar') + }) + + it('sets the default Accept header when headers are provided as an array of tuples', async function () { + await fetchJson(url('/json/hello'), { + headers: [['X-Foo', 'bar']], + }) + expect(server.lastReq.headers.accept).to.equal('application/json') + expect(server.lastReq.headers['x-foo']).to.equal('bar') + }) + + it('sets a Content-Type header of application/json when sending a JSON body', async function () { + await fetchJson(url('/json/add'), { + method: 'POST', + json: { a: 2, b: 3 }, + }) + expect(server.lastReq.headers['content-type']).to.equal( + 'application/json' + ) + }) + }) }) describe('fetchStream', function () {