From 7ab2f4b422b9b807958df2473a68aeb5946037ec Mon Sep 17 00:00:00 2001 From: axe312ger Date: Thu, 6 Jan 2022 16:04:35 +0100 Subject: [PATCH 1/6] fix: move queue from remote file node creation to remote file fetching --- packages/gatsby-core-utils/package.json | 1 + .../src/fetch-remote-file.ts | 58 +++++++++++++++++- .../gatsby-source-filesystem/package.json | 1 - .../src/create-remote-file-node.js | 61 ++----------------- 4 files changed, 62 insertions(+), 59 deletions(-) diff --git a/packages/gatsby-core-utils/package.json b/packages/gatsby-core-utils/package.json index 86f079f9387b0..77aa86dee0dd9 100644 --- a/packages/gatsby-core-utils/package.json +++ b/packages/gatsby-core-utils/package.json @@ -32,6 +32,7 @@ "@babel/runtime": "^7.15.4", "ci-info": "2.0.0", "configstore": "^5.0.1", + "fastq": "^1.13.0", "file-type": "^16.5.3", "fs-extra": "^10.0.0", "got": "^11.8.3", diff --git a/packages/gatsby-core-utils/src/fetch-remote-file.ts b/packages/gatsby-core-utils/src/fetch-remote-file.ts index 52e98d20cfcba..1fa6b7e183e1a 100644 --- a/packages/gatsby-core-utils/src/fetch-remote-file.ts +++ b/packages/gatsby-core-utils/src/fetch-remote-file.ts @@ -10,6 +10,8 @@ import { } from "./filename-utils" import type { IncomingMessage } from "http" import type { GatsbyCache } from "gatsby" +import Queue from "fastq" +import type { queue, done } from "fastq" export interface IFetchRemoteFileOptions { url: string @@ -72,9 +74,63 @@ const ERROR_CODES_TO_RETRY = [ `ERR_GOT_REQUEST_ERROR`, ] +/******************** + * Queue Management * + ********************/ + +const GATSBY_CONCURRENT_DOWNLOAD = process.env.GATSBY_CONCURRENT_DOWNLOAD + ? parseInt(process.env.GATSBY_CONCURRENT_DOWNLOAD, 10) || 0 + : 200 + +const q: queue = Queue( + pushToQueue, + GATSBY_CONCURRENT_DOWNLOAD +) + +/** + * pushToQueue + * -- + * Handle tasks that are pushed in to the Queue + */ +async function pushToQueue( + task: IFetchRemoteFileOptions, + cb: done +): Promise { + try { + const node = await fetchFile(task) + return cb(null, node) + } catch (e) { + return cb(e) + } +} + +/** + * pushTask + * -- + * pushes a task in to the Queue and the processing cache + * + * Promisfy a task in queue + * @param {CreateRemoteFileNodePayload} task + * @return {Promise} + */ +async function pushTask(task: IFetchRemoteFileOptions): Promise { + return new Promise((resolve, reject) => { + q.push(task, (err, node) => { + if (!err) { + resolve(node) + } else { + reject(err) + } + }) + }) +} let fetchCache = new Map() let latestBuildId = `` +/*************************** + * Fetch remote file logic * + ***************************/ + export async function fetchRemoteFile( args: IFetchRemoteFileOptions ): Promise { @@ -91,7 +147,7 @@ export async function fetchRemoteFile( } // Create file fetch promise and store it into cache - const fetchPromise = fetchFile(args) + const fetchPromise = pushTask(args) fetchCache.set(args.url, fetchPromise) return fetchPromise.catch(err => { diff --git a/packages/gatsby-source-filesystem/package.json b/packages/gatsby-source-filesystem/package.json index e182bc1010f28..11c8ba2fe85bf 100644 --- a/packages/gatsby-source-filesystem/package.json +++ b/packages/gatsby-source-filesystem/package.json @@ -9,7 +9,6 @@ "dependencies": { "@babel/runtime": "^7.15.4", "chokidar": "^3.5.2", - "fastq": "^1.13.0", "file-type": "^16.5.3", "fs-extra": "^10.0.0", "gatsby-core-utils": "^3.5.0-next.3", diff --git a/packages/gatsby-source-filesystem/src/create-remote-file-node.js b/packages/gatsby-source-filesystem/src/create-remote-file-node.js index 6f4596d1fc7f5..0a5abf6511486 100644 --- a/packages/gatsby-source-filesystem/src/create-remote-file-node.js +++ b/packages/gatsby-source-filesystem/src/create-remote-file-node.js @@ -6,7 +6,6 @@ const { } = require(`gatsby-core-utils`) const path = require(`path`) const { isWebUri } = require(`valid-url`) -const Queue = require(`fastq`) const { createFileNode } = require(`./create-file-node`) const { getRemoteFileExtension } = require(`./utils`) @@ -46,41 +45,6 @@ let showFlagWarning = !!process.env.GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER * @param {Reporter} [options.reporter] */ -/******************** - * Queue Management * - ********************/ - -const GATSBY_CONCURRENT_DOWNLOAD = process.env.GATSBY_CONCURRENT_DOWNLOAD - ? parseInt(process.env.GATSBY_CONCURRENT_DOWNLOAD, 10) || 0 - : 200 - -const queue = Queue(pushToQueue, GATSBY_CONCURRENT_DOWNLOAD) - -/** - * @callback {Queue~queueCallback} - * @param {*} error - * @param {*} result - */ - -/** - * pushToQueue - * -- - * Handle tasks that are pushed in to the Queue - * - * - * @param {CreateRemoteFileNodePayload} task - * @param {Queue~queueCallback} cb - * @return {Promise} - */ -async function pushToQueue(task, cb) { - try { - const node = await processRemoteNode(task) - return cb(null, node) - } catch (e) { - return cb(e) - } -} - /****************** * Core Functions * ******************/ @@ -155,25 +119,6 @@ async function fetchPlaceholder({ fromPath, url, cache, ext, name }) { * Index of promises resolving to File node from remote url */ const processingCache = {} -/** - * pushTask - * -- - * pushes a task in to the Queue and the processing cache - * - * Promisfy a task in queue - * @param {CreateRemoteFileNodePayload} task - * @return {Promise} - */ -const pushTask = task => - new Promise((resolve, reject) => { - queue.push(task, (err, node) => { - if (!err) { - resolve(node) - } else { - reject(`failed to process ${task.url}\n${err}`) - } - }) - }) /*************** * Entry Point * @@ -245,11 +190,13 @@ module.exports = function createRemoteFileNode({ if (!url || isWebUri(url) === undefined) { return Promise.reject( - `url passed to createRemoteFileNode is either missing or not a proper web uri: ${url}` + new Error( + `url passed to createRemoteFileNode is either missing or not a proper web uri: ${url}` + ) ) } - const fileDownloadPromise = pushTask({ + const fileDownloadPromise = processRemoteNode({ url, cache, createNode, From d3fd944c43482fb46930a4894dcb0965dae86b4f Mon Sep 17 00:00:00 2001 From: axe312ger Date: Tue, 11 Jan 2022 10:40:15 +0100 Subject: [PATCH 2/6] reduce number of concurrent requests per CPU core to 50 --- packages/gatsby-core-utils/src/fetch-remote-file.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby-core-utils/src/fetch-remote-file.ts b/packages/gatsby-core-utils/src/fetch-remote-file.ts index 1fa6b7e183e1a..33dd6ede5875f 100644 --- a/packages/gatsby-core-utils/src/fetch-remote-file.ts +++ b/packages/gatsby-core-utils/src/fetch-remote-file.ts @@ -80,7 +80,7 @@ const ERROR_CODES_TO_RETRY = [ const GATSBY_CONCURRENT_DOWNLOAD = process.env.GATSBY_CONCURRENT_DOWNLOAD ? parseInt(process.env.GATSBY_CONCURRENT_DOWNLOAD, 10) || 0 - : 200 + : 50 const q: queue = Queue( pushToQueue, From a7dd1c7def4c060dc80fb90d29655af063094a25 Mon Sep 17 00:00:00 2001 From: axe312ger Date: Tue, 11 Jan 2022 10:40:34 +0100 Subject: [PATCH 3/6] rename worker function to mark it as worker --- packages/gatsby-core-utils/src/fetch-remote-file.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/gatsby-core-utils/src/fetch-remote-file.ts b/packages/gatsby-core-utils/src/fetch-remote-file.ts index 33dd6ede5875f..18def3ad87ec2 100644 --- a/packages/gatsby-core-utils/src/fetch-remote-file.ts +++ b/packages/gatsby-core-utils/src/fetch-remote-file.ts @@ -83,16 +83,16 @@ const GATSBY_CONCURRENT_DOWNLOAD = process.env.GATSBY_CONCURRENT_DOWNLOAD : 50 const q: queue = Queue( - pushToQueue, + fetchWorker, GATSBY_CONCURRENT_DOWNLOAD ) /** - * pushToQueue + * fetchWorker * -- - * Handle tasks that are pushed in to the Queue + * Handle fetch requests that are pushed in to the Queue */ -async function pushToQueue( +async function fetchWorker( task: IFetchRemoteFileOptions, cb: done ): Promise { From 104545800d0c21e7fc37161c7de5d83c8b3830a0 Mon Sep 17 00:00:00 2001 From: axe312ger Date: Mon, 24 Jan 2022 11:21:57 +0100 Subject: [PATCH 4/6] improve typings --- packages/gatsby-core-utils/src/fetch-remote-file.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/gatsby-core-utils/src/fetch-remote-file.ts b/packages/gatsby-core-utils/src/fetch-remote-file.ts index 18def3ad87ec2..c526c2b213bed 100644 --- a/packages/gatsby-core-utils/src/fetch-remote-file.ts +++ b/packages/gatsby-core-utils/src/fetch-remote-file.ts @@ -82,7 +82,7 @@ const GATSBY_CONCURRENT_DOWNLOAD = process.env.GATSBY_CONCURRENT_DOWNLOAD ? parseInt(process.env.GATSBY_CONCURRENT_DOWNLOAD, 10) || 0 : 50 -const q: queue = Queue( +const q: queue = Queue( fetchWorker, GATSBY_CONCURRENT_DOWNLOAD ) @@ -94,13 +94,13 @@ const q: queue = Queue( */ async function fetchWorker( task: IFetchRemoteFileOptions, - cb: done -): Promise { + cb: done +): Promise { try { const node = await fetchFile(task) - return cb(null, node) + return void cb(null, node) } catch (e) { - return cb(e) + return void cb(e) } } From a955f169b605e831d4b3037a13d0d6a08b1728a4 Mon Sep 17 00:00:00 2001 From: axe312ger Date: Mon, 24 Jan 2022 11:25:29 +0100 Subject: [PATCH 5/6] refactor: remove GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER flag --- .../src/create-remote-file-node.js | 65 +++---------------- 1 file changed, 9 insertions(+), 56 deletions(-) diff --git a/packages/gatsby-source-filesystem/src/create-remote-file-node.js b/packages/gatsby-source-filesystem/src/create-remote-file-node.js index 0a5abf6511486..bfb4574d79eed 100644 --- a/packages/gatsby-source-filesystem/src/create-remote-file-node.js +++ b/packages/gatsby-source-filesystem/src/create-remote-file-node.js @@ -1,15 +1,6 @@ -const fs = require(`fs-extra`) -const { - createContentDigest, - fetchRemoteFile, - createFilePath, -} = require(`gatsby-core-utils`) -const path = require(`path`) +const { fetchRemoteFile } = require(`gatsby-core-utils`) const { isWebUri } = require(`valid-url`) const { createFileNode } = require(`./create-file-node`) -const { getRemoteFileExtension } = require(`./utils`) - -let showFlagWarning = !!process.env.GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER /******************** * Type Definitions * @@ -68,25 +59,14 @@ async function processRemoteNode({ ext, name, }) { - let filename - if (process.env.GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER) { - filename = await fetchPlaceholder({ - fromPath: process.env.GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER, - url, - cache, - ext, - name, - }) - } else { - filename = await fetchRemoteFile({ - url, - cache, - auth, - httpHeaders, - ext, - name, - }) - } + const filename = await fetchRemoteFile({ + url, + cache, + auth, + httpHeaders, + ext, + name, + }) // Create the file node. const fileNode = await createFileNode(filename, createNodeId, {}) @@ -102,19 +82,6 @@ async function processRemoteNode({ return fileNode } -async function fetchPlaceholder({ fromPath, url, cache, ext, name }) { - const pluginCacheDir = cache.directory - const digest = createContentDigest(url) - - if (!ext) { - ext = getRemoteFileExtension(url) - } - - const filename = createFilePath(path.join(pluginCacheDir, digest), name, ext) - fs.copySync(fromPath, filename) - return filename -} - /** * Index of promises resolving to File node from remote url */ @@ -147,20 +114,6 @@ module.exports = function createRemoteFileNode({ ext = null, name = null, }) { - if (showFlagWarning) { - showFlagWarning = false - // Note: This will use a placeholder image as the default for every file that is downloaded through this API. - // That may break certain cases, in particular when the file is not meant to be an image or when the image - // is expected to be of a particular type that is other than the placeholder. This API is meant to bypass - // the remote download for local testing only. - console.info( - `GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER: Any file downloaded by \`createRemoteFileNode\` will use the same placeholder image and skip the remote fetch. Note: This is an experimental flag that can change/disappear at any point.` - ) - console.info( - `GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER: File to use: \`${process.env.GATSBY_EXPERIMENTAL_REMOTE_FILE_PLACEHOLDER}\`` - ) - } - // validation of the input // without this it's notoriously easy to pass in the wrong `createNodeId` // see gatsbyjs/gatsby#6643 From e5a55794f08b1cbd8540e5f505c917b159954827 Mon Sep 17 00:00:00 2001 From: Ward Peeters Date: Fri, 28 Jan 2022 13:21:11 +0100 Subject: [PATCH 6/6] fix typing? --- packages/gatsby-core-utils/src/fetch-remote-file.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/gatsby-core-utils/src/fetch-remote-file.ts b/packages/gatsby-core-utils/src/fetch-remote-file.ts index c526c2b213bed..f739bc4104e4b 100644 --- a/packages/gatsby-core-utils/src/fetch-remote-file.ts +++ b/packages/gatsby-core-utils/src/fetch-remote-file.ts @@ -117,7 +117,8 @@ async function pushTask(task: IFetchRemoteFileOptions): Promise { return new Promise((resolve, reject) => { q.push(task, (err, node) => { if (!err) { - resolve(node) + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + resolve(node!) } else { reject(err) }