Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(gatsby-core-utils): fix 304 when file does not exists #34842

Merged
merged 1 commit into from
Feb 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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