-
Notifications
You must be signed in to change notification settings - Fork 10.3k
fix(gatsby): try to automatically recover when parcel segfaults #38773
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
Merged
Merged
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
0b5550f
fix(gatsby): try to automatically recover when parcel segfauls
pieh be2dfe4
test: make gatsby-worker test adjustment global
pieh 6360181
fix: handle actual compilation errors
pieh e21fb92
test: bump timeout for windows
pieh a5320af
init bundles array so TS is happy
pieh 31e7049
Merge remote-tracking branch 'origin/master' into fix/try-to-self-hea…
pieh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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<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`. | ||
|
|
@@ -107,33 +130,56 @@ 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() | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we are killing child process so this should be done automatically on process exits |
||
| // 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`)) { | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a trick for unit tests to be able to use source TS (and ESM) files when using
gatsby-worker/ child process setupIn actual usage TS source files become CJS and those will be used (all e2e tests would be using that for example)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is added in this PR and as global mock because
compile-gatsby-filesis 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 timegatsby-workeris used within those tests