fix(build-tools): surface real exceptions from done-file generation#27376
fix(build-tools): surface real exceptions from done-file generation#27376tylerbutler wants to merge 3 commits into
Conversation
|
Hi! Thank you for opening this PR. Want me to review it? Based on the diff (62 lines, 2 files), I've queued these reviewers:
How this works
|
There was a problem hiding this comment.
Pull request overview
This PR improves fluid-build incremental-cache diagnostics by ensuring exceptions thrown during done-file generation/comparison surface meaningful details (including underlying error info), rather than being swallowed and only causing opaque “non-incremental” behavior.
Changes:
- Enhance
LeafWithDoneFileTask.markExecDone()warning output to include both generation/write context and (when available) the error stack. - Improve
LeafWithDoneFileTask.checkLeafIsUpToDate()failure reporting by capturing the thrown error and handling expectedENOENT(first-run) cases separately. - Stop silently converting failures into
undefinedin done-file content generators by rethrowing fromLeafWithFileStatDoneFileTaskandTscDependentTask, letting the outer handler report the real cause.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| build-tools/packages/build-tools/src/fluidBuild/tasks/leaf/leafTask.ts | Surfaces underlying exceptions for done-file read/compare/generation (incl. stack) and special-cases missing done files. |
| build-tools/packages/build-tools/src/fluidBuild/tasks/leaf/tscTask.ts | Improves error detail surfaced when TypeScript version lookup fails and rethrows done-file generation failures for dependent tasks. |
🔭 PR Review Fleet ReportNote This report is generated by an experimental AI review fleet and is provided as a beta feature. Findings are a starting point for discussion, not a gate. Use your own judgement. Verdict: 0 Alert, 0 Stop, 1 Caution Findings
|
…IsUpToDate Previously, errors thrown by getDoneFileContent() (e.g. ENOENT from a missing input or output file) were caught by the same handler that treats ENOENT on the done file as an expected first-run condition. This could mislead users with a 'done file not found' trace even when the done file existed and the real failure was elsewhere. Split the try block so getDoneFileContent() has its own error handling and the ENOENT special case only applies to readFile of the done file.
Description
Improves error reporting in
fluid-build's incremental cache machinery so that exceptions thrown while computing or comparing "done file" content surface the real cause instead of being silently swallowed.Motivation
LeafWithDoneFileTask.getDoneFileContent()is the per-task hook that produces the cache key used to decide whether a task is up-to-date. Until now, several implementations and themarkExecDone/checkLeafIsUpToDatecallers wrapped this in a broadcatchthat converted any exception into eitherundefinedor a generic"unable to read compare file"trace — discarding the underlying error message and stack.In practice this means a buggy tool, a missing dependency, or a ReferenceError in a task implementation manifests only as:
…and an entire run gets marked non-incremental, with no way to find the root cause short of editing the source to add
console.logs.This change:
LeafWithDoneFileTask.markExecDone— when generation/write throws, log the error message and stack, and rename the warning to "unable to generate or write done file" (it covered both, but only mentioned writing).LeafWithDoneFileTask.checkLeafIsUpToDate— bind the caught error and include it in the trigger trace. Special-casesENOENT(expected on a fresh build) so first-run output stays quiet.LeafWithFileStatDoneFileTask.getDoneFileContentandgetHashDoneFileContent— re-throw exceptions instead of returningundefined. The outermarkExecDonehandler will format and log them.TscDependentTask.getDoneFileContent(api-extractor / eslint / generate-entrypoints / prettier) — same: re-throw instead of swallow.TscTask.checkTsBuildInfoVersionSame— include the underlying error in the existing "Unable to get installed package version for typescript" trace.Validation
Reproduced the original silent failure mode (a
ReferenceError: require is not definedinsidegetInstalledPackageVersion) before this change. With the change, the same defect produces:No
DEBUG=fluid-build:task:errorrequired.From
build-tools/packages/build-tools:pnpm run tsc— cleanpnpm run eslint— cleanpnpm test:mocha— 137/137 passingReviewer Guidance
The review process is outlined on this wiki page.
Behavioral change in the unhappy path only. Successful builds emit no new output. Failed/non-incremental tasks now print the underlying error and stack on stderr.