Skip to content

Commit

Permalink
fix(gatsby-core-utils): fix 304 when file does not exists (#34842)
Browse files Browse the repository at this point in the history
  • Loading branch information
wardpeet authored Feb 18, 2022
1 parent a76de9e commit 1f3eee0
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 15 deletions.
35 changes: 30 additions & 5 deletions packages/gatsby-core-utils/src/__tests__/fetch-remote-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -229,28 +229,28 @@ async function createMockCache(tmpDir) {

describe(`fetch-remote-file`, () => {
let cache
const cachePath = path.join(__dirname, `.cache-fetch`)
const cacheRoot = path.join(__dirname, `.cache-fetch`)
const cachePath = path.join(__dirname, `.cache-fetch`, `files`)

beforeAll(async () => {
// Establish requests interception layer before all tests.
server.listen()

cache = await createMockCache(cachePath)
await fs.ensureDir(cachePath)
storage.getDatabaseDir.mockReturnValue(cachePath)
storage.getDatabaseDir.mockReturnValue(cacheRoot)
})

afterAll(async () => {
await storage.closeDatabase()
await fs.remove(cachePath)
await fs.remove(cacheRoot)
delete global.__GATSBY

// Clean up after all tests are done, preventing this
// interception layer from affecting irrelevant tests.
server.close()
})

beforeEach(() => {
beforeEach(async () => {
// simulate a new build each run
global.__GATSBY = {
buildId: global.__GATSBY?.buildId
Expand All @@ -260,6 +260,9 @@ describe(`fetch-remote-file`, () => {
gotStream.mockClear()
fsMove.mockClear()
urlCount.clear()

await fs.remove(cachePath)
await fs.ensureDir(cachePath)
})

it(`downloads and create a svg file`, async () => {
Expand Down Expand Up @@ -359,6 +362,28 @@ describe(`fetch-remote-file`, () => {
global.__GATSBY = currentGlobal
})

it(`handles 304 responses correctly when file does not exists`, async () => {
const currentGlobal = global.__GATSBY
global.__GATSBY = { buildId: `304-3` }
const filePath = await fetchRemoteFile({
url: `http://external.com/dog-304.jpg`,
directory: cachePath,
})

await fs.remove(filePath)

global.__GATSBY = { buildId: `304-4` }
const filePathCached = await fetchRemoteFile({
url: `http://external.com/dog-304.jpg`,
directory: cachePath,
})

expect(filePathCached).toBe(filePath)
expect(fsMove).toBeCalledTimes(2)
expect(gotStream).toBeCalledTimes(2)
global.__GATSBY = currentGlobal
})

it(`fails when 404 is triggered`, async () => {
await expect(
fetchRemoteFile({
Expand Down
21 changes: 11 additions & 10 deletions packages/gatsby-core-utils/src/fetch-remote-file.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fileType from "file-type"
import path from "path"
import fs from "fs-extra"
import fs, { pathExists } from "fs-extra"
import Queue from "fastq"
import { createContentDigest } from "./create-content-digest"
import {
Expand Down Expand Up @@ -148,14 +148,6 @@ async function fetchFile({

const cachedEntry = await storage.remoteFileInfo.get(url)

// See if there's response headers for this url
// from a previous request.
const headers = { ...httpHeaders }
if (cachedEntry?.headers?.etag) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
headers[`If-None-Match`] = cachedEntry.headers.etag
}

// Add htaccess authentication if passed in. This isn't particularly
// extensible. We should define a proper API that we validate.
const httpOptions: Options = {}
Expand All @@ -176,14 +168,23 @@ async function fetchFile({
await fs.ensureDir(path.join(fileDirectory, digest))

const tmpFilename = createFilePath(fileDirectory, `tmp-${digest}`, ext)
const filename = createFilePath(path.join(fileDirectory, digest), name, ext)

// See if there's response headers for this url
// from a previous request.
const headers = { ...httpHeaders }
if (cachedEntry?.headers?.etag && (await pathExists(filename))) {
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
headers[`If-None-Match`] = cachedEntry.headers.etag
}

const response = await requestRemoteNode(
url,
headers,
tmpFilename,
httpOptions
)

const filename = createFilePath(path.join(fileDirectory, digest), name, ext)
if (response.statusCode === 200) {
// Save the response headers for future requests.
// If the user did not provide an extension and we couldn't get one from remote file, try and guess one
Expand Down

0 comments on commit 1f3eee0

Please sign in to comment.