fix(gatsby-core-utils): handle 304 correctly between builds (#33975)

Author: wardpeet

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

@@ -118,12 +118,28 @@ const server = setupServer(
     )
 
     return res(
-      ctx.set(`Content-Type`, `image/svg+xml`),
+      ctx.set(`Content-Type`, `image/jpg`),
       ctx.set(`Content-Length`, contentLength),
       ctx.status(200),
       ctx.body(content)
     )
   }),
+  rest.get(`http://external.com/dog-304.jpg`, async (req, res, ctx) => {
+    const { content, contentLength } = await getFileContent(
+      path.join(__dirname, `./fixtures/dog-thumbnail.jpg`),
+      req
+    )
+
+    // console.log(req.headers)
+
+    return res(
+      ctx.set(`Content-Type`, `image/jpg`),
+      ctx.set(`Content-Length`, contentLength),
+      ctx.set(`etag`, `abcd`),
+      ctx.status(req.headers.get(`if-none-match`) === `abcd` ? 304 : 200),
+      ctx.body(content)
+    )
+  }),
   rest.get(`http://external.com/404.jpg`, async (req, res, ctx) => {
     const content = `Page not found`
 
@@ -249,7 +265,7 @@ describe(`fetch-remote-file`, () => {
     jest.useRealTimers()
   })
 
-  it(`downloads and create a file`, async () => {
+  it(`downloads and create a svg file`, async () => {
     const filePath = await fetchRemoteFile({
       url: `http://external.com/logo.svg`,
       cache,
@@ -272,7 +288,7 @@ describe(`fetch-remote-file`, () => {
     expect(gotStream).toBeCalledTimes(1)
   })
 
-  it(`downloads and create a file`, async () => {
+  it(`downloads and create a jpg file`, async () => {
     const filePath = await fetchRemoteFile({
       url: `http://external.com/dog.jpg`,
       cache,
@@ -412,6 +428,35 @@ describe(`fetch-remote-file`, () => {
     expect(fsMove).toBeCalledTimes(2)
   })
 
+  it(`handles 304 responses correctly in different builds`, async () => {
+    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,
+    }
+
+    global.__GATSBY = { buildId: `1` }
+    const filePath = await fetchRemoteFile({
+      url: `http://external.com/dog-304.jpg`,
+      cache: workerCache,
+    })
+
+    global.__GATSBY = { buildId: `2` }
+    const filePathCached = await fetchRemoteFile({
+      url: `http://external.com/dog-304.jpg`,
+      cache: workerCache,
+    })
+
+    expect(filePathCached).toBe(filePath)
+    expect(fsMove).toBeCalledTimes(1)
+    expect(gotStream).toBeCalledTimes(2)
+  })
+
   it(`doesn't keep lock when file download failed`, async () => {
     const cacheInternals = new Map()
     const workerCache = {
@@ -562,6 +607,38 @@ describe(`fetch-remote-file`, () => {
     expect(fsMove).toBeCalledTimes(1)
   })
 
+  it(`handles 304 responses correctly in different builds and workers`, async () => {
+    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`)
+
+    global.__GATSBY = { buildId: `1` }
+    const filePath = await fetchRemoteFileInstanceOne({
+      url: `http://external.com/dog-304.jpg`,
+      cache: workerCache,
+    })
+
+    global.__GATSBY = { buildId: `2` }
+    const filePathCached = await fetchRemoteFileInstanceTwo({
+      url: `http://external.com/dog-304.jpg`,
+      cache: workerCache,
+    })
+
+    expect(filePathCached).toBe(filePath)
+    expect(fsMove).toBeCalledTimes(1)
+    expect(gotStream).toBeCalledTimes(2)
+  })
+
   it(`fails when 404 is triggered`, async () => {
     await expect(
       fetchRemoteFile({

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

@@ -164,8 +164,8 @@ async function fetchFile({
 
   // See if there's response headers for this url
   // from a previous request.
-  const cachedHeaders = await cache.get(cacheIdForHeaders(url))
-
+  const { headers: cachedHeaders, digest: originalDigest } =
+    (await cache.get(cacheIdForHeaders(url))) ?? {}
   const headers = { ...httpHeaders }
   if (cachedHeaders && cachedHeaders.etag) {
     headers[`If-None-Match`] = cachedHeaders.etag
@@ -207,7 +207,10 @@ async function fetchFile({
 
     if (response.statusCode === 200) {
       // Save the response headers for future requests.
-      await cache.set(cacheIdForHeaders(url), response.headers)
+      await cache.set(cacheIdForHeaders(url), {
+        headers: response.headers,
+        digest,
+      })
 
       // If the user did not provide an extension and we couldn't get one from remote file, try and guess one
       if (!ext) {
@@ -240,10 +243,11 @@ async function fetchFile({
 
     // If the status code is 200, move the piped temp file to the real name.
     const filename = createFilePath(
-      path.join(pluginCacheDir, digest),
+      path.join(pluginCacheDir, originalDigest ?? digest),
       name,
       ext as string
     )
+
     if (response.statusCode === 200) {
       await fs.move(tmpFilename, filename, { overwrite: true })
       // Else if 304, remove the empty response.