Skip to content

fix(build-tools): surface real exceptions from done-file generation#27376

Open
tylerbutler wants to merge 3 commits into
microsoft:mainfrom
tylerbutler:build-tools-error-reporting
Open

fix(build-tools): surface real exceptions from done-file generation#27376
tylerbutler wants to merge 3 commits into
microsoft:mainfrom
tylerbutler:build-tools-error-reporting

Conversation

@tylerbutler
Copy link
Copy Markdown
Member

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 the markExecDone / checkLeafIsUpToDate callers wrapped this in a broad catch that converted any exception into either undefined or 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:

@some/package: warning: unable to generate content for /path/to/foo.done.build.log

…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:

  1. 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).
  2. LeafWithDoneFileTask.checkLeafIsUpToDate — bind the caught error and include it in the trigger trace. Special-cases ENOENT (expected on a fresh build) so first-run output stays quiet.
  3. LeafWithFileStatDoneFileTask.getDoneFileContent and getHashDoneFileContent — re-throw exceptions instead of returning undefined. The outer markExecDone handler will format and log them.
  4. TscDependentTask.getDoneFileContent (api-extractor / eslint / generate-entrypoints / prettier) — same: re-throw instead of swallow.
  5. 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 defined inside getInstalledPackageVersion) before this change. With the change, the same defect produces:

@fluidframework/core-utils: warning: unable to generate or write done file /…/api-extractor-20f5d9ed.done.build.log
 error: ReferenceError: require is not defined
ReferenceError: require is not defined
    at getInstalledPackageVersion (file:///…/taskUtils.js:43:24)
    …

No DEBUG=fluid-build:task:error required.

From build-tools/packages/build-tools:

  • pnpm run tsc — clean
  • pnpm run eslint — clean
  • pnpm test:mocha — 137/137 passing

Reviewer 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

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:

  • Correctness — logic errors, race conditions, lifecycle issues
  • Security — vulnerabilities, secret exposure, injection
  • API Compatibility — breaking changes, release tags, type design
  • Performance — algorithmic regressions, memory leaks
  • Testing — coverage gaps, hollow tests

How this works

  • Adjust the reviewer set by ticking/unticking boxes above. Reviewer toggles alone don't trigger anything.

  • Tick Start review below to dispatch the review fleet.

  • After review finishes, tick Start review again to request another run — it auto-resets after each dispatch.

  • This comment updates as new commits land; your reviewer selections are preserved.

  • Start review

@tylerbutler tylerbutler marked this pull request as ready for review May 21, 2026 16:58
Copilot AI review requested due to automatic review settings May 21, 2026 16:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 expected ENOENT (first-run) cases separately.
  • Stop silently converting failures into undefined in done-file content generators by rethrowing from LeafWithFileStatDoneFileTask and TscDependentTask, 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.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 21, 2026

🔭 PR Review Fleet Report

Note

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: ⚠️ Approve with Suggestions

0 Alert, 0 Stop, 1 Caution

Findings

Sev # Area File What Fix
🚧 Caution M1 Correctness build-tools/packages/build-tools/src/fluidBuild/tasks/leaf/leafTask.ts:552-561 The checkLeafIsUpToDate catch block checks code === "ENOENT" to distinguish an expected "done file not found on first run" from other errors, and logs done file not found: ${doneFileFullPath}. Before this PR, getDoneFileContent() always returned undefined on error — meaning ENOENT in that catch could only come from readFile(doneFileFullPath, 'utf8') on line 537. After this PR, getDoneFileContent() now throws on error (e.g. when stat() or getFileHash() fails on a missing input or output file). If an input/output file is absent, stat() throws ENOENT, that error propagates through getDoneFileContent(), and is caught here. The code === "ENOENT" branch fires and logs done file not found: ${doneFileFullPath} — but doneFileFullPath names the done file, not the file that is actually missing. A user seeing this trace while the done file actually exists on disk would be misled about what is wrong. In the worst case (first build, output files haven't been created yet), this mis-attribution could mask a broken input-file configuration because the message looks like a routine "first run" trace rather than a real problem. Split the try block so that getDoneFileContent() and readFile(doneFileFullPath) have separate error handling, or re-throw errors that originate from getDoneFileContent() before reaching the ENOENT check. For example: compute doneFileExpectedContent outside the outer try, catch only its errors separately, then wrap just the readFile call in the try that does the ENOENT check. This preserves the original intent of suppressing the noisy "first run" ENOENT while still surfacing real errors with accurate messages.

View workflow run

…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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants