fix(gatsby): try to automatically recover when parcel segfaults #38773

pieh · 2023-12-20T14:30:09Z

Description

Seems like ARM macs parcel (at least version we use) cause quite a bit of problems and results in often seg faults - this tries to run parcel in child process so we can try to handle segfaults by clearing just parcel cache and retrying (few times)

Documentation

Tests

Related Issues

Possibly fixes #38483

pieh · 2023-12-20T14:31:53Z

packages/gatsby/src/utils/parcel/compile-gatsby-files.ts

-    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()


we are killing child process so this should be done automatically on process exits

pieh · 2023-12-20T15:08:26Z

.jestSetup.js

+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,
+  }
+})


This is just a trick for unit tests to be able to use source TS (and ESM) files when using gatsby-worker / child process setup

In actual usage TS source files become CJS and those will be used (all e2e tests would be using that for example)

This is added in this PR and as global mock because compile-gatsby-files is being directly and indrectly used in quite a bit of unit tests, so to avoid hunting for all indirect usage we just allow that to happen any time gatsby-worker is used within those tests

pieh · 2023-12-21T08:34:43Z

packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts

+                "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",


there is improvements to be made here to point out specific location as error we get is like this:

{ diagnostics: [ { message: "Expected ';', '}' or <eof>", codeFrames: [ { filePath: '/Users/misiek/dev/pgs-gatsby/packages/gatsby/src/utils/parcel/__tests__/fixtures/error-in-code-ts/gatsby-node.ts', codeHighlights: [ { message: 'This is the expression part of an expression statement', start: { line: 5, column: 1 }, end: { line: 5, column: 4 } }, { message: null, start: { line: 5, column: 6 }, end: { line: 5, column: 7 } } ] } ], hints: null, origin: '@parcel/transformer-js' } ] }

but it's out of scope of this PR - purpose here is to try to handle segfaults without introducing regression - this added tests is just just a test for case we didn't have (what's the behavior on ts files that are invalid) and that I did break before my most recent commit

laurogripa · 2023-12-27T09:40:53Z

Nice to know someone is working on this issue!

I'm trying to run your branch as a local version of Gatsby in my project because this issue makes it almost impossible for me to develop.
I added "gatsby": "file:../gatsby" to my package.json (the relative path is correct), the installation went okay but I'm getting this errors:

➜  erc55 git:(main) ✗ npm run develop
> erc55@0.2.15 develop
> gatsby develop
sh: gatsby: command not found

➜  erc55 git:(main) ✗ npx gatsby develop
npm ERR! Cannot read properties of undefined (reading 'replace')

Any suggestions on how to run your branch locally or should I just wait for it to get merged and published?

…l-from-parcel-seg-fault

pieh · 2024-01-02T11:44:47Z

@laurogripa I published canary version with those changes gatsby@5.14.0-alpha-fix-parcel-segfault.11 if you want to try it out

kathmbeck · 2024-01-08T16:19:21Z

packages/gatsby/src/utils/parcel/__tests__/fixtures/error-in-code-ts/gatsby-node.ts

+import { working } from "../utils/say-what-ts"
+import { createPages } from "../utils/create-pages-ts"
+
+this is wrong syntax that should't compile


Should this line be a comment?

Oh or is this there to fail the compilation 👍

Yeah, that's test fixture to test behavior when there is error in source file that should result in compilation problem and test itself is here

gatsby/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts

Lines 180 to 210 in 31e7049

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,

},

],

}

`)

})

})

I did break this behavior when iterating on this code and noticed we didn't have tests for this case so I added it as part of this PR

#38773 (comment) this comment highlight opportunity to improve error message possibly, but this was just out of scope of the fix

* 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 (cherry picked from commit 0a80cd6)

…) (#38799) * 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 (cherry picked from commit 0a80cd6) Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>

fix(gatsby): try to automatically recover when parcel segfauls

0b5550f

gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Dec 20, 2023

pieh commented Dec 20, 2023

View reviewed changes

test: make gatsby-worker test adjustment global

be2dfe4

pieh added topic: TypeScript Issues and PRs related to TS in general, public typings or gatsby-plugin-typescript and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Dec 20, 2023

pieh commented Dec 20, 2023

View reviewed changes

pieh changed the title ~~fix(gatsby): try to automatically recover when parcel segfauls~~ fix(gatsby): try to automatically recover when parcel segfaults Dec 20, 2023

pieh marked this pull request as ready for review December 21, 2023 06:24

pieh force-pushed the fix/try-to-self-heal-from-parcel-seg-fault branch from 2ac6db2 to 3e72735 Compare December 21, 2023 08:31

pieh commented Dec 21, 2023

View reviewed changes

fix: handle actual compilation errors

6360181

pieh force-pushed the fix/try-to-self-heal-from-parcel-seg-fault branch from 3e72735 to 6360181 Compare December 21, 2023 08:47

test: bump timeout for windows

e21fb92

pieh added 2 commits January 2, 2024 12:08

init bundles array so TS is happy

a5320af

Merge remote-tracking branch 'origin/master' into fix/try-to-self-hea…

31e7049

…l-from-parcel-seg-fault

kathmbeck reviewed Jan 8, 2024

View reviewed changes

kathmbeck approved these changes Jan 8, 2024

View reviewed changes

pieh merged commit 0a80cd6 into master Jan 9, 2024
33 checks passed

pieh deleted the fix/try-to-self-heal-from-parcel-seg-fault branch January 9, 2024 08:55

pieh added this to To cherry-pick in V5 Release hotfixes via automation Jan 9, 2024

gatsbybot mentioned this pull request Jan 9, 2024

fix(gatsby): try to automatically recover when parcel segfaults (#38773) #38799

Merged

pieh moved this from To cherry-pick to Backport PR opened in V5 Release hotfixes Jan 9, 2024

pieh moved this from Backport PR opened to Backported in V5 Release hotfixes Jan 10, 2024

pieh moved this from Backported to Published in V5 Release hotfixes Jan 23, 2024

Rutam21 mentioned this pull request Jan 23, 2024

[Snyk] Security upgrade gatsby from 2.32.13 to 5.13.2 Rutam21/gatsby#1774

Open

nidhi42 mentioned this pull request May 7, 2024

[Snyk] Security upgrade gatsby-source-contentful from 2.3.56 to 8.13.2 nidhi42/gatsby#3452

Open

This was referenced May 7, 2024

[Snyk] Upgrade gatsby from 5.0.0 to 5.13.4 X-oss-byte/gatsby-1#709

Open

[Snyk] Upgrade gatsby from 5.12.4 to 5.13.4 X-oss-byte/gatsby-1#710

Open

monizb mentioned this pull request May 8, 2024

[Snyk] Security upgrade gatsby-source-contentful from 2.3.56 to 8.13.2 monizb/gatsby#1452

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(gatsby): try to automatically recover when parcel segfaults #38773

fix(gatsby): try to automatically recover when parcel segfaults #38773

pieh commented Dec 20, 2023

pieh Dec 20, 2023

pieh Dec 20, 2023 •

edited

pieh Dec 20, 2023

pieh Dec 21, 2023

laurogripa commented Dec 27, 2023

pieh commented Jan 2, 2024

kathmbeck Jan 8, 2024

kathmbeck Jan 8, 2024

pieh Jan 9, 2024

	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,
	},
	],
	}
	`)
	})
	})

fix(gatsby): try to automatically recover when parcel segfaults #38773

fix(gatsby): try to automatically recover when parcel segfaults #38773

Conversation

pieh commented Dec 20, 2023

Description

Documentation

Tests

Related Issues

pieh Dec 20, 2023

Choose a reason for hiding this comment

pieh Dec 20, 2023 • edited

Choose a reason for hiding this comment

pieh Dec 20, 2023

Choose a reason for hiding this comment

pieh Dec 21, 2023

Choose a reason for hiding this comment

laurogripa commented Dec 27, 2023

pieh commented Jan 2, 2024

kathmbeck Jan 8, 2024

Choose a reason for hiding this comment

kathmbeck Jan 8, 2024

Choose a reason for hiding this comment

pieh Jan 9, 2024

Choose a reason for hiding this comment

pieh Dec 20, 2023 •

edited