From d996153f3be6bcc9af460300e61103425323b973 Mon Sep 17 00:00:00 2001 From: Jonathan Date: Tue, 10 Sep 2024 18:29:14 -0700 Subject: [PATCH] Serialize URL fetches to avoid excessive concurrent OpenVSX requests (#4627) A speculative fix for errors of the form: ``` [13:38:25] Start fetching https://open-vsx.org/vscode/gallery/publishers/quarto/vsextensions/quarto/1.114.0/vspackage (2 retry) [13:38:35] Fetching https://open-vsx.org/vscode/gallery/publishers/ms-vscode/vsextensions/js-debug-companion/1.1.3/vspackage failed: TypeError: fetch failed ``` It appears that (even with fairly generous timeouts) we are regularly failing to get extensions from OpenVSX during the build. @petetronic and I suspect that this may be because of aggressive parallelization combined with rate limits or concurrency limits on OpenVSX; that is, it is not going to allow us to download 10 extensions at once, and we repeatedly get timeouts. The "fix" is to serialize these requests, so that instead of requesting all the extensions from OpenVSX at once, we only ask for one at a time, and wait for it to finish before requesting the next one. (the fix technically applies to _all_ URL fetching because this requires the fewest edits and may be useful elsewhere, but we could also scope it to extension downloads if there are concerns about the reach of this approach) --- build/lib/fetch.js | 80 ++++++++++++++++++++++++++++++++++++++++++- build/lib/fetch.ts | 85 +++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 163 insertions(+), 2 deletions(-) diff --git a/build/lib/fetch.js b/build/lib/fetch.js index b7da65f4af2..31a3447982d 100644 --- a/build/lib/fetch.js +++ b/build/lib/fetch.js @@ -13,6 +13,8 @@ const log = require("fancy-log"); const ansiColors = require("ansi-colors"); const crypto = require("crypto"); const through2 = require("through2"); +// --- Start Positron --- +const util_1 = require("./util"); function fetchUrls(urls, options) { if (options === undefined) { options = {}; @@ -25,11 +27,15 @@ function fetchUrls(urls, options) { } return es.readArray(urls).pipe(es.map((data, cb) => { const url = [options.base, data].join(''); - fetchUrl(url, options).then(file => { + // --- Start Positron --- + // Replace call to fetchUrl with fetchUrlQueued to limit the number of + // concurrent requests to OpenVSX + fetchUrlQueued(url, options).then(file => { cb(undefined, file); }, error => { cb(error); }); + // --- End Positron --- })); } async function fetchUrl(url, options, retries = 10, retryDelay = 1000) { @@ -135,4 +141,76 @@ function fetchGithub(repo, options) { } })); } +// --- Start Positron --- +/// A promise that fetches a URL from `fetchUrl` and returns a Vinyl file +class FetchPromise extends util_1.PromiseHandles { + url; + options; + retries; + retryDelay; + constructor(url, options, retries = 10, retryDelay = 1000) { + super(); + this.url = url; + this.options = options; + this.retries = retries; + this.retryDelay = retryDelay; + } +} +/// An array of fetch promises that are queued to be fetched +const fetchQueue = []; +let fetching = false; +/** + * Fetches a URL using `fetchUrl` and returns a Vinyl file. This function is + * similar to the `fetchUrl` function it wraps, but queues the request to limit + * the number of concurrent requests. + * + * @param url The URL to fetch + * @param options The fetch options + * @param retries The number of retries to attempt + * @param retryDelay The delay between retries + * + * @returns A promise that resolves to a Vinyl file + */ +function fetchUrlQueued(url, options, retries = 10, retryDelay = 1000) { + // Create a new fetch promise and push it to the fetch queue + const promise = new FetchPromise(url, options, retries, retryDelay); + fetchQueue.push(promise); + // Process the fetch queue immediately (a no-op if we are already fetching) + processFetchQueue(); + return promise.promise; +} +/// Processes the fetch queue by fetching the next URL in the queue +function processFetchQueue() { + // If we are already fetching or the fetch queue is empty, do nothing + if (fetching) { + return; + } + if (fetchQueue.length === 0) { + return; + } + // Splice off the next URL in the queue + const next = fetchQueue.splice(0, 1)[0]; + fetching = true; + // Determine if we should log verbose output (e.g. when running in CI) + const verbose = !!next.options.verbose || !!process.env['CI'] || !!process.env['BUILD_ARTIFACTSTAGINGDIRECTORY']; + if (verbose) { + log(`[Fetch queue] start fetching ${next.url} (${fetchQueue.length} remaining)`); + } + // Perform the fetch and resolve the promise when done + fetchUrl(next.url, next.options, next.retries, next.retryDelay).then((vinyl) => { + if (verbose) { + log(`[Fetch queue] completed fetching ${next.url} (${fetchQueue.length} remaining)`); + } + next.resolve(vinyl); + }).catch((e) => { + if (verbose) { + log(`[Fetch queue] failed fetching ${next.url} (${fetchQueue.length} remaining): ${e}`); + } + next.reject(e); + }).finally(() => { + fetching = false; + processFetchQueue(); + }); +} +// --- End Positron --- //# sourceMappingURL=fetch.js.map \ No newline at end of file diff --git a/build/lib/fetch.ts b/build/lib/fetch.ts index 0c44b8e567f..200e9b3e7a5 100644 --- a/build/lib/fetch.ts +++ b/build/lib/fetch.ts @@ -11,6 +11,10 @@ import * as crypto from 'crypto'; import * as through2 from 'through2'; import { Stream } from 'stream'; +// --- Start Positron --- +import { PromiseHandles } from './util'; +// --- End Positron --- + export interface IFetchOptions { base?: string; nodeFetchOptions?: RequestInit; @@ -33,11 +37,15 @@ export function fetchUrls(urls: string[] | string, options: IFetchOptions): es.T return es.readArray(urls).pipe(es.map((data: string, cb) => { const url = [options.base, data].join(''); - fetchUrl(url, options).then(file => { + // --- Start Positron --- + // Replace call to fetchUrl with fetchUrlQueued to limit the number of + // concurrent requests to OpenVSX + fetchUrlQueued(url, options).then(file => { cb(undefined, file); }, error => { cb(error); }); + // --- End Positron --- })); } @@ -148,3 +156,78 @@ export function fetchGithub(repo: string, options: IGitHubAssetOptions): Stream } })); } + +// --- Start Positron --- + +/// A promise that fetches a URL from `fetchUrl` and returns a Vinyl file +class FetchPromise extends PromiseHandles { + constructor(readonly url: string, readonly options: IFetchOptions, readonly retries = 10, readonly retryDelay = 1000) { + super(); + } +} + +/// An array of fetch promises that are queued to be fetched +const fetchQueue: Array = []; +let fetching = false; + +/** + * Fetches a URL using `fetchUrl` and returns a Vinyl file. This function is + * similar to the `fetchUrl` function it wraps, but queues the request to limit + * the number of concurrent requests. + * + * @param url The URL to fetch + * @param options The fetch options + * @param retries The number of retries to attempt + * @param retryDelay The delay between retries + * + * @returns A promise that resolves to a Vinyl file + */ +function fetchUrlQueued(url: string, options: IFetchOptions, retries = 10, retryDelay = 1000): Promise { + + // Create a new fetch promise and push it to the fetch queue + const promise = new FetchPromise(url, options, retries, retryDelay); + fetchQueue.push(promise); + + // Process the fetch queue immediately (a no-op if we are already fetching) + processFetchQueue(); + return promise.promise; +} + +/// Processes the fetch queue by fetching the next URL in the queue +function processFetchQueue() { + // If we are already fetching or the fetch queue is empty, do nothing + if (fetching) { + return; + } + if (fetchQueue.length === 0) { + return; + } + + // Splice off the next URL in the queue + const next = fetchQueue.splice(0, 1)[0]; + fetching = true; + + // Determine if we should log verbose output (e.g. when running in CI) + const verbose = !!next.options.verbose || !!process.env['CI'] || !!process.env['BUILD_ARTIFACTSTAGINGDIRECTORY']; + if (verbose) { + log(`[Fetch queue] start fetching ${next.url} (${fetchQueue.length} remaining)`); + } + + // Perform the fetch and resolve the promise when done + fetchUrl(next.url, next.options, next.retries, next.retryDelay).then((vinyl) => { + if (verbose) { + log(`[Fetch queue] completed fetching ${next.url} (${fetchQueue.length} remaining)`); + } + next.resolve(vinyl); + }).catch((e) => { + if (verbose) { + log(`[Fetch queue] failed fetching ${next.url} (${fetchQueue.length} remaining): ${e}`); + } + next.reject(e); + }).finally(() => { + fetching = false; + processFetchQueue(); + }); +} + +// --- End Positron ---