fix(gatsby-core-utils): fix fetchRemoteFile when called in main process after being called in worker (#33932)

Co-authored-by: Vladimir Razuvaev <[email protected]>

Author: pieh

packages/gatsby-core-utils/src/__tests__/fetch-remote-file.js

@@ -209,8 +209,8 @@ async function createMockCache() {
   fs.ensureDir(tmpDir)
 
   return {
-    get: jest.fn(),
-    set: jest.fn(),
+    get: jest.fn(() => Promise.resolve(null)),
+    set: jest.fn(() => Promise.resolve(null)),
     directory: tmpDir,
   }
 }
@@ -445,6 +445,123 @@ describe(`fetch-remote-file`, () => {
     expect(fsMove).toBeCalledTimes(0)
   })
 
+  it(`downloading a file in main process after downloading it in worker`, async () => {
+    // we don't want to wait for polling to finish
+    jest.useFakeTimers()
+    jest.runAllTimers()
+
+    const cacheInternals = new Map()
+    const workerCache = {
+      get(key) {
+        return Promise.resolve(cacheInternals.get(key))
+      },
+      set(key, value) {
+        return Promise.resolve(cacheInternals.set(key, value))
+      },
+      directory: cache.directory,
+    }
+
+    const fetchRemoteFileInstanceOne = getFetchInWorkerContext(`1`)
+
+    const resultFromWorker = await fetchRemoteFileInstanceOne({
+      url: `http://external.com/logo.svg`,
+      cache: workerCache,
+    })
+
+    jest.runAllTimers()
+
+    const resultFromMain = await fetchRemoteFile({
+      url: `http://external.com/logo.svg`,
+      cache: workerCache,
+    })
+
+    expect(resultFromWorker).not.toBeUndefined()
+    expect(resultFromMain).not.toBeUndefined()
+
+    jest.useRealTimers()
+
+    expect(gotStream).toBeCalledTimes(1)
+    expect(fsMove).toBeCalledTimes(1)
+  })
+
+  it(`downloading a file in worker process after downloading it in main`, async () => {
+    // we don't want to wait for polling to finish
+    jest.useFakeTimers()
+    jest.runAllTimers()
+
+    const cacheInternals = new Map()
+    const workerCache = {
+      get(key) {
+        return Promise.resolve(cacheInternals.get(key))
+      },
+      set(key, value) {
+        return Promise.resolve(cacheInternals.set(key, value))
+      },
+      directory: cache.directory,
+    }
+
+    const fetchRemoteFileInstanceOne = getFetchInWorkerContext(`1`)
+
+    const resultFromMain = await fetchRemoteFile({
+      url: `http://external.com/logo.svg`,
+      cache: workerCache,
+    })
+
+    jest.runAllTimers()
+
+    const resultFromWorker = await fetchRemoteFileInstanceOne({
+      url: `http://external.com/logo.svg`,
+      cache: workerCache,
+    })
+
+    jest.runAllTimers()
+    jest.useRealTimers()
+
+    expect(resultFromWorker).not.toBeUndefined()
+    expect(resultFromMain).not.toBeUndefined()
+    expect(gotStream).toBeCalledTimes(1)
+    expect(fsMove).toBeCalledTimes(1)
+  })
+
+  it(`downloading a file in worker process after downloading it in another worker`, async () => {
+    // we don't want to wait for polling to finish
+    jest.useFakeTimers()
+    jest.runAllTimers()
+
+    const cacheInternals = new Map()
+    const workerCache = {
+      get(key) {
+        return Promise.resolve(cacheInternals.get(key))
+      },
+      set(key, value) {
+        return Promise.resolve(cacheInternals.set(key, value))
+      },
+      directory: cache.directory,
+    }
+
+    const fetchRemoteFileInstanceOne = getFetchInWorkerContext(`1`)
+    const fetchRemoteFileInstanceTwo = getFetchInWorkerContext(`2`)
+
+    const resultFromWorker1 = await fetchRemoteFileInstanceOne({
+      url: `http://external.com/logo.svg`,
+      cache: workerCache,
+    })
+    jest.runAllTimers()
+
+    const resultFromWorker2 = await fetchRemoteFileInstanceTwo({
+      url: `http://external.com/logo.svg`,
+      cache: workerCache,
+    })
+
+    jest.runAllTimers()
+    jest.useRealTimers()
+
+    expect(resultFromWorker1).not.toBeUndefined()
+    expect(resultFromWorker2).not.toBeUndefined()
+    expect(gotStream).toBeCalledTimes(1)
+    expect(fsMove).toBeCalledTimes(1)
+  })
+
   it(`fails when 404 is triggered`, async () => {
     await expect(
       fetchRemoteFile({

packages/gatsby-core-utils/src/fetch-remote-file.ts

@@ -105,11 +105,6 @@ function pollUntilComplete(
   buildId: string,
   cb: (err?: Error, result?: string) => void
 ): void {
-  if (!IS_WORKER) {
-    // We are not in a worker, so we shouldn't use the cache
-    return void cb()
-  }
-
   cache.get(cacheIdForWorkers(url)).then(entry => {
     if (!entry || entry.buildId !== buildId) {
       return void cb()
@@ -160,14 +155,12 @@ async function fetchFile({
     return result
   }
 
-  if (IS_WORKER) {
-    await cache.set(cacheIdForWorkers(url), {
-      status: `pending`,
-      result: null,
-      workerId: WORKER_ID,
-      buildId: BUILD_ID,
-    })
-  }
+  await cache.set(cacheIdForWorkers(url), {
+    status: `pending`,
+    result: null,
+    workerId: WORKER_ID,
+    buildId: BUILD_ID,
+  })
 
   // See if there's response headers for this url
   // from a previous request.
@@ -231,7 +224,7 @@ async function fetchFile({
       }
     }
 
-    // Multiple workers have started the fetch and we need another check to only let one complete
+    // Multiple processes have started the fetch and we need another check to only let one complete
     const cacheEntry = await cache.get(cacheIdForWorkers(url))
     if (cacheEntry && cacheEntry.workerId !== WORKER_ID) {
       return new Promise<string>((resolve, reject) => {
@@ -258,35 +251,27 @@ async function fetchFile({
       await fs.remove(tmpFilename)
     }
 
-    if (IS_WORKER) {
+    await cache.set(cacheIdForWorkers(url), {
+      status: `complete`,
+      result: filename,
+      workerId: WORKER_ID,
+      buildId: BUILD_ID,
+    })
+
+    return filename
+  } catch (err) {
+    // enable multiple processes to continue when done
+    const cacheEntry = await cache.get(cacheIdForWorkers(url))
+
+    if (!cacheEntry || cacheEntry.workerId === WORKER_ID) {
       await cache.set(cacheIdForWorkers(url), {
-        status: `complete`,
-        result: filename,
+        status: `failed`,
+        result: err.toString ? err.toString() : err.message ? err.message : err,
         workerId: WORKER_ID,
         buildId: BUILD_ID,
       })
     }
 
-    return filename
-  } catch (err) {
-    // enable multiple workers to continue when done
-    if (IS_WORKER) {
-      const cacheEntry = await cache.get(cacheIdForWorkers(url))
-
-      if (!cacheEntry || cacheEntry.workerId === WORKER_ID) {
-        await cache.set(cacheIdForWorkers(url), {
-          status: `failed`,
-          result: err.toString
-            ? err.toString()
-            : err.message
-            ? err.message
-            : err,
-          workerId: WORKER_ID,
-          buildId: BUILD_ID,
-        })
-      }
-    }
-
     throw err
   }
 }