Skip to content

Commit

Permalink
Serialize URL fetches to avoid excessive concurrent OpenVSX requests (#…
Browse files Browse the repository at this point in the history
…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)
  • Loading branch information
jmcphers committed Sep 11, 2024
1 parent 475768a commit d996153
Show file tree
Hide file tree
Showing 2 changed files with 163 additions and 2 deletions.
80 changes: 79 additions & 1 deletion build/lib/fetch.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

85 changes: 84 additions & 1 deletion build/lib/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -33,11 +37,15 @@ export function fetchUrls(urls: string[] | string, options: IFetchOptions): es.T

return es.readArray(urls).pipe(es.map<string, VinylFile | void>((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 ---
}));
}

Expand Down Expand Up @@ -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<VinylFile> {
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<FetchPromise> = [];
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<VinylFile> {

// 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 ---

0 comments on commit d996153

Please sign in to comment.