Skip to content

Commit

Permalink
fix(gatsby): try to automatically recover when parcel segfaults (#38773)
Browse files Browse the repository at this point in the history
* fix(gatsby): try to automatically recover when parcel segfauls

* test: make gatsby-worker test adjustment global

* fix: handle actual compilation errors

* test: bump timeout for windows

* init bundles array so TS is happy
  • Loading branch information
pieh authored Jan 9, 2024
1 parent c85246e commit 0a80cd6
Show file tree
Hide file tree
Showing 4 changed files with 173 additions and 25 deletions.
29 changes: 26 additions & 3 deletions .jestSetup.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,30 @@ if (
typeof globalThis.TextEncoder === "undefined" ||
typeof globalThis.TextDecoder === "undefined"
) {
const utils = require("util");
globalThis.TextEncoder = utils.TextEncoder;
globalThis.TextDecoder = utils.TextDecoder;
const utils = require("util")
globalThis.TextEncoder = utils.TextEncoder
globalThis.TextDecoder = utils.TextDecoder
}

jest.mock(`gatsby-worker`, () => {
const gatsbyWorker = jest.requireActual(`gatsby-worker`)

const { WorkerPool: OriginalWorkerPool } = gatsbyWorker

class WorkerPoolThatCanUseTS extends OriginalWorkerPool {
constructor(workerPath, options) {
options.env = {
...(options.env ?? {}),
NODE_OPTIONS: `--require ${require.resolve(
`./packages/gatsby/src/utils/worker/__tests__/test-helpers/ts-register.js`
)}`,
}
super(workerPath, options)
}
}

return {
...gatsbyWorker,
WorkerPool: WorkerPoolThatCanUseTS,
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ const dir = {
misnamedJS: `${__dirname}/fixtures/misnamed-js`,
misnamedTS: `${__dirname}/fixtures/misnamed-ts`,
gatsbyNodeAsDirectory: `${__dirname}/fixtures/gatsby-node-as-directory`,
errorInCode: `${__dirname}/fixtures/error-in-code-ts`,
}

jest.setTimeout(15000)
jest.setTimeout(60_000)

jest.mock(`@parcel/core`, () => {
const parcelCore = jest.requireActual(`@parcel/core`)
Expand Down Expand Up @@ -175,6 +176,37 @@ describe(`gatsby file compilation`, () => {
})
})
})

it(`handles errors in TS code`, async () => {
process.chdir(dir.errorInCode)
await remove(`${dir.errorInCode}/.cache`)
await compileGatsbyFiles(dir.errorInCode)

expect(reporterPanicMock).toMatchInlineSnapshot(`
[MockFunction] {
"calls": Array [
Array [
Object {
"context": Object {
"filePath": "<PROJECT_ROOT>/gatsby-node.ts",
"generalMessage": "Expected ';', '}' or <eof>",
"hints": null,
"origin": "@parcel/transformer-js",
"specificMessage": "This is the expression part of an expression statement",
},
"id": "11901",
},
],
],
"results": Array [
Object {
"type": "return",
"value": undefined,
},
],
}
`)
})
})

describe(`gatsby-node directory is allowed`, () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { GatsbyNode } from "gatsby"
import { working } from "../utils/say-what-ts"
import { createPages } from "../utils/create-pages-ts"

this is wrong syntax that should't compile
export const onPreInit: GatsbyNode["onPreInit"] = ({ reporter }) => {
reporter.info(working)
}
type Character = {
id: string
name: string
}
export const sourceNodes: GatsbyNode["sourceNodes"] = async ({ actions, createNodeId, createContentDigest }) => {
const { createNode } = actions
let characters: Array<Character> = [
{
id: `0`,
name: `A`
},
{
id: `1`,
name: `B`
}
]
characters.forEach((character: Character) => {
const node = {
...character,
id: createNodeId(`characters-${character.id}`),
parent: null,
children: [],
internal: {
type: 'Character',
content: JSON.stringify(character),
contentDigest: createContentDigest(character),
},
}

createNode(node)
})
}

export { createPages }
88 changes: 67 additions & 21 deletions packages/gatsby/src/utils/parcel/compile-gatsby-files.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { LMDBCache, Cache } from "@parcel/cache"
import path from "path"
import type { Diagnostic } from "@parcel/diagnostic"
import reporter from "gatsby-cli/lib/reporter"
import { WorkerPool } from "gatsby-worker"
import { ensureDir, emptyDir, existsSync, remove, readdir } from "fs-extra"
import telemetry from "gatsby-telemetry"
import { isNearMatch } from "../is-near-match"
Expand Down Expand Up @@ -52,6 +53,28 @@ export function constructParcel(siteRoot: string, cache?: Cache): Parcel {
})
}

interface IProcessBundle {
filePath: string
mainEntryPath?: string
}

type RunParcelReturn = Array<IProcessBundle>

export async function runParcel(siteRoot: string): Promise<RunParcelReturn> {
const cache = new LMDBCache(getCacheDir(siteRoot)) as unknown as Cache
const parcel = constructParcel(siteRoot, cache)
const { bundleGraph } = await parcel.run()
const bundles = bundleGraph.getBundles()
// bundles is not serializable, so we need to extract the data we need
// so it crosses IPC boundaries
return bundles.map(bundle => {
return {
filePath: bundle.filePath,
mainEntryPath: bundle.getMainEntry()?.filePath,
}
})
}

/**
* Compile known gatsby-* files (e.g. `gatsby-config`, `gatsby-node`)
* and output in `<SITE_ROOT>/.cache/compiled`.
Expand Down Expand Up @@ -107,33 +130,59 @@ export async function compileGatsbyFiles(
})
}

const worker = new WorkerPool<typeof import("./compile-gatsby-files")>(
require.resolve(`./compile-gatsby-files`),
{
numWorkers: 1,
}
)

const distDir = `${siteRoot}/${COMPILED_CACHE_DIR}`
await ensureDir(distDir)
await emptyDir(distDir)

await exponentialBackoff(retry)

// for whatever reason TS thinks LMDBCache is some browser Cache and not actually Parcel's Cache
// so we force type it to Parcel's Cache
const cache = new LMDBCache(getCacheDir(siteRoot)) as unknown as Cache
const parcel = constructParcel(siteRoot, cache)
const { bundleGraph } = await parcel.run()
let cacheClosePromise = Promise.resolve()
let bundles: RunParcelReturn = []
try {
// @ts-ignore store is public field on LMDBCache class, but public interface for Cache
// doesn't have it. There doesn't seem to be proper public API for this, so we have to
// resort to reaching into internals. Just in case this is wrapped in try/catch if
// parcel changes internals in future (closing cache is only needed when retrying
// so the if the change happens we shouldn't fail on happy builds)
cacheClosePromise = cache.store.close()
} catch (e) {
reporter.verbose(`Failed to close parcel cache\n${e.toString()}`)
// sometimes parcel segfaults which is not something we can recover from, so we run parcel
// in child process and IF it fails we try to delete parcel's cache (this seems to "fix" the problem
// causing segfaults?) and retry few times
// not ideal, but having gatsby segfaulting is really frustrating and common remedy is to clean
// entire .cache for users, which is not ideal either especially when we can just delete parcel's cache
// and to recover automatically
bundles = await worker.single.runParcel(siteRoot)
} catch (error) {
if (error.diagnostics) {
handleErrors(error.diagnostics)
return
} else if (retry >= RETRY_COUNT) {
reporter.panic({
id: `11904`,
error,
context: {
siteRoot,
retries: RETRY_COUNT,
sourceMessage: error.message,
},
})
} else {
await exponentialBackoff(retry)
try {
await remove(getCacheDir(siteRoot))
} catch {
// in windows we might get "EBUSY" errors if LMDB failed to close, so this try/catch is
// to prevent EBUSY errors from potentially hiding real import errors
}
await compileGatsbyFiles(siteRoot, retry + 1)
return
}
} finally {
worker.end()
}

await exponentialBackoff(retry)

const bundles = bundleGraph.getBundles()

if (bundles.length === 0) return

let compiledTSFilesCount = 0
Expand All @@ -150,7 +199,7 @@ export async function compileGatsbyFiles(
siteRoot,
retries: RETRY_COUNT,
compiledFileLocation: bundle.filePath,
sourceFileLocation: bundle.getMainEntry()?.filePath,
sourceFileLocation: bundle.mainEntryPath,
},
})
} else if (retry > 0) {
Expand All @@ -165,9 +214,6 @@ export async function compileGatsbyFiles(
)
}

// sometimes parcel cache gets in weird state and we need to clear the cache
await cacheClosePromise

try {
await remove(getCacheDir(siteRoot))
} catch {
Expand All @@ -179,7 +225,7 @@ export async function compileGatsbyFiles(
return
}

const mainEntry = bundle.getMainEntry()?.filePath
const mainEntry = bundle.mainEntryPath
// mainEntry won't exist for shared chunks
if (mainEntry) {
if (mainEntry.endsWith(`.ts`)) {
Expand Down

0 comments on commit 0a80cd6

Please sign in to comment.