From 0b5550f63d4d84ffa7d896400466b94e39beaa59 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Wed, 20 Dec 2023 15:23:43 +0100 Subject: [PATCH 1/5] fix(gatsby): try to automatically recover when parcel segfauls --- .../parcel/__tests__/compile-gatsby-files.ts | 23 +++++ .../src/utils/parcel/compile-gatsby-files.ts | 83 ++++++++++++++----- 2 files changed, 86 insertions(+), 20 deletions(-) diff --git a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts index 76ec8a9fabaf2..3ba9923613ac7 100644 --- a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts +++ b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts @@ -20,6 +20,29 @@ const dir = { gatsbyNodeAsDirectory: `${__dirname}/fixtures/gatsby-node-as-directory`, } +jest.mock(`gatsby-worker`, () => { + const gatsbyWorker = jest.requireActual(`gatsby-worker`) + + const { WorkerPool: OriginalWorkerPool } = gatsbyWorker + + class WorkerPoolThatCanUseTS extends OriginalWorkerPool { + constructor(workerPath: string, options: any) { + options.env = { + ...(options.env ?? {}), + NODE_OPTIONS: `--require ${require.resolve( + `../../worker/__tests__/test-helpers/ts-register.js` + )}`, + } + super(workerPath, options) + } + } + + return { + ...gatsbyWorker, + WorkerPool: WorkerPoolThatCanUseTS, + } +}) + jest.setTimeout(15000) jest.mock(`@parcel/core`, () => { diff --git a/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts b/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts index 5b71db2cee73b..eed6bb9106ea9 100644 --- a/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts +++ b/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts @@ -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" @@ -52,6 +53,28 @@ export function constructParcel(siteRoot: string, cache?: Cache): Parcel { }) } +interface IProcessBundle { + filePath: string + mainEntryPath?: string +} + +type RunParcelReturn = Array + +export async function runParcel(siteRoot: string): Promise { + 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 `/.cache/compiled`. @@ -107,33 +130,56 @@ export async function compileGatsbyFiles( }) } + const worker = new WorkerPool( + 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() + // 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 (e) { - reporter.verbose(`Failed to close parcel cache\n${e.toString()}`) + if (retry >= RETRY_COUNT) { + reporter.panic({ + id: `11904`, + error: e, + context: { + siteRoot, + retries: RETRY_COUNT, + sourceMessage: e.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 @@ -150,7 +196,7 @@ export async function compileGatsbyFiles( siteRoot, retries: RETRY_COUNT, compiledFileLocation: bundle.filePath, - sourceFileLocation: bundle.getMainEntry()?.filePath, + sourceFileLocation: bundle.mainEntryPath, }, }) } else if (retry > 0) { @@ -165,9 +211,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 { @@ -179,7 +222,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`)) { From be2dfe48137e810bcf39f7b5360b6db478647f27 Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Wed, 20 Dec 2023 15:49:29 +0100 Subject: [PATCH 2/5] test: make gatsby-worker test adjustment global --- .jestSetup.js | 29 +++++++++++++++++-- .../parcel/__tests__/compile-gatsby-files.ts | 23 --------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/.jestSetup.js b/.jestSetup.js index 60ffd7f15782d..7c86cf8eb3440 100644 --- a/.jestSetup.js +++ b/.jestSetup.js @@ -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, + } +}) diff --git a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts index 3ba9923613ac7..76ec8a9fabaf2 100644 --- a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts +++ b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts @@ -20,29 +20,6 @@ const dir = { gatsbyNodeAsDirectory: `${__dirname}/fixtures/gatsby-node-as-directory`, } -jest.mock(`gatsby-worker`, () => { - const gatsbyWorker = jest.requireActual(`gatsby-worker`) - - const { WorkerPool: OriginalWorkerPool } = gatsbyWorker - - class WorkerPoolThatCanUseTS extends OriginalWorkerPool { - constructor(workerPath: string, options: any) { - options.env = { - ...(options.env ?? {}), - NODE_OPTIONS: `--require ${require.resolve( - `../../worker/__tests__/test-helpers/ts-register.js` - )}`, - } - super(workerPath, options) - } - } - - return { - ...gatsbyWorker, - WorkerPool: WorkerPoolThatCanUseTS, - } -}) - jest.setTimeout(15000) jest.mock(`@parcel/core`, () => { From 63601815707c441c45d61fc60c14a1e69a63835b Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Thu, 21 Dec 2023 09:08:55 +0100 Subject: [PATCH 3/5] fix: handle actual compilation errors --- .../parcel/__tests__/compile-gatsby-files.ts | 32 +++++++++++++ .../fixtures/error-in-code-ts/gatsby-node.ts | 47 +++++++++++++++++++ .../src/utils/parcel/compile-gatsby-files.ts | 11 +++-- 3 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 packages/gatsby/src/utils/parcel/__tests__/fixtures/error-in-code-ts/gatsby-node.ts diff --git a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts index 76ec8a9fabaf2..e5e6cb92da849 100644 --- a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts +++ b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts @@ -18,6 +18,7 @@ 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) @@ -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": "/gatsby-node.ts", + "generalMessage": "Expected ';', '}' or ", + "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`, () => { diff --git a/packages/gatsby/src/utils/parcel/__tests__/fixtures/error-in-code-ts/gatsby-node.ts b/packages/gatsby/src/utils/parcel/__tests__/fixtures/error-in-code-ts/gatsby-node.ts new file mode 100644 index 0000000000000..a72e769a773d8 --- /dev/null +++ b/packages/gatsby/src/utils/parcel/__tests__/fixtures/error-in-code-ts/gatsby-node.ts @@ -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 = [ + { + 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 } diff --git a/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts b/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts index eed6bb9106ea9..2136d1c85b13d 100644 --- a/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts +++ b/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts @@ -152,15 +152,18 @@ export async function compileGatsbyFiles( // 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 (e) { - if (retry >= RETRY_COUNT) { + } catch (error) { + if (error.diagnostics) { + handleErrors(error.diagnostics) + return + } else if (retry >= RETRY_COUNT) { reporter.panic({ id: `11904`, - error: e, + error, context: { siteRoot, retries: RETRY_COUNT, - sourceMessage: e.message, + sourceMessage: error.message, }, }) } else { From e21fb92694b90bb4b5256d8586a3072e56100fdb Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Thu, 21 Dec 2023 12:56:44 +0100 Subject: [PATCH 4/5] test: bump timeout for windows --- .../gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts index e5e6cb92da849..def5de6064dce 100644 --- a/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts +++ b/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts @@ -21,7 +21,7 @@ const dir = { errorInCode: `${__dirname}/fixtures/error-in-code-ts`, } -jest.setTimeout(15000) +jest.setTimeout(60_000) jest.mock(`@parcel/core`, () => { const parcelCore = jest.requireActual(`@parcel/core`) From a5320aff636fe407dfb96482964efb1088afcbef Mon Sep 17 00:00:00 2001 From: Michal Piechowiak Date: Tue, 2 Jan 2024 12:08:46 +0100 Subject: [PATCH 5/5] init bundles array so TS is happy --- packages/gatsby/src/utils/parcel/compile-gatsby-files.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts b/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts index 2136d1c85b13d..6edde27b95f86 100644 --- a/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts +++ b/packages/gatsby/src/utils/parcel/compile-gatsby-files.ts @@ -143,7 +143,7 @@ export async function compileGatsbyFiles( await exponentialBackoff(retry) - let bundles: RunParcelReturn + let bundles: RunParcelReturn = [] try { // 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