From 4491977dc9b99910fdde43e3cd3951e30c8fcaf7 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:00 +0200 Subject: [PATCH 01/16] refactor(project): Handle recoverable build errors without crash Classify caught task errors into three branches: abort, transient (a concurrent source change), and normal. Transient errors re-queue the affected projects and keep held reader requests open so they resolve on the retry. Normal errors report via ServeLogger. The "error" event stays reserved for fatal failures like a watcher crash, so the process still exits on those. --- packages/project/lib/build/BuildServer.js | 32 ++++++++++++++++--- .../test/lib/build/BuildServer.integration.js | 9 +++--- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 22cc4bd873a..745a36d5d6a 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -440,8 +440,10 @@ class BuildServer extends EventEmitter { // skipped without needing a second guard in #scheduleBackgroundValidation. this.#reconcileServerState({hrtime, mayValidate: true}); }).catch((err) => { + // Reached only for unexpected failures outside the per-build catch inside + // #processBuildRequests (which handles task errors itself). Surface via + // ServeLogger; keep the server alive. this.#setState(SERVER_STATES.ERROR, {error: err}); - this.emit("error", err); }); }, 10); } @@ -505,16 +507,33 @@ class BuildServer extends EventEmitter { this.#pendingBuildRequest.add(projectName); } } + } else if (signal.aborted || this.#resourceChangeQueue.size > 0) { + // Task threw while sources were changing (e.g. mid-`git pull`). The build state + // is untrustworthy but the error itself is very likely spurious — a fresh build + // against the settled tree will typically succeed. Silently re-queue affected + // projects and leave the reader-request queue intact so held requests resolve + // on the retry. + log.warn( + `Build failed during concurrent source change — treating as transient: ${err.message}`); + for (const projectName of projectsToBuild) { + const projectBuildStatus = this.#projectBuildStatus.get(projectName); + if (!projectBuildStatus.isFresh()) { + this.#pendingBuildRequest.add(projectName); + } + } } else { log.error(`Build failed: ${err.message}`); + if (err?.stack) { + log.verbose(err.stack); + } // Build failed - reject promises for projects that weren't built for (const projectName of projectsToBuild) { const projectBuildStatus = this.#projectBuildStatus.get(projectName); projectBuildStatus.rejectReaderRequests(err); } - // Capture the error for emission below; do NOT re-throw so the queue keeps processing - // and #activeBuild is cleared. Subsequent requests for affected projects will re-enqueue - // builds via #getReaderForProject. + // Capture the error so the outer loop can surface it via ServeLogger without + // re-throwing, keeping the queue alive so future requests re-enqueue builds + // via #getReaderForProject. buildError = err; } }); @@ -528,7 +547,10 @@ class BuildServer extends EventEmitter { } if (buildError) { this.#setState(SERVER_STATES.ERROR, {error: buildError}); - this.emit("error", buildError); + // Surfaced via ServeLogger.serveError from #setState. The "error" event + // stays reserved for fatal, non-recoverable failures (e.g. watcher crash); + // build errors keep the server alive so the user can fix the source and + // trigger a rebuild. // Continue processing any remaining pending requests for unaffected projects. continue; } diff --git a/packages/project/test/lib/build/BuildServer.integration.js b/packages/project/test/lib/build/BuildServer.integration.js index 0abcf7765a9..233890977ea 100644 --- a/packages/project/test/lib/build/BuildServer.integration.js +++ b/packages/project/test/lib/build/BuildServer.integration.js @@ -822,8 +822,8 @@ test.serial("Serve application.a with --cache=Force (2)", async (t) => { // Regression: a non-abort build error used to leave #activeBuild set, deadlocking the BuildServer // so subsequent resource requests would hang forever. The fix in #processBuildRequests clears -// #activeBuild in a finally block and emits "error" instead of throwing — verify a second request -// still rejects (with the same root cause) instead of hanging. +// #activeBuild in a finally block and surfaces the error via ServeLogger instead of throwing — +// verify a second request still rejects (with the same root cause) instead of hanging. test.serial("Build server recovers from non-abort build error (no deadlock)", async (t) => { const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a"); @@ -849,9 +849,10 @@ test.serial("Build server recovers from non-abort build error (no deadlock)", as `Second request rejects with the same error instead of deadlocking. Got: ${secondError && secondError.message}` ); - // Each failed build emits exactly one "error" event + // Build errors are surfaced via ServeLogger, not via the "error" event — the latter stays + // reserved for fatal failures (watcher crash, etc.) that must terminate the server. await setTimeout(50); - t.is(errorEvents.length, 2, "Two build errors were emitted, one per failed build attempt"); + t.is(errorEvents.length, 0, "No fatal 'error' events emitted for recoverable build failures"); }); // ProjectBuildCache's StageCache must be cleared correctly when a build is aborted. From 2ca5658eb718b7e3a7c8dd9adbf087f1cbf588a2 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:02 +0200 Subject: [PATCH 02/16] refactor(project): Gate rebuilds on ERRORED and clear on graph activity A failed build latches its projects into a new ERRORED state and short-circuits #getReaderForProject with the captured error. An unchanged rebuild would just reproduce the failure, so hold the gate until real signal arrives. Three signals lift it: - Source change on the project itself: invalidate() already clears #lastError. - Source change on a dependent: _projectResourceChanged invalidate()s each dependent, lifting ERRORED as a side effect. - Source change on a transitive dependency, or any successful build cycle: both call the new ProjectBuildStatus.clearError() (ERRORED -> INVALIDATED, drops #lastError without touching the reader or aborting a build). Without this a request resolving through a gated dependency would replay the error forever. Correct the banner for the error path: _projectResourceChanged transitions ERROR -> STALE, #processBuildRequests re-signals BUILDING per iteration so retry progress replaces the red state, #getStaleProjectNames excludes ERRORED projects, and the post-cycle transition preserves ERROR for projects that stay gated. Add an integration test for the gate plus four unit tests: a dependent's change lifts its dependency's gate, a successful cycle lifts unrelated gates, no signal keeps the gate closed, and a change on the ERRORED project lifts its own gate. --- packages/project/lib/build/BuildServer.js | 133 ++++++- .../test/lib/build/BuildServer.integration.js | 34 ++ .../project/test/lib/build/BuildServer.js | 340 ++++++++++++++++++ 3 files changed, 499 insertions(+), 8 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 745a36d5d6a..4b42f0126e3 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -263,6 +263,16 @@ class BuildServer extends EventEmitter { if (projectBuildStatus.isFresh()) { return projectBuildStatus.getReader(); } + + // Last build failed and nothing has changed since. Rebuilding would re-produce the same + // error (builds are deterministic), so short-circuit with the captured error. The gate + // lifts as soon as a source change in this project or one of its (transitive) dependencies + // invalidates the status via #_projectResourceChanged. + const lastError = projectBuildStatus.getError(); + if (lastError) { + throw lastError; + } + const {promise, resolve, reject} = Promise.withResolvers(); // Always queue the request on the status. It owns the "who resolves this" // contract: a running validation pass drains its own queue via setReader @@ -343,6 +353,24 @@ class BuildServer extends EventEmitter { }); } + // Lift ERRORED gates on the changed project's transitive dependencies too. + // #getReaderForProject short-circuits ERRORED projects with the captured error, + // so a request for e.g. library.a would keep returning HTTP 500 until library.a + // itself sees a file change — even after every dependent has rebuilt cleanly. + // The change here isn't proof that the dependency's inputs shifted, but it IS + // user activity: retry with a real build instead of replaying the old error. + // Kept narrow (transitive deps only, not the whole graph): a change on an + // unrelated sibling project shouldn't clear a genuinely-failing dep either. + // The broader lift lives in #clearErroredGatesAfterSuccessfulBuild, invoked + // when a build cycle completes successfully. + for (const depName of this.#graph.getTransitiveDependencies(project.getName())) { + const depStatus = this.#projectBuildStatus.get(depName); + if (depStatus?.clearError()) { + log.verbose(`Lifted ERRORED gate on dependency '${depName}' ` + + `after source change in dependent project '${project.getName()}'`); + } + } + // Enqueue resource change for processing before next build const queuedChanges = this.#resourceChangeQueue.get(project.getName()); if (queuedChanges) { @@ -351,13 +379,17 @@ class BuildServer extends EventEmitter { this.#resourceChangeQueue.set(project.getName(), new Set([filePath])); } - // If the server is currently idle, surface the new STALE state right away. - // During VALIDATING, the same shortcut applies: the change is already registered - // here, so reporting VALIDATING any longer would mislead consumers. The validation - // pass's finally clause guards on `#serverState === VALIDATING` and bails when the - // state has moved on, so there's no double-transition risk. + // Surface the new STALE state right away from any "quiet" state. + // IDLE and ERROR are both quiet — ERROR is sticky, but a change lifts it because + // the input tree has moved and the previous failure isn't the final word anymore. + // VALIDATING is quiet in the same sense: the change is already registered here, so + // reporting VALIDATING any longer would mislead consumers. The validation pass's + // finally clause guards on `#serverState === VALIDATING` and bails when the state + // has moved on, so there's no double-transition risk. + // BUILDING already implies progress, so don't disturb it. if (this.#serverState === SERVER_STATES.IDLE || - this.#serverState === SERVER_STATES.VALIDATING) { + this.#serverState === SERVER_STATES.VALIDATING || + this.#serverState === SERVER_STATES.ERROR) { this.#setState(SERVER_STATES.STALE); } @@ -460,6 +492,12 @@ class BuildServer extends EventEmitter { async #processBuildRequests() { // Process queue while there are pending requests while (this.#pendingBuildRequest.size > 0) { + // Each iteration is a fresh build attempt — ensure the state machine + // reflects that even on re-entries after a transient failure or a + // normal error whose queue survived. #setState is a no-op when + // already BUILDING, so the initial-entry case (state was set by + // #triggerRequestQueue) is unaffected. + this.#setState(SERVER_STATES.BUILDING); // Collect all pending projects for this batch const projectsToBuild = Array.from(this.#pendingBuildRequest); let buildRootProject = false; @@ -564,13 +602,43 @@ class BuildServer extends EventEmitter { this.#triggerRequestQueue(); return; } + // A successful build cycle proves the environment can build. Lift ERRORED gates on any + // *other* project (the graph-local case is handled in _projectResourceChanged; this + // broadens it to unrelated projects a dependent-change never reaches). The next reader + // request for such a project re-enqueues a real build instead of replaying its captured + // error indefinitely. + this.#clearErroredGatesAfterSuccessfulBuild(projectsToBuild); + } + } + + /** + * Sweep all project statuses and lift any lingering ERRORED gates that a + * successful build cycle has effectively refuted. The freshly-built projects + * themselves are FRESH at this point (or, if invalidated mid-cycle, back to + * INVALIDATED via setReader's short-circuit) — they can't be ERRORED, so the + * sweep is a no-op for them. + * + * @param {string[]} justBuiltProjects Names of projects the completed cycle + * built. Logged for the verbose trace; not otherwise consulted. + */ + #clearErroredGatesAfterSuccessfulBuild(justBuiltProjects) { + for (const [name, status] of this.#projectBuildStatus) { + if (status.clearError()) { + log.verbose(`Lifted ERRORED gate on '${name}' after successful build of ` + + `${justBuiltProjects.join(", ")}`); + } } } #getStaleProjectNames() { + // "Stale" here means "needs a rebuild if requested" — invalidated projects + // that the next request will re-enqueue. Errored projects are NOT stale in + // that sense: their rebuild is gated until the input changes, so surfacing + // them as stale would understate the situation (the user needs to fix the + // error, not just wait for the next request). const stale = []; for (const [name, status] of this.#projectBuildStatus) { - if (!status.isFresh()) { + if (!status.isFresh() && !status.getError()) { stale.push(name); } } @@ -830,6 +898,13 @@ const PROJECT_STATES = Object.freeze({ VALIDATING: "validating", BUILDING: "building", FRESH: "fresh", + // Last build failed with a non-transient error. Held in this state until + // something invalidates the project — either a direct source change or a + // change in a (transitive) dependency, both of which route through + // #_projectResourceChanged → invalidate(). This gate prevents deterministic + // rebuild loops on failing builds: repeat requests reject immediately with + // the captured error until the input actually changes. + ERRORED: "errored", }); class ProjectBuildStatus { @@ -838,6 +913,7 @@ class ProjectBuildStatus { #reader; #abortController = new AbortController(); #onBuildRequired; + #lastError = null; /** * @param {Function} [onBuildRequired] Invoked when the status leaves the VALIDATING @@ -869,6 +945,10 @@ class ProjectBuildStatus { // a stale reader must not survive a file add/remove. this.#reader = null; } + // Any invalidation lifts the ERRORED gate — the input changed, so a + // fresh build has a chance of succeeding even if the previous one + // failed deterministically. + this.#lastError = null; if (this.#state === PROJECT_STATES.INVALIDATED) { return; } @@ -959,6 +1039,39 @@ class ProjectBuildStatus { return this.#state === PROJECT_STATES.VALIDATING; } + /** + * Returns the captured error when the project is gated in ERRORED state, else + * null. Callers should use this to short-circuit rebuild attempts + * when the input hasn't changed since the failure. + */ + getError() { + return this.#state === PROJECT_STATES.ERRORED ? this.#lastError : null; + } + + /** + * Lifts the ERRORED gate without asserting that this project's input tree + * changed — used by the {@link BuildServer} when a source change or a + * successful build elsewhere in the graph signals that the earlier failure + * may have been environmental rather than deterministic. The project drops + * back to INVALIDATED so the next reader request re-enqueues a real build. + * + * No-op unless the project is currently ERRORED. Does not abort any in-flight + * build (there can't be one; ERRORED is a terminal cycle-end state) and does + * not touch the cached reader (which is always null in ERRORED, + * since a failed build never called setReader). + * + * @returns {boolean} True if the gate was lifted, false if the project was + * not in ERRORED to begin with. + */ + clearError() { + if (this.#state !== PROJECT_STATES.ERRORED) { + return false; + } + this.#lastError = null; + this.#state = PROJECT_STATES.INVALIDATED; + return true; + } + getReader() { return this.#reader; } @@ -983,7 +1096,11 @@ class ProjectBuildStatus { } rejectReaderRequests(error) { - this.#state = PROJECT_STATES.INVALIDATED; + // Latch the error and gate future rebuilds on invalidate(). Deterministic + // builds don't recover without input change, so re-running the same build + // would just re-produce the same failure. + this.#state = PROJECT_STATES.ERRORED; + this.#lastError = error; for (const {reject} of this.#readerQueue) { reject(error); } diff --git a/packages/project/test/lib/build/BuildServer.integration.js b/packages/project/test/lib/build/BuildServer.integration.js index 233890977ea..4e0c8cf8d46 100644 --- a/packages/project/test/lib/build/BuildServer.integration.js +++ b/packages/project/test/lib/build/BuildServer.integration.js @@ -855,6 +855,40 @@ test.serial("Build server recovers from non-abort build error (no deadlock)", as t.is(errorEvents.length, 0, "No fatal 'error' events emitted for recoverable build failures"); }); +// A normal build error must gate further rebuilds of the same project: deterministic +// builds recover only when their input changes, so re-running the same build would just +// re-produce the same failure. The gate lifts on any source change that invalidates the +// errored project (directly or via a dependency), routed through _projectResourceChanged. +test.serial("Errored project is not rebuilt until input changes", async (t) => { + const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a"); + await fixtureTester.serveProject({config: {cache: Cache.Force}, expectBuildErrors: true}); + + // First request triggers a build that fails because cache=Force has no cache. + const firstError = await t.throwsAsync(() => fixtureTester.requestResource({resource: "/test.js"})); + + // Second and third requests must reject with the *same error instance* — the gate + // short-circuits with the captured error rather than re-running the build (which + // would produce a new Error instance carrying the same message). + const secondError = await t.throwsAsync(() => fixtureTester.requestResource({resource: "/test.js"})); + const thirdError = await t.throwsAsync(() => fixtureTester.requestResource({resource: "/test.js"})); + t.is(secondError, firstError, "Gate returns the captured error instance instead of rebuilding"); + t.is(thirdError, firstError, "Gate keeps returning the same error until input changes"); + + // Simulate a source change that invalidates the errored project. This should lift the + // gate; the next request attempts a rebuild (still fails since cache=Force has no cache, + // but with a *new* error instance — proving the gate released and the build ran). + fixtureTester.buildServer._projectResourceChanged( + fixtureTester.graph.getProject("application.a"), + "/resources/application/a/test.js", + false + ); + await setTimeout(50); + const afterChangeError = await t.throwsAsync(() => fixtureTester.requestResource({resource: "/test.js"})); + t.not(afterChangeError, firstError, "Source change lifted the gate; a fresh build ran"); + t.true(afterChangeError.message.includes(`Cache is in "Force" mode`), + "Fresh build produced an equivalent error"); +}); + // ProjectBuildCache's StageCache must be cleared correctly when a build is aborted. // A task that completed during an aborted attempt has already called recordTaskResult, // which adds its stage to the in-memory StageCache. On retry, diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 9e9200a978d..1571ab10a38 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -329,6 +329,10 @@ test.serial("serve-status: build failure emits serveError and no orphan building const lastBuildingIdx = seq.lastIndexOf("serve-building"); const errorIdx = seq.indexOf("serve-error"); t.true(errorIdx > lastBuildingIdx, "serve-error follows serve-building"); + // The error must remain the terminal state of the cycle — no serve-stale or + // serve-ready may follow it, as that would clear the red banner while the + // project is still gated on its captured error. + t.is(seq[seq.length - 1], "serve-error", "serve-error is the final status of a failed cycle"); }); // When a build cycle drains with INITIAL-state dependencies left over, the server @@ -950,3 +954,339 @@ test.serial( }); +// Regression suite for the "sticky HTTP 500" observed during manual testing. +// +// Reproduces two escapes from the ERRORED-project gate that survive a build +// failure: +// +// 1. A build fails on a dependency (library.a). The user then edits a file +// in an *unrelated* part of the tree — say the root project. That change +// routes through _projectResourceChanged(root, …), which walks +// traverseDependents(root, true) and only invalidates root and its +// dependents. library.a is a *dependency* of root, not a dependent, so +// its ProjectBuildStatus stays in ERRORED. The server-level banner still +// transitions ERROR → STALE → BUILDING → READY off the root project's +// recovery path (root's build succeeds because it doesn't touch the +// failed library resource path), giving the impression of full recovery. +// Any HTTP request that resolves through library.a keeps rejecting with +// the captured file-not-found error → the terminal Express error handler +// turns each into an HTTP 500 with the same stack. +// +// 2. A build fails during a rapid file-change window (e.g. `git checkout`). +// By the time the catch block runs, `signal.aborted` is false and +// `#resourceChangeQueue` has already been flushed, so the failure lands +// on the "normal" branch and latches ERRORED. The trailing watcher events +// arrive *after* the queue was flushed but are absorbed by the *rebuild* +// the transient branch would have run — none of them cascade into +// invalidate(library.a), and the gate stays closed. +// +// Both variants are captured below. + +function makeErrorGraphWithLibDep() { + const rootProject = {getName: () => "root.project"}; + const libProject = {getName: () => "library.a"}; + const projectsByName = {"root.project": rootProject, "library.a": libProject}; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libProject], + getTransitiveDependencies: () => ["library.a"], + getProject: (name) => projectsByName[name], + // traverseDependents(x) yields x + everything that transitively depends on x. + // library.a has NO dependents other than the root project; the root project + // itself has no dependents. So a change in root.project only invalidates + // root; a change in library.a invalidates library.a and root. + traverseDependents: function* (projectName, includeStart) { + if (projectName === "library.a") { + if (includeStart) yield {project: libProject}; + yield {project: rootProject}; + } else { + if (includeStart) yield {project: rootProject}; + } + }, + }; + return {rootProject, libProject, graph}; +} + +// Builds a BuildServer whose BuildReader was mocked to capture the +// buildServerInterface (getReaderForProject/getReaderForProjects), so tests can +// simulate the byPath → getReaderForProject call chain a real HTTP request +// would follow without depending on express. +async function makeBuildServerWithCapturedInterface(t, graph, projectBuilder, initialDeps = ["library.a"]) { + const {sinon} = t.context; + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + class FakeWatchHandler { + constructor() { + this.destroy = sinon.stub().resolves(); + this.on = sinon.stub(); + this.watch = sinon.stub().resolves(); + } + } + const BuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + const buildServer = await BuildServer.create(graph, projectBuilder, true, initialDeps, []); + t.teardown(() => buildServer.destroy()); + // Swallow emitted "error" events so AVA doesn't see unhandled rejections. + // The failure paths under test do not emit "error" (that's reserved for + // fatal failures) but installing the noop listener is defensive. + buildServer.on("error", () => {}); + return {buildServer, getInterface: () => capturedInterface}; +} + +// Builder factory that fails the first invocation and succeeds subsequent ones. +// Mirrors the "task threw ENOENT once, then the tree settled" shape produced by +// a branch switch. +function makeFailOnceThenSucceedBuilder(sinon, buildError) { + const invocations = []; + let calls = 0; + const builder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().resolves([]), + build: sinon.stub().callsFake((opts, perProjectCb) => { + const invocation = {opts, perProjectCb, index: calls}; + invocations.push(invocation); + const isFirst = calls === 0; + calls++; + if (isFirst) { + return Promise.reject(buildError); + } + if (opts.includeRootProject) { + perProjectCb("root.project", {getReader: () => ({builtReader: "root.project"})}); + } + for (const depName of opts.includedDependencies || []) { + perProjectCb(depName, {getReader: () => ({builtReader: depName})}); + } + return Promise.resolve( + (opts.includeRootProject ? ["root.project"] : []).concat(opts.includedDependencies || [])); + }), + }; + return {builder, invocations}; +} + +test.serial( + "error-recovery: change in a dependent project lifts the ERRORED gate on its dependency", + async (t) => { + // Fix 2 in _projectResourceChanged: a source change in root.project must + // also clear the ERRORED gate on library.a (a transitive dependency), even + // though traverseDependents(root) never reaches library.a. Without this, + // any HTTP request that resolves through library.a would keep returning + // the captured build error indefinitely — the "sticky HTTP 500" reported + // after manual testing. + const {sinon, clock} = t.context; + + const {graph, rootProject} = makeErrorGraphWithLibDep(); + const buildError = new Error("ENOENT: no such file or directory, open '/tmp/vanished.js'"); + const statusEvents = makeStatusRecorder(t); + const {builder: projectBuilder, invocations} = makeFailOnceThenSucceedBuilder(sinon, buildError); + + const {buildServer, getInterface} = await makeBuildServerWithCapturedInterface(t, graph, projectBuilder); + + // (1) Drive the initial build cycle to failure — the whole batch (root + + // library.a) rejects, both are latched into ERRORED. + await clock.tickAsync(10); + t.is(invocations.length, 1, "initial build was invoked"); + t.true(invocations[0].opts.includeRootProject && invocations[0].opts.includedDependencies.includes("library.a"), + "initial build covered root + library.a"); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-error"); + t.is(statusEvents[statusEvents.length - 1].status, "serve-error", "cycle ends in serve-error"); + + // (2) User edits a file in root.project. Fix 2 walks + // getTransitiveDependencies(root.project) and clears library.a's ERRORED + // gate, since the user's activity signals retry intent. + buildServer._projectResourceChanged(rootProject, "/index.html", false); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-stale"); + + // (3) A request for library.a's reader must now enqueue a real build + // rather than replay the captured error. + const iface = getInterface(); + const libReq = iface.getReaderForProject("library.a"); + // Drive the 10ms request-queue tick + drain the build promise. The + // follow-up build succeeds (fail-once mock). + await clock.tickAsync(10); + const reader = await libReq; + t.deepEqual(reader, {builtReader: "library.a"}, + "library.a rebuild resolved cleanly after the gate was lifted"); + + // And confirm the mechanism: a rebuild of library.a was actually + // attempted after the initial failure. + t.true( + invocations.some((inv, i) => i > 0 && inv.opts.includedDependencies?.includes("library.a")), + "at least one post-failure invocation built library.a"); + }); + +test.serial( + "error-recovery: successful build cycle lifts ERRORED gates on unrelated projects", async (t) => { + // Fix 1 in #processBuildRequests: after any successful build cycle, sweep + // #projectBuildStatus and lift ERRORED gates on projects the cycle didn't + // touch. This covers the case where the failed project has no graph + // relationship to the changed one (multi-root workspaces, sibling libs + // that only share the root as a common dependent). + const {sinon, clock} = t.context; + + // Two independent libraries sharing only the root. Fix 2's + // getTransitiveDependencies(root) covers both, so to isolate Fix 1 we use + // a graph where a change in libA does NOT reach libB via any traversal — + // libA/libB are siblings, both dependencies of root, with no dependents + // of their own. + const rootProject = {getName: () => "root.project"}; + const libA = {getName: () => "library.a"}; + const libB = {getName: () => "library.b"}; + const projectsByName = { + "root.project": rootProject, "library.a": libA, "library.b": libB, + }; + const graph = { + getRoot: () => rootProject, + getProjects: () => [rootProject, libA, libB], + getTransitiveDependencies: (name) => { + if (name === "root.project") return ["library.a", "library.b"]; + return []; + }, + getProject: (name) => projectsByName[name], + traverseDependents: function* (projectName, includeStart) { + if (projectName === "library.a") { + if (includeStart) yield {project: libA}; + yield {project: rootProject}; + } else if (projectName === "library.b") { + if (includeStart) yield {project: libB}; + yield {project: rootProject}; + } else { + if (includeStart) yield {project: rootProject}; + } + }, + }; + + const buildError = new Error("ENOENT: no such file or directory, open '/tmp/vanished.js'"); + const statusEvents = makeStatusRecorder(t); + + // First cycle fails; every subsequent cycle succeeds. + let calls = 0; + const invocations = []; + const projectBuilder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + validateCaches: sinon.stub().resolves([]), + build: sinon.stub().callsFake((opts, perProjectCb) => { + const invocation = {opts, perProjectCb, index: calls}; + invocations.push(invocation); + const isFirst = calls === 0; + calls++; + if (isFirst) { + return Promise.reject(buildError); + } + if (opts.includeRootProject) { + perProjectCb("root.project", {getReader: () => ({builtReader: "root.project"})}); + } + for (const depName of opts.includedDependencies || []) { + perProjectCb(depName, {getReader: () => ({builtReader: depName})}); + } + return Promise.resolve( + (opts.includeRootProject ? ["root.project"] : []).concat(opts.includedDependencies || [])); + }), + }; + + const {buildServer, getInterface} = await makeBuildServerWithCapturedInterface( + t, graph, projectBuilder, ["library.a", "library.b"]); + + // (1) Initial build fails — root, library.a, library.b all ERRORED. + await clock.tickAsync(10); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-error"); + + // (2) Source change ONLY on library.a. This invalidates library.a (via + // traverseDependents) and lifts library.a's error gate via invalidate(). + // Fix 2 also clears getTransitiveDependencies(library.a) = [] — no effect + // on library.b. Only Fix 1 (the post-cycle sweep) can lift library.b's + // gate. root's gate lifts too (root is a dependent of library.a). + buildServer._projectResourceChanged(libA, "/src/library.a/foo.js", false); + + // (3) Rebuild library.a — this succeeds and, on cycle completion, sweeps + // #projectBuildStatus, clearing library.b's ERRORED gate. + const iface = getInterface(); + const libAReq = iface.getReaderForProject("library.a"); + await clock.tickAsync(10); + await libAReq; + + // (4) A request for library.b must NOT replay the captured error. The + // post-cycle sweep dropped library.b's gate to INVALIDATED, so this + // request enqueues a real rebuild. + const libBReq = iface.getReaderForProject("library.b"); + await clock.tickAsync(10); + const libBReader = await libBReq; + t.deepEqual(libBReader, {builtReader: "library.b"}, + "library.b rebuilt after its ERRORED gate was lifted by the successful cycle"); + }); + +test.serial( + "error-recovery: no source change and no other build → ERRORED gate stays closed", async (t) => { + // Complement to the two "gate lifts" tests: without ANY signal — no source + // change, no successful build — the ERRORED gate is still deterministic + // (a re-request would just re-produce the same failure). This confirms + // the gate is only lifted by an explicit signal, not by wall-clock time. + const {sinon, clock} = t.context; + + const {graph} = makeErrorGraphWithLibDep(); + const buildError = new Error("ENOENT: no such file or directory, open '/tmp/vanished.js'"); + const {builder: projectBuilder, invocations} = makeFailOnceThenSucceedBuilder(sinon, buildError); + const statusEvents = makeStatusRecorder(t); + + const {getInterface} = await makeBuildServerWithCapturedInterface(t, graph, projectBuilder); + + await clock.tickAsync(10); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-error"); + t.is(invocations.length, 1, "only the initial build ran"); + + const iface = getInterface(); + // Five sequential reader requests, no source change in between. Each + // must reject with the exact captured error, and no rebuild is attempted. + const errs = []; + for (let i = 0; i < 5; i++) { + const p = iface.getReaderForProject("library.a"); + await clock.tickAsync(1); + errs.push(await p.catch((e) => e)); + } + t.is(invocations.length, 1, "no rebuild was triggered by any of the 5 requests"); + for (const err of errs) { + t.is(err, buildError, "each request rejects with the captured build error"); + } + }); + +test.serial( + "error-recovery: file change on the ERRORED project itself lifts the gate", async (t) => { + // Sanity-check the counterpart to the sticky path — a change *on* + // library.a does invalidate its ProjectBuildStatus (invalidate() clears + // #lastError), so the follow-up reader request enqueues a real rebuild. + // This documents the current recovery contract and pins down that the + // escape reproduced above is specifically about invalidations that + // don't reach the ERRORED project. + const {sinon, clock} = t.context; + + const {graph, libProject} = makeErrorGraphWithLibDep(); + const buildError = new Error("ENOENT: no such file or directory, open '/tmp/vanished.js'"); + const {builder: projectBuilder, invocations} = makeFailOnceThenSucceedBuilder(sinon, buildError); + const statusEvents = makeStatusRecorder(t); + + const {buildServer, getInterface} = await makeBuildServerWithCapturedInterface(t, graph, projectBuilder); + + await clock.tickAsync(10); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-error"); + + // Change on library.a itself → invalidate(library.a) + invalidate(root). + buildServer._projectResourceChanged(libProject, "/src/library.a/foo.js", false); + + const iface = getInterface(); + const p = iface.getReaderForProject("library.a"); + await clock.tickAsync(10); + const reader = await p; + t.deepEqual(reader, {builtReader: "library.a"}, + "reader request for library.a resolved via the follow-up build after its own change"); + t.true(invocations.length >= 2, "a rebuild of library.a was triggered"); + t.true( + invocations.some((inv, i) => i > 0 && inv.opts.includedDependencies?.includes("library.a")), + "at least one post-failure invocation built library.a"); + }); From 7ed90d88fcc19ff8fa35ce8434852f16965353b2 Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Jul 2026 15:12:25 +0200 Subject: [PATCH 03/16] refactor(server): Add terminal Express error handler Every next(err) in the middleware chain now flows through a custom 4-argument handler that responds with HTTP 500, text/plain, and the error stack (falling back to the message or String(err)). Registered last so it catches errors from every earlier middleware, including third-party custom middleware we cannot wrap in a try/catch. If the response has already started, delegate to Express's default handler, which closes the connection. Console-log routing (separating gated build errors from unexpected ones) stays out of this handler for now; it lands in a follow-up. --- .../server/lib/middleware/errorHandler.js | 28 +++++++ packages/server/lib/server.js | 5 ++ .../lib/server/middleware/errorHandler.js | 81 +++++++++++++++++++ 3 files changed, 114 insertions(+) create mode 100644 packages/server/lib/middleware/errorHandler.js create mode 100644 packages/server/test/lib/server/middleware/errorHandler.js diff --git a/packages/server/lib/middleware/errorHandler.js b/packages/server/lib/middleware/errorHandler.js new file mode 100644 index 00000000000..3fa3a9c7c63 --- /dev/null +++ b/packages/server/lib/middleware/errorHandler.js @@ -0,0 +1,28 @@ +/** + * Express error-handling middleware for the UI5 dev server. + * + * Handles every next(err) call in the middleware chain by responding with + * HTTP 500 and a plain-text body containing the error message and (when available) the + * stack trace. Registering a 4-argument middleware makes Express treat it as an error + * handler; it must be added AFTER all normal middlewares so it acts as the terminal + * catch-all. + * + * Console logging semantics live outside this handler — a follow-up will differentiate + * "expected" build errors (surfaced via the live banner already) from unexpected ones + * that still warrant an error-level console log. + * + * @returns {Function} Express-style (err, req, res, next) middleware. + */ +export default function createErrorHandler() { + return function errorHandler(err, req, res, next) { + // If the response is already partly sent, we can't rewrite it — hand off to + // Express's default handler which will close the connection. + if (res.headersSent) { + return next(err); + } + const message = err?.stack || err?.message || String(err); + res.status(500); + res.type("text/plain; charset=utf-8"); + res.send(message); + }; +} diff --git a/packages/server/lib/server.js b/packages/server/lib/server.js index 7bab0757a59..e66ac86a100 100644 --- a/packages/server/lib/server.js +++ b/packages/server/lib/server.js @@ -4,6 +4,7 @@ import process from "node:process"; import express from "express"; import portscanner from "portscanner"; import MiddlewareManager from "./middleware/MiddlewareManager.js"; +import createErrorHandler from "./middleware/errorHandler.js"; import attachLiveReloadServer from "./liveReload/server.js"; import {createReaderCollection} from "@ui5/fs/resourceFactory"; import ReaderCollectionPrioritized from "@ui5/fs/ReaderCollectionPrioritized"; @@ -237,6 +238,10 @@ export async function serve(graph, { let app = express(); await middlewareManager.applyMiddleware(app); + // Terminal error handler for the middleware chain. Registered after applyMiddleware + // so it sits last and intercepts every next(err) — including those from custom + // middleware where we can't wrap a try/catch. + app.use(createErrorHandler()); if (h2) { const nodeVersion = parseInt(process.versions.node.split(".")[0], 10); diff --git a/packages/server/test/lib/server/middleware/errorHandler.js b/packages/server/test/lib/server/middleware/errorHandler.js new file mode 100644 index 00000000000..2e9fdf04f03 --- /dev/null +++ b/packages/server/test/lib/server/middleware/errorHandler.js @@ -0,0 +1,81 @@ +import test from "ava"; +import createErrorHandler from "../../../../lib/middleware/errorHandler.js"; + +function mockRes() { + const state = { + statusCode: null, + contentType: null, + body: null, + headersSent: false, + }; + return { + state, + get headersSent() { + return state.headersSent; + }, + status(code) { + state.statusCode = code; + return this; + }, + type(t) { + state.contentType = t; + return this; + }, + send(body) { + state.body = body; + return this; + }, + }; +} + +test("Responds with 500 and the error stack as plain text", (t) => { + const handler = createErrorHandler(); + const err = new Error("boom"); + const res = mockRes(); + let nextCalled = false; + + handler(err, {}, res, () => { + nextCalled = true; + }); + + t.is(res.state.statusCode, 500); + t.is(res.state.contentType, "text/plain; charset=utf-8"); + t.is(res.state.body, err.stack, "Body contains the error stack"); + t.false(nextCalled, "next is not called on the normal path"); +}); + +test("Falls back to the error message when no stack is present", (t) => { + const handler = createErrorHandler(); + const err = {message: "just a message"}; + const res = mockRes(); + + handler(err, {}, res, () => t.fail("next should not be called")); + + t.is(res.state.statusCode, 500); + t.is(res.state.body, "just a message"); +}); + +test("Falls back to String(err) when the error carries neither stack nor message", (t) => { + const handler = createErrorHandler(); + const res = mockRes(); + + handler("bare string", {}, res, () => t.fail("next should not be called")); + + t.is(res.state.statusCode, 500); + t.is(res.state.body, "bare string"); +}); + +test("Delegates to next(err) when headers were already sent", (t) => { + const handler = createErrorHandler(); + const err = new Error("late failure"); + const res = mockRes(); + res.state.headersSent = true; + let nextArg; + + handler(err, {}, res, (arg) => { + nextArg = arg; + }); + + t.is(nextArg, err, "The error is forwarded to Express's default handler"); + t.is(res.state.statusCode, null, "Response state is left untouched once headers are out"); +}); From 13d0953c3e25523d9d4dccc841c2ffdfa0eae102 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:05 +0200 Subject: [PATCH 04/16] refactor(project): Recover from dropped file watcher events @parcel/watcher can report that OS-level FS events were dropped and its event cache is unreliable ("Events were dropped by the FSEvents client. File system must be re-scanned."). The build cache derives what changed solely from watcher events fed through resourcesChanged(), so dropped events leave the change signal incomplete and cached results can be served stale. Any WatchHandler error was previously terminal: the server went to ERROR with no recovery. Treat a watcher error as recoverable: recreate the watch subscriptions, force a full source re-scan of every project, and return the server to STALE so the next reader request rebuilds against a re-indexed tree. - Extract ProjectBuildCache.discardIncrementalState() from the reset block allTasksCompleted() already used for build-end source drift. It re-arms initSourceIndex() to re-glob and diff against the persisted index, catching changes the watcher never reported. - Add ProjectBuilder.forceFullRescan() (guarded by #buildIsRunning), delegating through BuildContext / ProjectBuildContext. - BuildServer.#recoverWatcher() orchestrates recovery: guarded against re-entrant error storms, quiesces validation and the active build, recreates the watcher, forces the re-scan, invalidates all projects, and drains parked reader requests. #triggerRequestQueue and #scheduleBackgroundValidation are suppressed during recovery so nothing races the builder lock. A sliding-window counter escalates to terminal ERROR if recovery keeps failing, since dropped events arrive via the subscription callback rather than a watch() rejection and would otherwise loop forever. emit("error") stays reserved for the terminal fallback; a successful recovery emits sourcesChanged so clients reload. --- packages/project/lib/build/BuildServer.js | 152 ++++++++++++++- packages/project/lib/build/ProjectBuilder.js | 21 +++ .../lib/build/cache/ProjectBuildCache.js | 44 ++++- .../project/lib/build/helpers/BuildContext.js | 15 ++ .../lib/build/helpers/ProjectBuildContext.js | 9 + .../test/lib/build/BuildServer.integration.js | 59 +++++- .../project/test/lib/build/BuildServer.js | 173 ++++++++++++++++++ .../test/lib/build/cache/ProjectBuildCache.js | 44 ++++- .../test/lib/build/helpers/WatchHandler.js | 29 +++ 9 files changed, 530 insertions(+), 16 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 4b42f0126e3..2913eec75ab 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -11,6 +11,14 @@ const log = getLogger("build:BuildServer"); // results in a single notification. const SOURCES_CHANGED_DEBOUNCE_MS = 100; +// Loop protection for watcher recovery. A persistently failing watcher (e.g. a watched +// path that keeps erroring on re-subscribe, or an FS that keeps dropping events) would +// otherwise cycle error → recover → error indefinitely. If more than +// WATCHER_RECOVERY_MAX_ATTEMPTS recoveries complete within WATCHER_RECOVERY_WINDOW_MS, the +// watcher is treated as unrecoverable and the server escalates to the terminal ERROR state. +const WATCHER_RECOVERY_MAX_ATTEMPTS = 5; +const WATCHER_RECOVERY_WINDOW_MS = 60000; + // The server's lifecycle state. Mutated exclusively through #setState. const SERVER_STATES = Object.freeze({ IDLE: "idle", // No pending requests, no recent changes, no unvalidated caches. @@ -72,6 +80,12 @@ class BuildServer extends EventEmitter { // is its controller, used to preempt validation when a real build is requested. #activeValidation = null; #validationAbort = null; + // Watcher recovery state. `#recoveringWatcher` guards against re-entrant recovery while a + // recovery pass is in flight (a dropped-events fault emits one error per subscribed path + // in a synchronous burst). `#watcherRecoveryTimestamps` holds the completion times of + // recent recoveries for the loop-protection window. + #recoveringWatcher = false; + #watcherRecoveryTimestamps = []; /** * Creates a new BuildServer instance @@ -171,17 +185,135 @@ class BuildServer extends EventEmitter { async #initWatcher() { const watchHandler = new WatchHandler(); this.#watchHandler = watchHandler; + this.#wireWatchHandler(watchHandler); + await watchHandler.watch(this.#graph.getProjects()); + } + + /** + * Wires the change and error listeners onto a WatchHandler instance. Shared by the + * initial setup and the recovery path so both attach identical handlers. + * + * @param {WatchHandler} watchHandler Handler to wire listeners onto + */ + #wireWatchHandler(watchHandler) { watchHandler.on("error", (err) => { - this.#setState(SERVER_STATES.ERROR, {error: err}); - this.emit("error", err); + this.#recoverWatcher(err); }); watchHandler.on("change", (eventType, resourcePath, project) => { log.verbose(`Source change detected: ${eventType} ${resourcePath} in project '${project.getName()}'`); this._projectResourceChanged(project, resourcePath, ["create", "delete"].includes(eventType)); }); - await watchHandler.watch(this.#graph.getProjects()); } + /** + * Recovers from a WatchHandler error by recreating the watch subscriptions and forcing a + * full source re-scan of every project. + * + * The incremental build cache derives "what changed" solely from discrete watcher events + * (see {@link #_projectResourceChanged} → {@link ProjectBuilder#resourcesChanged}). When + * the watcher errors — most notably when the OS reports that FS events were dropped and + * the file system must be re-scanned — that signal is unreliable: some source changes + * were never reported, so cached build results may be stale. Rather than wedging the + * server in ERROR (the prior behavior, from which the only exit was a further watcher + * event that may never arrive), recreate the watcher and re-scan. + * + * A successful recovery does not emit the fatal error event; it is reserved + * for the terminal fallback when recovery itself fails or loops. + * + * @param {Error} err The error emitted by the WatchHandler + */ + async #recoverWatcher(err) { + // Collapse the error storm (parcel emits one error per subscribed path synchronously) + // into a single recovery, and stay out of the way of a server that is shutting down. + // Set synchronously before the first await so re-entrant emissions bail here. + if (this.#destroyed || this.#recoveringWatcher) { + return; + } + this.#recoveringWatcher = true; + log.warn(`File watcher error, attempting to recover: ${err?.message ?? err}`); + if (err?.stack) { + log.verbose(err.stack); + } + + // Loop protection: a persistently failing watcher would otherwise cycle forever, since + // dropped-events faults arrive via the subscription callback (not a watch() rejection) + // and so never trip the reject-based fallback below. + const now = Date.now(); + this.#watcherRecoveryTimestamps = this.#watcherRecoveryTimestamps + .filter((ts) => now - ts < WATCHER_RECOVERY_WINDOW_MS); + if (this.#watcherRecoveryTimestamps.length >= WATCHER_RECOVERY_MAX_ATTEMPTS) { + this.#recoveringWatcher = false; + log.error(`File watcher failed to recover after ${WATCHER_RECOVERY_MAX_ATTEMPTS} attempts ` + + `within ${WATCHER_RECOVERY_WINDOW_MS} ms. Giving up.`); + this.#setState(SERVER_STATES.ERROR, {error: err}); + this.emit("error", err); + return; + } + + try { + // Quiesce in-flight work so ProjectBuilder.forceFullRescan() can claim the builder. + // #buildIsRunning only clears in the builder's finally after cache writes, so the + // active build must be awaited to completion — not merely aborted — before the + // re-scan. The build promise swallows abort errors, so this resolves cleanly. + await this.#stopActiveValidation("Watcher recovery"); + if (this.#activeBuild) { + try { + await this.#activeBuild; + } catch (buildErr) { + log.verbose(`Active build settled during watcher recovery: ${buildErr?.message ?? buildErr}`); + } + } + if (this.#destroyed) { + return; + } + + // Recreate the watcher. The old handler's listeners stay attached on purpose: a + // teardown "error" (from a failed unsubscribe) then re-enters #recoverWatcher, which + // bails on the #recoveringWatcher guard set above. Removing the listeners instead + // would let destroy()'s emit("error") throw, since Node's EventEmitter throws when + // "error" has no listener. + const oldHandler = this.#watchHandler; + await oldHandler.destroy(); + + const watchHandler = new WatchHandler(); + this.#watchHandler = watchHandler; + this.#wireWatchHandler(watchHandler); + // Subscribe before the re-scan: a change during the teardown window is then caught + // either by the re-glob below or by the freshly-armed watcher. + await watchHandler.watch(this.#graph.getProjects()); + + // Force the full re-scan. forceFullRescan re-arms each project's source index so the + // next build re-globs and diffs against the persisted index, and invalidate() drops + // cached readers (fileAddedOrRemoved) so the rebuild re-reads the tree. + this.#projectBuilder.forceFullRescan(); + for (const status of this.#projectBuildStatus.values()) { + status.invalidate({reason: "File watcher recovery", fileAddedOrRemoved: true}); + } + + this.#watcherRecoveryTimestamps.push(Date.now()); + log.info(`File watcher recovered. Re-scanning all project sources.`); + + // Every project is now non-fresh. Surface STALE and prompt connected clients to + // reload, which drives the lazy rebuild against the re-scanned index. + this.#setState(SERVER_STATES.STALE, {staleProjects: this.#getStaleProjectNames()}); + this.emit("sourcesChanged"); + } catch (recoveryErr) { + // Recreation itself failed (e.g. watch() rejected). The watcher is genuinely broken; + // fall back to the terminal ERROR behavior. + log.error(`File watcher recovery failed: ${recoveryErr?.message ?? recoveryErr}`); + this.#setState(SERVER_STATES.ERROR, {error: recoveryErr}); + this.emit("error", recoveryErr); + return; + } finally { + this.#recoveringWatcher = false; + } + // Drain reader requests parked before the error (and any suppressed during recovery): + // invalidate() alone does not enqueue their builds, so without this they would hang + // until a brand-new request arrives. + this.#triggerRequestQueue(); + } + + async destroy() { this.#destroyed = true; clearTimeout(this.#processBuildRequestsTimeout); @@ -439,7 +571,10 @@ class BuildServer extends EventEmitter { } #triggerRequestQueue() { - if (this.#destroyed || this.#activeBuild) { + if (this.#destroyed || this.#activeBuild || this.#recoveringWatcher) { + // While recovering the watcher, suppress builds so ProjectBuilder.forceFullRescan() + // can claim the builder without racing a build the abort/re-queue path may start. + // #recoverWatcher re-triggers the queue once recovery completes. return; } // If no build is active, trigger queue processing debounced @@ -451,7 +586,9 @@ class BuildServer extends EventEmitter { // the builder's "buildIsRunning" lock. Validation will be re-scheduled // after the build cycle drains. await this.#stopActiveValidation("Build request received"); - if (this.#destroyed) { + if (this.#destroyed || this.#recoveringWatcher) { + // A watcher recovery claimed the builder during the await above (or is about + // to). #recoverWatcher re-triggers the queue once it completes. return; } // A concurrent timer may have claimed the build slot during the await @@ -741,7 +878,10 @@ class BuildServer extends EventEmitter { * When false, callers may want to transition the server to STALE explicitly. */ #scheduleBackgroundValidation({hrtime} = {}) { - if (this.#destroyed || this.#activeValidation || this.#activeBuild) { + if (this.#destroyed || this.#activeValidation || this.#activeBuild || this.#recoveringWatcher) { + // While recovering the watcher, a validation pass would claim the builder's + // buildIsRunning lock and make forceFullRescan() throw. Recovery re-triggers the + // queue on completion, which drives the next validation/build. return false; } const projectsToValidate = []; diff --git a/packages/project/lib/build/ProjectBuilder.js b/packages/project/lib/build/ProjectBuilder.js index 94a724b34fd..c487dd4060c 100644 --- a/packages/project/lib/build/ProjectBuilder.js +++ b/packages/project/lib/build/ProjectBuilder.js @@ -142,6 +142,27 @@ class ProjectBuilder { return this._buildContext.propagateResourceChanges(changes); } + /** + * Discards all in-memory build caches so that the next build of each project re-scans its + * source tree from scratch and diffs it against the persisted index. + * + * Intended for long-running consumers (such as the + * [BuildServer]{@link @ui5/project/build/BuildServer}) to recover when the incremental + * change signal became unreliable — most notably when the OS file watcher reports that + * events were dropped and the file system must be re-scanned. After this call the next + * build treats every source file as a change candidate, so cached results that a missed + * event would otherwise have kept stale are re-validated. + * + * @public + * @throws {Error} If a build is currently running + */ + forceFullRescan() { + if (this.#buildIsRunning) { + throw new Error(`Unable to safely force a full re-scan. Build is currently running.`); + } + this._buildContext.discardIncrementalState(); + } + /** * Releases the build cache database connection and any underlying storage resources. * diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index 30dd53d3268..17a61f412b9 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -1224,6 +1224,41 @@ export default class ProjectBuildCache { this.#project.getProjectResources().setFrozenSourceReader(casSourceReader); } + /** + * Discards the in-memory source index and task caches so that the next build + * re-initializes the source index from scratch via a full byGlob("/**\/*") + * re-scan (see {@link #initSourceIndex}), diffing the live source tree against the + * persisted index. Used to recover from situations where the incremental change signal + * became unreliable — the file watcher dropping OS-level FS events, or a source file + * changing during a build. + * + * The change accumulators are cleared as well: their contents are superseded by the + * full re-scan. Content-addressed state that stays correct across the reset — + * #stageCache (a stale entry only matches when its content matches) and + * #cachedFrozenSourceMetadata (re-read from the persisted cache during + * #initSourceIndex). + * + * No-op in {@link @ui5/project/build/cache/Cache}.Off mode, where no index or result + * cache exists to reset. + * + * @public + */ + discardIncrementalState() { + if (this.#cacheMode === Cache.Off) { + return; + } + this.#combinedIndexState = INDEX_STATES.RESTORING_PROJECT_INDICES; + this.#sourceIndex = null; + this.#taskCache.clear(); + // Reset the result cache state: a prior validateCache may have moved it to + // NO_CACHE or FRESH_AND_IN_USE. The next build's validateCache asserts + // PENDING_VALIDATION after the dependency-index restore step, so any other + // value would trip that assertion. + this.#resultCacheState = RESULT_CACHE_STATES.PENDING_VALIDATION; + this.#changedProjectSourcePaths = []; + this.#changedDependencyResourcePaths = []; + } + /** * Signals that all tasks have completed and switches to the result stage * @@ -1261,14 +1296,7 @@ export default class ProjectBuildCache { // Reset index state so that the next build attempt will re-initialize the source index // from scratch. Without this, a retry in the BuildServer would reuse the stale index // and perpetually detect the same change. - this.#combinedIndexState = INDEX_STATES.RESTORING_PROJECT_INDICES; - this.#sourceIndex = null; - this.#taskCache.clear(); - // Result cache state must also be reset: the aborted build's validateCache may - // have already transitioned it to NO_CACHE or FRESH_AND_IN_USE. The retry's - // validateCache asserts PENDING_VALIDATION after the dependency-index restore - // step, so a leftover non-PENDING_VALIDATION value would trip that assertion. - this.#resultCacheState = RESULT_CACHE_STATES.PENDING_VALIDATION; + this.discardIncrementalState(); throw new SourceChangedDuringBuildError(this.#project.getName()); } diff --git a/packages/project/lib/build/helpers/BuildContext.js b/packages/project/lib/build/helpers/BuildContext.js index 50251ac9ddc..59821282710 100644 --- a/packages/project/lib/build/helpers/BuildContext.js +++ b/packages/project/lib/build/helpers/BuildContext.js @@ -186,6 +186,21 @@ class BuildContext { })); } + /** + * Forces a full source re-scan for every project that has an initialized build context. + * + * Discards each context's in-memory build cache so the next build re-globs the source + * tree and diffs it against the persisted index, picking up changes that were never + * reported through {@link #propagateResourceChanges} (e.g. file watcher events dropped + * by the OS). Never-built projects have no context yet and re-scan naturally on first + * build, so iterating existing contexts only is sufficient. + */ + discardIncrementalState() { + for (const ctx of this._projectBuildContexts.values()) { + ctx.discardIncrementalState(); + } + } + closeCacheManager() { if (this.#cacheManager) { this.#cacheManager.close(); diff --git a/packages/project/lib/build/helpers/ProjectBuildContext.js b/packages/project/lib/build/helpers/ProjectBuildContext.js index f7cb3913135..b269bc3f5ce 100644 --- a/packages/project/lib/build/helpers/ProjectBuildContext.js +++ b/packages/project/lib/build/helpers/ProjectBuildContext.js @@ -342,6 +342,15 @@ class ProjectBuildContext { return this._buildCache.dependencyResourcesChanged(changedPaths); } + /** + * Discards this project's in-memory build cache so that the next build re-scans the + * source tree from scratch. Delegates to + * {@link @ui5/project/build/cache/ProjectBuildCache#discardIncrementalState}. + */ + discardIncrementalState() { + return this._buildCache.discardIncrementalState(); + } + propagateResourceChanges(changedPaths) { if (!changedPaths.length) { return; diff --git a/packages/project/test/lib/build/BuildServer.integration.js b/packages/project/test/lib/build/BuildServer.integration.js index 4e0c8cf8d46..2332fe9c4ab 100644 --- a/packages/project/test/lib/build/BuildServer.integration.js +++ b/packages/project/test/lib/build/BuildServer.integration.js @@ -85,7 +85,17 @@ function createParcelWatcherMock() { subscriptions.length = 0; } - return {api, fire, reset}; + // Deliver an error to every active subscription callback, mirroring how @parcel/watcher + // surfaces a dropped-events condition ("File system must be re-scanned."). + async function fireError(err) { + // Snapshot: recovery destroys/recreates subscriptions while we iterate. + for (const sub of subscriptions.slice()) { + sub.callback(err); + } + await new Promise((resolve) => setImmediate(resolve)); + } + + return {api, fire, fireError, reset}; } test.beforeEach((t) => { @@ -204,6 +214,46 @@ test.serial("Serve application.a, request application resource", async (t) => { t.true(servedFileContent.includes(`test("line added");`), "Resource contains changed file content"); }); +// The incremental cache learns "what changed" only from watcher events. When @parcel/watcher +// reports that events were dropped, a source change may go unreported — a naive rebuild would +// then serve a stale cache hit. The recovery path forces a full re-scan so the un-notified +// change is still picked up. +test.serial("Serve application.a, dropped watcher events force a full re-scan", async (t) => { + const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a"); + + await fixtureTester.serveProject(); + + // #1 build and cache the resource. + const before = await fixtureTester.requestResource({resource: "/test.js"}); + t.false((await before.getString()).includes(`test("dropped-event change");`), + "baseline content does not yet contain the change"); + + // #2 confirm the cache is warm — a repeated request rebuilds nothing. + await fixtureTester.requestResource({ + resource: "/test.js", + assertions: {projects: {}}, + }); + + // Modify a source file WITHOUT firing a watcher change event: this models the OS dropping + // the FS event. Without recovery, the cache would keep serving the stale build result. + const changedFilePath = `${fixtureTester.fixturePath}/webapp/test.js`; + await fs.appendFile(changedFilePath, `\ntest("dropped-event change");\n`); + + // The watcher reports the drop instead of the change. Recovery runs asynchronously and + // emits `sourcesChanged` on completion; await that so the forced re-scan + invalidation + // have settled before the next request. + const recovered = new Promise((resolve) => fixtureTester.buildServer.once("sourcesChanged", resolve)); + await fixtureTester.fireWatcherError( + new Error("Events were dropped by the FSEvents client. File system must be re-scanned.")); + await recovered; + + // #3 the next request must reflect the un-notified change, proving the forced re-scan + // re-indexed the source tree and invalidated the stale cache. + const after = await fixtureTester.requestResource({resource: "/test.js"}); + t.true((await after.getString()).includes(`test("dropped-event change");`), + "resource reflects the change the watcher never reported, after the forced re-scan"); +}); + test.serial("Serve application.a, create and delete a source file", async (t) => { const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a"); @@ -1159,6 +1209,13 @@ class FixtureTester { await watcherMock.fire(type, filePath); } + // Fires a dropped-events error through the in-process @parcel/watcher mock. Models the + // real-world "Events were dropped by the FSEvents client. File system must be + // re-scanned." fault: the incremental change signal is now known to be incomplete. + async fireWatcherError(err) { + await watcherMock.fireError(err); + } + _assertBuild(assertions) { const {projects = {}} = assertions; diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 1571ab10a38..98db0b73819 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -1290,3 +1290,176 @@ test.serial( invocations.some((inv, i) => i > 0 && inv.opts.includedDependencies?.includes("library.a")), "at least one post-failure invocation built library.a"); }); +// Builds a BuildServer whose WatchHandler and BuildReader are both mocked so tests can +// (a) drive the buildServerInterface (getReaderForProject) and (b) fire the watcher error +// callback and inspect re-subscription. `watchHandlers` accumulates every WatchHandler the +// server constructs — index 0 is the initial handler, later entries are recovery handlers. +// `opts.failWatchFrom` makes watch() reject for the handler at that index and beyond +// (used to exercise the re-subscription-failure fallback). +async function makeRecoverableBuildServer(t, graph, projectBuilder, {initialDeps = ["library.a"], + failWatchFrom = Infinity} = {}) { + const {sinon} = t.context; + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + const watchHandlers = []; + class FakeWatchHandler { + constructor() { + const index = watchHandlers.length; + this.listeners = Object.create(null); + this.destroy = sinon.stub().resolves(); + this.watch = index >= failWatchFrom ? + sinon.stub().rejects(new Error("watch() rejected")) : + sinon.stub().resolves(); + this.on = sinon.stub().callsFake((event, cb) => { + (this.listeners[event] ??= []).push(cb); + }); + this.emitError = (err) => { + for (const cb of this.listeners.error ?? []) { + cb(err); + } + }; + watchHandlers.push(this); + } + } + const BuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + const buildServer = await BuildServer.create(graph, projectBuilder, true, initialDeps, []); + t.teardown(() => buildServer.destroy()); + const errorEvents = []; + buildServer.on("error", (err) => errorEvents.push(err)); + return {buildServer, getInterface: () => capturedInterface, watchHandlers, errorEvents}; +} + +function makeRescanBuilder(sinon) { + const invocations = []; + const builder = { + closeCacheManager: sinon.stub(), + resourcesChanged: sinon.stub(), + forceFullRescan: sinon.stub(), + validateCaches: sinon.stub().resolves([]), + build: sinon.stub().callsFake((opts, perProjectCb) => { + invocations.push({opts}); + if (opts.includeRootProject) { + perProjectCb("root.project", {getReader: () => ({builtReader: "root.project"})}); + } + for (const depName of opts.includedDependencies || []) { + perProjectCb(depName, {getReader: () => ({builtReader: depName})}); + } + return Promise.resolve( + (opts.includeRootProject ? ["root.project"] : []).concat(opts.includedDependencies || [])); + }), + }; + return {builder, invocations}; +} + +const droppedEventsError = () => + new Error("Events were dropped by the FSEvents client. File system must be re-scanned."); + +test.serial( + "watcher-recovery: dropped-events error recreates watcher and forces a full re-scan", async (t) => { + const {sinon, clock, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; + const {graph} = makeErrorGraphWithLibDep(); + const {builder: projectBuilder} = makeRescanBuilder(sinon); + const statusEvents = makeStatusRecorder(t); + + const {buildServer, watchHandlers, errorEvents} = + await makeRecoverableBuildServer(t, graph, projectBuilder); + + // Let the initial build cycle settle. + await clock.tickAsync(20); + t.is(watchHandlers.length, 1, "one watcher initially"); + + const sourcesChanged = sinon.stub(); + buildServer.on("sourcesChanged", sourcesChanged); + + // Fire the dropped-events error on the initial watcher. + watchHandlers[0].emitError(droppedEventsError()); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-stale"); + + t.true(watchHandlers[0].destroy.calledOnce, "old watcher destroyed"); + t.is(watchHandlers.length, 2, "a fresh watcher was created"); + t.true(watchHandlers[1].watch.calledOnce, "fresh watcher re-subscribed"); + t.true(projectBuilder.forceFullRescan.calledOnce, "full re-scan forced on the builder"); + t.is(errorEvents.length, 0, "no fatal error event on successful recovery"); + + await clock.tickAsync(SOURCES_CHANGED_DEBOUNCE_MS); + t.true(sourcesChanged.calledOnce, "sourcesChanged emitted so clients reload"); + + const seq = statusEvents.map((e) => e.status); + t.is(seq[seq.length - 1], "serve-stale", "server settles on STALE after recovery"); + }); + +test.serial( + "watcher-recovery: a reader request parked before the error resolves after recovery", async (t) => { + const {sinon, clock} = t.context; + const {graph} = makeErrorGraphWithLibDep(); + const {builder: projectBuilder, invocations} = makeRescanBuilder(sinon); + makeStatusRecorder(t); + + const {getInterface, watchHandlers} = + await makeRecoverableBuildServer(t, graph, projectBuilder, {initialDeps: []}); + + await clock.tickAsync(20); + const buildsBefore = invocations.length; + + // Park a reader request, then fire the watcher error before it is served. + const readerPromise = getInterface().getReaderForProject("library.a"); + watchHandlers[0].emitError(droppedEventsError()); + + // Recovery drains the queue on completion; the parked request must resolve. + await clock.tickAsync(30); + const reader = await readerPromise; + t.deepEqual(reader, {builtReader: "library.a"}, "parked reader request resolved after recovery"); + t.true(invocations.length > buildsBefore, "a build ran to serve the parked request post-recovery"); + }); + +test.serial( + "watcher-recovery: repeated errors within the window escalate to a fatal error", async (t) => { + const {sinon, clock} = t.context; + const {graph} = makeErrorGraphWithLibDep(); + const {builder: projectBuilder} = makeRescanBuilder(sinon); + const statusEvents = makeStatusRecorder(t); + + const {watchHandlers, errorEvents} = + await makeRecoverableBuildServer(t, graph, projectBuilder); + await clock.tickAsync(20); + + // Drive more recoveries than the loop-protection budget allows within the window. + // Each successful recovery creates a new handler; fire the error on the latest one. + let escalated = false; + for (let i = 0; i < 10 && !escalated; i++) { + watchHandlers[watchHandlers.length - 1].emitError(droppedEventsError()); + await clock.tickAsync(30); + escalated = errorEvents.length > 0; + } + + t.true(escalated, "loop protection eventually escalated to a fatal error event"); + const seq = statusEvents.map((e) => e.status); + t.is(seq[seq.length - 1], "serve-error", "server ends in ERROR after giving up"); + }); + +test.serial( + "watcher-recovery: failure to re-subscribe falls back to fatal error", async (t) => { + const {sinon, clock} = t.context; + const {graph} = makeErrorGraphWithLibDep(); + const {builder: projectBuilder} = makeRescanBuilder(sinon); + const statusEvents = makeStatusRecorder(t); + + // Fail watch() from the first recovery handler (index 1) onward; the initial + // handler (index 0) subscribes normally so the server starts up. + const {watchHandlers, errorEvents} = + await makeRecoverableBuildServer(t, graph, projectBuilder, {failWatchFrom: 1}); + await clock.tickAsync(20); + + watchHandlers[0].emitError(droppedEventsError()); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-error"); + + t.is(errorEvents.length, 1, "a fatal error event was emitted"); + t.is(errorEvents[0].message, "watch() rejected", "the re-subscription failure is surfaced"); + }); diff --git a/packages/project/test/lib/build/cache/ProjectBuildCache.js b/packages/project/test/lib/build/cache/ProjectBuildCache.js index 82929a02ab7..bf7756b93d3 100644 --- a/packages/project/test/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/test/lib/build/cache/ProjectBuildCache.js @@ -303,7 +303,49 @@ test("allTasksCompleted returns changed resource paths", async (t) => { t.true(Array.isArray(changedPaths), "Returns array of changed paths"); }); -// ===== TASK EXECUTION AND RECORDING TESTS ===== +test("discardIncrementalState re-arms a full source re-scan", async (t) => { + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + + // A byGlob stub whose result set only grows after the initial build completes: the extra + // /added.js appears solely because discardIncrementalState re-globs — no watcher event ever + // reported it. Keeping the set stable during the first build avoids tripping the + // build-end source-drift detector (#revalidateSourceIndex). + const initialResource = createMockResource("/test.js", "hash1", 1000, 100, 1); + const addedResource = createMockResource("/added.js", "hash2", 2000, 50, 2); + let currentResources = [initialResource]; + const byGlob = sinon.stub().callsFake(async () => currentResources.slice()); + const byPath = sinon.stub().callsFake(async (path) => + currentResources.find((r) => r.getPath() === path) ?? null); + project.getSourceReader.callsFake(() => ({byGlob, byPath})); + + const cache = await ProjectBuildCache.create(project, "sig", cacheManager); + await cache.initSourceIndex(); + await cache.allTasksCompleted(); + t.true(cache.isFresh(), "cache is fresh after the initial build"); + const globCallsAfterFirstBuild = byGlob.callCount; + + // A source file appears that the watcher never reported, then force a full re-scan. + currentResources = [initialResource, addedResource]; + cache.discardIncrementalState(); + t.false(cache.isFresh(), "cache is no longer fresh after reset"); + + // initSourceIndex is gated on RESTORING_PROJECT_INDICES; the reset re-armed it, so the + // source tree is re-globbed even though no projectSourcesChanged was ever recorded. + await cache.initSourceIndex(); + t.true(byGlob.callCount > globCallsAfterFirstBuild, + "discardIncrementalState re-armed initSourceIndex to re-glob the source tree"); +}); + +test("discardIncrementalState is a no-op in Cache.Off mode", async (t) => { + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + const cache = await ProjectBuildCache.create(project, "sig", cacheManager, Cache.Off); + await cache.initSourceIndex(); + + // Must not throw and must leave the (absent) index untouched. + t.notThrows(() => cache.discardIncrementalState()); +}); test("prepareTaskExecutionAndValidateCache: task needs execution when no cache exists", async (t) => { const project = createMockProject(); diff --git a/packages/project/test/lib/build/helpers/WatchHandler.js b/packages/project/test/lib/build/helpers/WatchHandler.js index a7c32ccebf1..87f5d8e3935 100644 --- a/packages/project/test/lib/build/helpers/WatchHandler.js +++ b/packages/project/test/lib/build/helpers/WatchHandler.js @@ -146,6 +146,35 @@ test.serial("watch: emits error from watcher callback error", async (t) => { await handler.destroy(); }); +// @parcel/watcher surfaces dropped OS-level FS events as a callback error whose message +// asks the consumer to re-scan the file system. WatchHandler forwards it verbatim; the +// re-scan/recovery decision lives in the BuildServer error handler. +test.serial("watch: forwards dropped-events error from watcher callback", async (t) => { + const subscription = createMockSubscription(); + const callbackReady = captureCallback(subscription); + + const handler = new WatchHandler(); + const project = { + getSourcePaths: () => ["/src"], + getName: () => "test-project" + }; + + await handler.watch([project]); + const callback = await callbackReady; + + const droppedError = new Error( + "Events were dropped by the FSEvents client. File system must be re-scanned."); + const errorPromise = new Promise((resolve) => { + handler.on("error", (err) => { + t.is(err, droppedError, "forwards the original error instance"); + resolve(); + }); + }); + callback(droppedError); + await errorPromise; + await handler.destroy(); +}); + test.serial("watch: emits error when handler throws", async (t) => { const subscription = createMockSubscription(); const callbackReady = captureCallback(subscription); From af5e0a52b60434ccdc36a5684c27ffa0e8721e16 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:06 +0200 Subject: [PATCH 05/16] refactor(project): Reset failing project state to recover build errors A build-task failure (e.g. a LESS syntax error from buildThemes) left the reused in-memory build state corrupted, so `ui5 serve` kept returning the same error after the source was fixed, until a server restart. When a task throws, runTasks rejects before allTasksCompleted() runs. The project's ProjectResources keeps the partial stage writers from the broken source, and #currentResultSignature keeps the last successful build's value. On the next build the recovered source signature matches the retained #currentResultSignature, so #findResultCache short-circuits, marks the project usesCache, and serves the stale broken stages. Dependent theme libraries then recompile against that and fail the same way. Discard the failing project's in-memory state on a genuine (non-abort) failure so a later rebuild re-imports clean stages from the persistent cache. ProjectBuilder#build tracks the in-flight context and calls discardIncrementalState() on it; abort and SourceChangedDuringBuildError are excluded via isAbortError. Harden discardIncrementalState() against the same corruption: also clear #currentResultSignature, #cachedResultSignature, #currentStageSignatures, and #writtenResultResourcePaths, and reset the stage pipeline via the new ProjectResources.reset(). Otherwise the #findResultCache early return would still fire and skip re-importing the cached stages after a reset. --- packages/project/lib/build/ProjectBuilder.js | 25 +++++++--- .../lib/build/cache/ProjectBuildCache.js | 35 ++++++++++---- .../project/lib/resources/ProjectResources.js | 22 +++++++++ .../test/lib/build/BuildServer.integration.js | 42 +++++++++++++++++ .../project/test/lib/build/ProjectBuilder.js | 46 +++++++++++++++++++ .../test/lib/build/cache/ProjectBuildCache.js | 37 +++++++++++++++ .../test/lib/resources/ProjectResources.js | 24 ++++++++++ 7 files changed, 217 insertions(+), 14 deletions(-) diff --git a/packages/project/lib/build/ProjectBuilder.js b/packages/project/lib/build/ProjectBuilder.js index c487dd4060c..0fe74d04e8a 100644 --- a/packages/project/lib/build/ProjectBuilder.js +++ b/packages/project/lib/build/ProjectBuilder.js @@ -146,12 +146,11 @@ class ProjectBuilder { * Discards all in-memory build caches so that the next build of each project re-scans its * source tree from scratch and diffs it against the persisted index. * - * Intended for long-running consumers (such as the - * [BuildServer]{@link @ui5/project/build/BuildServer}) to recover when the incremental - * change signal became unreliable — most notably when the OS file watcher reports that - * events were dropped and the file system must be re-scanned. After this call the next - * build treats every source file as a change candidate, so cached results that a missed - * event would otherwise have kept stale are re-validated. + * For long-running consumers (such as the [BuildServer]{@link @ui5/project/build/BuildServer}) + * to recover when the incremental change signal became unreliable, most notably when the OS + * file watcher reports dropped events and the file system must be re-scanned. After this call + * the next build treats every source file as a change candidate, re-validating cached results + * that a missed event would otherwise have kept stale. * * @public * @throws {Error} If a build is currently running @@ -434,9 +433,14 @@ class ProjectBuilder { } const startTime = process.hrtime(); + // Tracks the project currently being processed so a genuine (non-abort) failure + // can discard its in-memory build state (see the catch below). Cleared once the + // loop completes without throwing. + let failingProjectContext = null; try { for (const projectBuildContext of queue) { signal?.throwIfAborted(); + failingProjectContext = projectBuildContext; const project = projectBuildContext.getProject(); const projectName = project.getName(); const projectType = project.getType(); @@ -475,6 +479,7 @@ class ProjectBuilder { projectBuildContext.buildFinished(); } + failingProjectContext = null; this.#log.info(`Build succeeded in ${this._getElapsedTime(startTime)}`); } catch (err) { // A cooperative abort (e.g. caller aborted the signal, or the @@ -486,6 +491,14 @@ class ProjectBuilder { this.#log.verbose(`Build aborted: ${err?.message ?? err}`); } else { this.#log.error(`Build failed`); + // A task threw mid-build, so allTasksCompleted never ran: the project's stage + // pipeline still holds the failing task's partial output, and its result + // signature is the previous successful build's. Discard that in-memory state so + // a later rebuild (after the source is fixed) re-imports clean stages from the + // persistent cache instead of serving the partial output. + // SourceChangedDuringBuildError already self-resets and is filtered out above by + // isAbortError. + failingProjectContext?.discardIncrementalState(); } throw err; } diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index 17a61f412b9..b8d9eb2c44b 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -1225,15 +1225,23 @@ export default class ProjectBuildCache { } /** - * Discards the in-memory source index and task caches so that the next build - * re-initializes the source index from scratch via a full byGlob("/**\/*") - * re-scan (see {@link #initSourceIndex}), diffing the live source tree against the - * persisted index. Used to recover from situations where the incremental change signal - * became unreliable — the file watcher dropping OS-level FS events, or a source file - * changing during a build. + * Discards the in-memory source index and task caches so the next build re-initializes + * the source index from scratch via a full byGlob("/**\/*") re-scan (see + * {@link #initSourceIndex}), diffing the live source tree against the persisted index. + * Recovers from an unreliable incremental change signal (the file watcher dropping + * OS-level FS events, or a source file changing during a build) and from a build that + * threw mid-execution. * - * The change accumulators are cleared as well: their contents are superseded by the - * full re-scan. Content-addressed state that stays correct across the reset — + * Also clears the change accumulators (superseded by the re-scan), the derived per-build + * signatures (#currentResultSignature, #cachedResultSignature, + * #currentStageSignatures) and the written-path accumulator, and resets the + * project's stage pipeline via {@link @ui5/project/resources/ProjectResources#reset}. A + * build that threw leaves these pointing at partial output and a stale result signature; + * without clearing them, the next {@link #findResultCache} matches the retained + * #currentResultSignature and serves the partial output instead of + * re-importing the cached stages. + * + * Keeps content-addressed state that stays correct across the reset: * #stageCache (a stale entry only matches when its content matches) and * #cachedFrozenSourceMetadata (re-read from the persisted cache during * #initSourceIndex). @@ -1257,6 +1265,17 @@ export default class ProjectBuildCache { this.#resultCacheState = RESULT_CACHE_STATES.PENDING_VALIDATION; this.#changedProjectSourcePaths = []; this.#changedDependencyResourcePaths = []; + // Derived per-build state a discarded (failed) build must not leave behind. + // #currentResultSignature drives the #findResultCache early return; #currentStageSignatures + // drives the isInitialImport/setStage guards in #importStages; #writtenResultResourcePaths + // is normally emptied by allTasksCompleted, which a thrown build never reaches. + this.#currentResultSignature = undefined; + this.#cachedResultSignature = undefined; + this.#currentStageSignatures = new Map(); + this.#writtenResultResourcePaths = []; + // Reset the stage pipeline so #importStages re-initializes stages and re-imports + // cached results instead of reusing the failed build's partial writers. + this.#project.getProjectResources().reset(); } /** diff --git a/packages/project/lib/resources/ProjectResources.js b/packages/project/lib/resources/ProjectResources.js index 8698f81a1cc..cf918464a5e 100644 --- a/packages/project/lib/resources/ProjectResources.js +++ b/packages/project/lib/resources/ProjectResources.js @@ -276,6 +276,28 @@ class ProjectResources { } } + /** + * Discards all stage state and returns the instance to its constructor-equivalent + * baseline: a single initial stage, no frozen source reader, and empty + * tag collections. + * + * Used to recover from a build that threw mid-execution. Such a build leaves the + * current stage pointed at the failing task's writer (holding partial output) and + * the tag collections populated, none of which a later build clears on its own. + * After this call the next {@link #getStage} reports the initial stage, + * so the build cache re-initializes stages and re-imports cached results. + * + * @public + */ + reset() { + this.#initStageMetadata(); + // #initStageMetadata resets the project tag collection and stage pipeline but not + // the build-level or monitored collections, which a failed build may have populated. + this.#buildResourceTagCollection = null; + this.#monitoredProjectResourceTagCollection = null; + this.#monitoredBuildResourceTagCollection = null; + } + /** * Get the current stage. * diff --git a/packages/project/test/lib/build/BuildServer.integration.js b/packages/project/test/lib/build/BuildServer.integration.js index 2332fe9c4ab..2018762e531 100644 --- a/packages/project/test/lib/build/BuildServer.integration.js +++ b/packages/project/test/lib/build/BuildServer.integration.js @@ -939,6 +939,48 @@ test.serial("Errored project is not rebuilt until input changes", async (t) => { "Fresh build produced an equivalent error"); }); +// A build task that throws (here: buildThemes on a LESS syntax error) leaves the project's +// reused in-memory state holding the failing task's partial output and the previous +// successful build's result signature. Without a failure-path reset, fixing the source and +// re-requesting keeps serving the broken stages: the recovered source signature matches the +// retained result signature, so #findResultCache short-circuits and never re-imports the +// (uncorrupted) cached stages. This is the theme-build "fails to recover" scenario. +test.serial("Failed theme build recovers after the source is fixed", async (t) => { + const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "theme.library.e"); + await fixtureTester.serveProject({expectBuildErrors: true}); + + const cssResource = "/resources/theme/library/e/themes/my_theme/library.css"; + const lessFilePath = + `${fixtureTester.fixturePath}/src/theme/library/e/themes/my_theme/library.source.less`; + const originalLess = await fs.readFile(lessFilePath, {encoding: "utf8"}); + + // #1 initial request builds the theme successfully. + const initialCss = await fixtureTester.requestResource({resource: cssResource}); + t.true((await initialCss.getString()).includes("background-color"), + "Initial build produced valid CSS"); + + // Inject a LESS syntax error and notify the watcher. + await fs.writeFile(lessFilePath, `${originalLess}\n@@@ this is not valid less @@@\n`); + await fixtureTester.fireWatcherEvent("update", lessFilePath); + + // #2 request now fails: buildThemes throws on the malformed input. + const buildError = await t.throwsAsync(() => fixtureTester.requestResource({resource: cssResource})); + t.truthy(buildError, "Build fails while the LESS file has a syntax error"); + + // Fix the source and notify the watcher. + await fs.writeFile(lessFilePath, originalLess); + await fixtureTester.fireWatcherEvent("update", lessFilePath); + + // #3 request after the fix must recover and serve valid CSS — the failed build's partial + // state was discarded and clean stages were re-imported. + const recoveredCss = await fixtureTester.requestResource({resource: cssResource}); + const recoveredContent = await recoveredCss.getString(); + t.true(recoveredContent.includes("background-color"), + "Build recovers and serves valid CSS after the source is fixed"); + t.false(recoveredContent.includes("test{"), + "Recovered CSS does not contain artifacts of the broken build"); +}); + // ProjectBuildCache's StageCache must be cleared correctly when a build is aborted. // A task that completed during an aborted attempt has already called recordTaskResult, // which adds its stage to the in-memory StageCache. On retry, diff --git a/packages/project/test/lib/build/ProjectBuilder.js b/packages/project/test/lib/build/ProjectBuilder.js index d6360ee1305..fa86a635fa2 100644 --- a/packages/project/test/lib/build/ProjectBuilder.js +++ b/packages/project/test/lib/build/ProjectBuilder.js @@ -232,6 +232,7 @@ test("build: Failure", async (t) => { possiblyRequiresBuild: possiblyRequiresBuildStub, validateCache: validateCacheStub, buildProject: buildProjectStub, + discardIncrementalState: sinon.stub(), getProject: sinon.stub().returns(getMockProject("library")) }; sinon.stub(builder._buildContext, "getRequiredProjectContexts") @@ -252,12 +253,57 @@ test("build: Failure", async (t) => { t.is(writeResultsStub.callCount, 0, "_writeResults did not get called"); + t.is(projectBuildContextMock.discardIncrementalState.callCount, 1, + "discardIncrementalState called once on the failing project to discard its partial in-memory state"); + t.is(deregisterCleanupSigHooksStub.callCount, 1, "_deregisterCleanupSigHooks got called once"); t.is(deregisterCleanupSigHooksStub.getCall(0).args[0], "cleanup sig hooks", "_deregisterCleanupSigHooks got called with correct arguments"); t.is(executeCleanupTasksStub.callCount, 1, "_executeCleanupTasksStub got called once"); }); +test("build: Abort does not reset the project", async (t) => { + const {graph, taskRepository, ProjectBuilder, sinon} = t.context; + + const builder = new ProjectBuilder({graph, taskRepository}); + + const filterProjectStub = sinon.stub().returns(true); + sinon.stub(builder, "_createProjectFilter").returns(filterProjectStub); + + // An aborted build is transient — the BuildServer re-queues it and the in-memory + // state is still trustworthy — so the failing-project reset must NOT fire. + const abortError = new Error("The operation was aborted"); + abortError.name = "AbortError"; + + const possiblyRequiresBuildStub = sinon.stub().returns(true); + const validateCacheStub = sinon.stub().resolves(false); + const buildProjectStub = sinon.stub().rejects(abortError); + const projectBuildContextMock = { + possiblyRequiresBuild: possiblyRequiresBuildStub, + validateCache: validateCacheStub, + buildProject: buildProjectStub, + discardIncrementalState: sinon.stub(), + getProject: sinon.stub().returns(getMockProject("library")) + }; + sinon.stub(builder._buildContext, "getRequiredProjectContexts") + .resolves(new Map().set("project.a", projectBuildContextMock)); + + sinon.stub(builder, "_registerCleanupSigHooks").returns("cleanup sig hooks"); + sinon.stub(builder, "_writeResults").resolves(); + sinon.stub(builder, "_deregisterCleanupSigHooks"); + sinon.stub(builder, "_executeCleanupTasks").resolves(); + + const err = await t.throwsAsync(builder.buildToTarget({ + destPath: "dest/path", + includedDependencies: ["dep a"], + excludedDependencies: ["dep b"] + })); + + t.is(err.name, "AbortError", "Threw the abort error"); + t.is(projectBuildContextMock.discardIncrementalState.callCount, 0, + "discardIncrementalState not called on abort — the in-memory state is still trustworthy"); +}); + test.serial("build: Multiple projects", async (t) => { const {graph, taskRepository, sinon} = t.context; diff --git a/packages/project/test/lib/build/cache/ProjectBuildCache.js b/packages/project/test/lib/build/cache/ProjectBuildCache.js index bf7756b93d3..2487c062419 100644 --- a/packages/project/test/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/test/lib/build/cache/ProjectBuildCache.js @@ -42,6 +42,10 @@ function createMockProject(name = "test.project", id = "test-project-id") { buildFinished: sinon.stub(), setFrozenSourceReader: sinon.stub(), importTagOperations: sinon.stub(), + reset: sinon.stub().callsFake(() => { + currentStage = {getId: () => "initial"}; + stages.clear(); + }), }; return { @@ -337,6 +341,39 @@ test("discardIncrementalState re-arms a full source re-scan", async (t) => { "discardIncrementalState re-armed initSourceIndex to re-glob the source tree"); }); +test("discardIncrementalState clears the retained result signature and resets ProjectResources", async (t) => { + // A build that threw mid-execution leaves #currentResultSignature at the previous + // successful build's value and the stage pipeline holding partial output. If reset + // kept that signature, the next validateCache would take the "result stage signature + // unchanged" early return in #findResultCache, mark the project usesCache, and serve + // the failed build's partial stages. Assert reset drops the project out of FRESH and + // resets the stage pipeline so cached stages are re-imported on the next build. + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + + const src = createMockResource("/test.js", "hash-src", 1000, 100, 1); + project.getSourceReader.callsFake(() => ({ + byGlob: sinon.stub().resolves([src]), + byPath: sinon.stub().resolves(src), + })); + + const cache = await ProjectBuildCache.create(project, "sig", cacheManager); + await cache.initSourceIndex(); + // A completed build sets #currentResultSignature via allTasksCompleted. + await cache.allTasksCompleted(); + t.true(cache.isFresh(), "cache is fresh after the initial build"); + + const resetStub = project.getProjectResources().reset; + const resetCallsBefore = resetStub.callCount; + + cache.discardIncrementalState(); + + t.true(resetStub.callCount > resetCallsBefore, + "discardIncrementalState resets the project's stage pipeline so cached stages re-import"); + t.false(cache.isFresh(), + "cache is no longer fresh, so the retained result signature can't serve stale stages"); +}); + test("discardIncrementalState is a no-op in Cache.Off mode", async (t) => { const project = createMockProject(); const cacheManager = createMockCacheManager(); diff --git a/packages/project/test/lib/resources/ProjectResources.js b/packages/project/test/lib/resources/ProjectResources.js index e59adcda215..0d72a1a19cc 100644 --- a/packages/project/test/lib/resources/ProjectResources.js +++ b/packages/project/test/lib/resources/ProjectResources.js @@ -112,6 +112,30 @@ test("initStages clears frozen source reader", (t) => { t.not(reader1, reader2, "Reader was recreated after initStages"); }); +test("reset: returns to the initial stage after a stage switch", (t) => { + const {pr} = createProjectResources(); + + // Simulate a build that switched to a task stage (as a failed build would leave it) + pr.initStages(["task/taskA", "task/taskB"]); + pr.useStage("task/taskA"); + t.is(pr.getStage().getId(), "task/taskA", "On a task stage before reset"); + + pr.reset(); + + t.is(pr.getStage().getId(), "initial", "Back on the initial stage after reset"); +}); + +test("reset: clears the frozen source reader", (t) => { + const frozenReader = {name: "frozen-cas-reader"}; + const {pr} = createProjectResources({frozenSourceReader: frozenReader}); + + const reader1 = pr.getReader(); + pr.reset(); + const reader2 = pr.getReader(); + + t.not(reader1, reader2, "Reader was recreated after reset (frozen reader dropped)"); +}); + test("Frozen source reader takes priority over filesystem source reader", async (t) => { const resourcePath = "/resources/test/some.js"; const filesystemContent = "filesystem content"; From ebeb25c753f0275715f8523695e0a0e493a356f5 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:06 +0200 Subject: [PATCH 06/16] refactor(project): Emit sourcesChanged on leading edge with trailing settle sourcesChanged drives live-reload, so a lone edit should reach clients fast. On small projects the triggered build finishes well under 100 ms, so the previous 100 ms trailing debounce dominated edit-to-reload latency. Emit on the leading edge: the first change of a quiet period notifies immediately, and a trailing settle window coalesces the rest of a burst into one further emit. A single edit now reaches clients at the watcher's own latency floor. Raise the window to 550 ms, above @parcel/watcher's 500 ms coalescing cap. The watcher delivers a continuous operation like `git checkout` as batches up to 500 ms apart; a window below the cap would emit per batch, while one above it lets each batch reset the window so the whole operation collapses to one leading plus one trailing emit. With leading-edge emission the window size no longer affects single-edit latency. Rename SOURCES_CHANGED_DEBOUNCE_MS to SOURCES_CHANGED_SETTLE_MS. --- packages/project/lib/build/BuildServer.js | 40 +++++++-- .../project/test/lib/build/BuildServer.js | 87 ++++++++++--------- 2 files changed, 79 insertions(+), 48 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 2913eec75ab..1935171d65e 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -7,9 +7,21 @@ import {getLogger} from "@ui5/logger"; import ServeLogger from "@ui5/logger/internal/loggers/Serve"; const log = getLogger("build:BuildServer"); -// Debounce window for the `sourcesChanged` event so a burst of file changes -// results in a single notification. -const SOURCES_CHANGED_DEBOUNCE_MS = 100; +// Settle window for the `sourcesChanged` event, in milliseconds. +// +// The event drives live-reload notifications, so a lone edit must reach connected clients as +// fast as possible: the build it triggers can complete in well under 100 ms on small projects, +// and a trailing debounce would then dominate the edit-to-reload latency. The emit is therefore +// leading-edge — the first change of a quiet period fires immediately — followed by this +// suppression window that coalesces the rest of a burst into a single trailing emit. +// +// The value is tied to @parcel/watcher's MAX_WAIT_TIME (500 ms): the watcher caps its own +// coalescing at that interval, so an operation emitting changes continuously (e.g. `git checkout`) +// is delivered as batches up to 500 ms apart rather than one quiet-terminated batch. A window +// below that cap would see quiet between batches and emit once per batch; keeping it above the cap +// lets each batch reset the window so the whole operation collapses to one leading + one trailing +// emit. Do not lower below 500 ms without revisiting that relationship. +const SOURCES_CHANGED_SETTLE_MS = 550; // Loop protection for watcher recovery. A persistently failing watcher (e.g. a watched // path that keeps erroring on re-subscribe, or an FS that keeps dropping events) would @@ -67,6 +79,10 @@ class BuildServer extends EventEmitter { #activeBuild = null; #processBuildRequestsTimeout; #sourcesChangedTimeout; + // True while a trailing `sourcesChanged` emit is owed: set when a change lands inside the + // settle window (the leading emit already fired), cleared when the trailing emit fires or the + // server is destroyed. + #sourcesChangedPending = false; #destroyed = false; #allReader; #rootReader; @@ -318,6 +334,7 @@ class BuildServer extends EventEmitter { this.#destroyed = true; clearTimeout(this.#processBuildRequestsTimeout); clearTimeout(this.#sourcesChangedTimeout); + this.#sourcesChangedPending = false; await this.#watchHandler.destroy(); try { // Cancel any running background validation pass and wait for it to settle. @@ -525,14 +542,23 @@ class BuildServer extends EventEmitter { this.#setState(SERVER_STATES.STALE); } - // Debounced emit so a burst of file changes results in a single reload notification + // Leading-edge emit with a trailing settle window. The first change of a quiet period + // notifies immediately (a lone edit reaches clients at the watcher's own latency floor); + // further changes within the window only push the trailing emit out, so a burst collapses + // into one leading + one trailing notification. if (this.#sourcesChangedTimeout) { clearTimeout(this.#sourcesChangedTimeout); + this.#sourcesChangedPending = true; + } else { + this.emit("sourcesChanged"); } this.#sourcesChangedTimeout = setTimeout(() => { this.#sourcesChangedTimeout = null; - this.emit("sourcesChanged"); - }, SOURCES_CHANGED_DEBOUNCE_MS); + if (this.#sourcesChangedPending) { + this.#sourcesChangedPending = false; + this.emit("sourcesChanged"); + } + }, SOURCES_CHANGED_SETTLE_MS); } #flushResourceChanges() { @@ -1282,6 +1308,6 @@ export default BuildServer; /* istanbul ignore else */ if (process.env.NODE_ENV === "test") { BuildServer.__internals__ = { - SOURCES_CHANGED_DEBOUNCE_MS + SOURCES_CHANGED_SETTLE_MS }; } diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 98db0b73819..1e80bef5725 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -91,7 +91,7 @@ test.beforeEach(async (t) => { "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, })).default; t.context.BuildServer = BuildServer; - t.context.SOURCES_CHANGED_DEBOUNCE_MS = BuildServer.__internals__.SOURCES_CHANGED_DEBOUNCE_MS; + t.context.SOURCES_CHANGED_SETTLE_MS = BuildServer.__internals__.SOURCES_CHANGED_SETTLE_MS; // Use the static factory so #watchHandler is initialized — needed for destroy() in some tests. t.context.buildServer = await BuildServer.create( t.context.graph, t.context.projectBuilder, false, [], []); @@ -101,84 +101,89 @@ test.afterEach.always((t) => { t.context.sinon.restore(); }); -test.serial("sourcesChanged: emitted once after debounce window for a single change", (t) => { - const {buildServer, rootProject, clock, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; +test.serial("sourcesChanged: emitted immediately for a single change (leading edge)", (t) => { + const {buildServer, rootProject, clock, SOURCES_CHANGED_SETTLE_MS} = t.context; const listener = t.context.sinon.stub(); buildServer.on("sourcesChanged", listener); buildServer._projectResourceChanged(rootProject, "/foo.js", false); - t.is(listener.callCount, 0, "Not emitted synchronously"); + t.is(listener.callCount, 1, "Leading edge: emitted immediately on the first change"); - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS - 1); - t.is(listener.callCount, 0, "Not emitted before window elapses"); - - clock.tick(1); - t.is(listener.callCount, 1, "Emitted exactly once after window elapses"); + // A lone change owes no trailing emit — the settle window elapses silently. + clock.tick(SOURCES_CHANGED_SETTLE_MS); + t.is(listener.callCount, 1, "No trailing emit for a single change"); }); -test.serial("sourcesChanged: burst of changes within window collapses to one emit", (t) => { - const {buildServer, rootProject, clock, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; +test.serial("sourcesChanged: burst collapses to one leading + one trailing emit", (t) => { + const {buildServer, rootProject, clock, SOURCES_CHANGED_SETTLE_MS} = t.context; const listener = t.context.sinon.stub(); buildServer.on("sourcesChanged", listener); - // 5 rapid changes, each well within the debounce window. + // 5 rapid changes, each well within the settle window. for (let i = 0; i < 5; i++) { buildServer._projectResourceChanged(rootProject, `/foo${i}.js`, false); clock.tick(10); } - t.is(listener.callCount, 0, "No emit while bursts are within window"); + t.is(listener.callCount, 1, "Leading edge fired once; the rest of the burst is suppressed"); - // Advance past the remaining debounce window from the last change. - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS); - t.is(listener.callCount, 1, "Burst collapsed to a single emit"); + // Advance past the settle window from the last change → single trailing emit. + clock.tick(SOURCES_CHANGED_SETTLE_MS); + t.is(listener.callCount, 2, "Burst collapsed to one leading + one trailing emit"); }); -test.serial("sourcesChanged: each change resets the debounce timer", (t) => { - const {buildServer, rootProject, clock, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; +test.serial("sourcesChanged: each change resets the trailing settle timer", (t) => { + const {buildServer, rootProject, clock, SOURCES_CHANGED_SETTLE_MS} = t.context; const listener = t.context.sinon.stub(); buildServer.on("sourcesChanged", listener); buildServer._projectResourceChanged(rootProject, "/a.js", false); - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS - 10); - // Second change just before the original timer would fire — must reset, not fire-then-reset. + t.is(listener.callCount, 1, "Leading edge emitted on the first change"); + + clock.tick(SOURCES_CHANGED_SETTLE_MS - 10); + // Second change just before the trailing timer would fire — must reset it. buildServer._projectResourceChanged(rootProject, "/b.js", false); - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS - 10); - t.is(listener.callCount, 0, - "Second change reset the timer; no emit yet despite > debounce window of total elapsed time"); + clock.tick(SOURCES_CHANGED_SETTLE_MS - 10); + t.is(listener.callCount, 1, + "Second change reset the trailing timer; no trailing emit yet despite > settle window elapsed"); clock.tick(10); - t.is(listener.callCount, 1, "Emitted once the reset window elapses"); + t.is(listener.callCount, 2, "Trailing emit fires once the reset window elapses"); }); -test.serial("sourcesChanged: separate change windows produce separate emits", (t) => { - const {buildServer, rootProject, clock, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; +test.serial("sourcesChanged: a change after the window settles fires a fresh leading edge", (t) => { + const {buildServer, rootProject, clock, SOURCES_CHANGED_SETTLE_MS} = t.context; const listener = t.context.sinon.stub(); buildServer.on("sourcesChanged", listener); buildServer._projectResourceChanged(rootProject, "/a.js", false); - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS); - t.is(listener.callCount, 1, "First window emitted"); + t.is(listener.callCount, 1, "First leading edge"); + // Let the window elapse with no further change — no trailing emit is owed. + clock.tick(SOURCES_CHANGED_SETTLE_MS); + t.is(listener.callCount, 1, "No trailing emit for the lone first change"); - // A change after the first emit starts a new debounce window. + // A change after the window closed starts a fresh leading edge. buildServer._projectResourceChanged(rootProject, "/b.js", false); - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS); - t.is(listener.callCount, 2, "Second window produced a second emit"); + t.is(listener.callCount, 2, "Second window fires its own leading edge immediately"); + clock.tick(SOURCES_CHANGED_SETTLE_MS); + t.is(listener.callCount, 2, "Still lone — no trailing emit"); }); -test.serial("sourcesChanged: destroy cancels a pending emit", async (t) => { - const {buildServer, rootProject, clock, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; +test.serial("sourcesChanged: destroy cancels a pending trailing emit", async (t) => { + const {buildServer, rootProject, clock, SOURCES_CHANGED_SETTLE_MS} = t.context; const listener = t.context.sinon.stub(); buildServer.on("sourcesChanged", listener); + // Two changes: leading edge fires, and a second within the window owes a trailing emit. buildServer._projectResourceChanged(rootProject, "/a.js", false); - t.is(listener.callCount, 0, "Pre-destroy: pending emit not yet fired"); + buildServer._projectResourceChanged(rootProject, "/b.js", false); + t.is(listener.callCount, 1, "Pre-destroy: only the leading edge has fired"); await buildServer.destroy(); - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS * 5); - t.is(listener.callCount, 0, "Pending sourcesChanged emit was cancelled by destroy()"); + clock.tick(SOURCES_CHANGED_SETTLE_MS * 5); + t.is(listener.callCount, 1, "Pending trailing sourcesChanged emit was cancelled by destroy()"); }); test.serial("serve-status: serve-ready emitted when no initial build is requested", async (t) => { @@ -193,14 +198,14 @@ test.serial("serve-status: serve-ready emitted when no initial build is requeste t.is(readyCalls.length, 1, "serve-ready emitted once when no initial build is enqueued"); }); -test.serial("serve-status: serve-stale emitted on debounced sources change", (t) => { - const {buildServer, rootProject, clock, sinon, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; +test.serial("serve-status: serve-stale emitted on sources change", (t) => { + const {buildServer, rootProject, clock, sinon, SOURCES_CHANGED_SETTLE_MS} = t.context; const statusHandler = sinon.stub(); process.on("ui5.serve-status", statusHandler); t.teardown(() => process.off("ui5.serve-status", statusHandler)); buildServer._projectResourceChanged(rootProject, "/foo.js", false); - clock.tick(SOURCES_CHANGED_DEBOUNCE_MS); + clock.tick(SOURCES_CHANGED_SETTLE_MS); const staleCalls = statusHandler.getCalls() .filter((c) => c.args[0]?.status === "serve-stale"); @@ -1363,7 +1368,7 @@ const droppedEventsError = () => test.serial( "watcher-recovery: dropped-events error recreates watcher and forces a full re-scan", async (t) => { - const {sinon, clock, SOURCES_CHANGED_DEBOUNCE_MS} = t.context; + const {sinon, clock, SOURCES_CHANGED_SETTLE_MS} = t.context; const {graph} = makeErrorGraphWithLibDep(); const {builder: projectBuilder} = makeRescanBuilder(sinon); const statusEvents = makeStatusRecorder(t); @@ -1388,7 +1393,7 @@ test.serial( t.true(projectBuilder.forceFullRescan.calledOnce, "full re-scan forced on the builder"); t.is(errorEvents.length, 0, "no fatal error event on successful recovery"); - await clock.tickAsync(SOURCES_CHANGED_DEBOUNCE_MS); + await clock.tickAsync(SOURCES_CHANGED_SETTLE_MS); t.true(sourcesChanged.calledOnce, "sourcesChanged emitted so clients reload"); const seq = statusEvents.map((e) => e.status); From 886f053193cd267d753ea5ad5bdceb9c21f16566 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:07 +0200 Subject: [PATCH 07/16] refactor(project): Defer build restart after abort until changes settle When a source change lands mid-build, the build is aborted and its projects re-queued. The restart used the same 10 ms debounce as a reader request, so a burst of changes restarted a build per batch, each aborted by the next. @parcel/watcher caps its coalescing at 500 ms, so a continuous operation (git checkout, save-all, a bundler writing many files) arrives as batches up to 500 ms apart, producing a build -> abort -> build cycle for the whole operation. Hold the post-abort restart until changes are quiet for 550 ms (above the watcher's cap), resetting the window on each further change via #_projectResourceChanged, so the burst collapses into a single rebuild. The delay is scoped to the speculative restart: a reader request still enqueues on BUILD_REQUEST_DEBOUNCE_MS and supersedes a pending restart, so serving a request is unaffected. Extract the inline 10 ms request-queue debounce into BUILD_REQUEST_DEBOUNCE_MS and give #triggerRequestQueue a delay parameter. --- packages/project/lib/build/BuildServer.js | 53 +++++++++++++++--- .../project/test/lib/build/BuildServer.js | 54 +++++++++++++++++++ 2 files changed, 99 insertions(+), 8 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 1935171d65e..894d221de31 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -23,6 +23,21 @@ const log = getLogger("build:BuildServer"); // emit. Do not lower below 500 ms without revisiting that relationship. const SOURCES_CHANGED_SETTLE_MS = 550; +// Debounce for the request queue. A reader request enqueues a build and triggers the queue after +// this short delay so a batch of near-simultaneous requests builds together. Kept small so the +// first build after a quiet period starts promptly. +const BUILD_REQUEST_DEBOUNCE_MS = 10; + +// Settle window for restarting a build that a source change aborted. When a change lands mid-build +// the running build is aborted immediately, but the restart is held until changes have been quiet +// for this long (each further change resets it). During a burst of changes — a `git checkout`, a +// save-all, a bundler writing many files — @parcel/watcher delivers batches up to its MAX_WAIT_TIME +// (500 ms) apart; restarting on the snappy BUILD_REQUEST_DEBOUNCE_MS would spawn a build per batch, +// each aborted by the next. Holding the restart above the watcher's cap collapses the burst into a +// single build against the settled tree. Reader-request-driven builds keep the snappy debounce, so +// this delay only applies to the speculative post-abort restart, not to serving a request. +const ABORTED_BUILD_RESTART_SETTLE_MS = 550; + // Loop protection for watcher recovery. A persistently failing watcher (e.g. a watched // path that keeps erroring on re-subscribe, or an FS that keeps dropping events) would // otherwise cycle error → recover → error indefinitely. If more than @@ -78,6 +93,10 @@ class BuildServer extends EventEmitter { #pendingBuildRequest = new Set(); #activeBuild = null; #processBuildRequestsTimeout; + // True while a source-change-aborted build waits out its settle window before restarting. + // A further source change during the window reschedules the restart (resetting the timer) + // instead of leaving the fired-once restart to race the still-arriving burst. + #pendingAbortRestart = false; #sourcesChangedTimeout; // True while a trailing `sourcesChanged` emit is owed: set when a change lands inside the // settle window (the leading emit already fired), cleared when the trailing emit fires or the @@ -333,6 +352,7 @@ class BuildServer extends EventEmitter { async destroy() { this.#destroyed = true; clearTimeout(this.#processBuildRequestsTimeout); + this.#pendingAbortRestart = false; clearTimeout(this.#sourcesChangedTimeout); this.#sourcesChangedPending = false; await this.#watchHandler.destroy(); @@ -542,6 +562,13 @@ class BuildServer extends EventEmitter { this.#setState(SERVER_STATES.STALE); } + // Reschedule a pending post-abort restart so its settle window measures quiet from this + // change, not from the abort. Only fires while a restart is waiting (no active build); + // #triggerRequestQueue clears the armed timer and re-arms it at the settle delay. + if (this.#pendingAbortRestart) { + this.#triggerRequestQueue(ABORTED_BUILD_RESTART_SETTLE_MS); + } + // Leading-edge emit with a trailing settle window. The first change of a quiet period // notifies immediately (a lone edit reaches clients at the watcher's own latency floor); // further changes within the window only push the trailing emit out, so a burst collapses @@ -596,7 +623,7 @@ class BuildServer extends EventEmitter { this.#triggerRequestQueue(); } - #triggerRequestQueue() { + #triggerRequestQueue(delay = BUILD_REQUEST_DEBOUNCE_MS) { if (this.#destroyed || this.#activeBuild || this.#recoveringWatcher) { // While recovering the watcher, suppress builds so ProjectBuilder.forceFullRescan() // can claim the builder without racing a build the abort/re-queue path may start. @@ -608,6 +635,9 @@ class BuildServer extends EventEmitter { clearTimeout(this.#processBuildRequestsTimeout); } this.#processBuildRequestsTimeout = setTimeout(async () => { + // The restart (if this was the post-abort one) is now running; further source + // changes should schedule a fresh restart rather than reset this fired timer. + this.#pendingAbortRestart = false; // Abort any in-flight background validation pass so the build can claim // the builder's "buildIsRunning" lock. Validation will be re-scheduled // after the build cycle drains. @@ -640,7 +670,7 @@ class BuildServer extends EventEmitter { // ServeLogger; keep the server alive. this.#setState(SERVER_STATES.ERROR, {error: err}); }); - }, 10); + }, delay); } /** @@ -758,11 +788,16 @@ class BuildServer extends EventEmitter { this.emit("buildFinished", builtProjects); if (signal.aborted) { log.verbose(`Build aborted for projects: ${projectsToBuild.join(", ")}`); - // Do not continue processing the queue if the build was aborted, but re-trigger processing debounced - // to ensure that any source changes are properly queued before the next build. - // This is also essential to re-trigger the build in case all resources changes have already been - // processed while the build was still aborting. Otherwise the build would not be re-triggered. - this.#triggerRequestQueue(); + // A source change aborted this build. Re-trigger processing so the re-queued + // projects rebuild — but hold the restart until changes settle rather than + // restarting on the snappy request debounce. A burst (git checkout, save-all) + // arrives as watcher batches up to MAX_WAIT_TIME apart; restarting between + // batches would spawn a build per batch, each aborted by the next. The settle + // delay collapses the burst into a single rebuild against the settled tree. + // Each further source change reschedules this restart (see #_projectResourceChanged), + // so the window measures quiet from the last change, not from the abort. + this.#pendingAbortRestart = true; + this.#triggerRequestQueue(ABORTED_BUILD_RESTART_SETTLE_MS); return; } // A successful build cycle proves the environment can build. Lift ERRORED gates on any @@ -1308,6 +1343,8 @@ export default BuildServer; /* istanbul ignore else */ if (process.env.NODE_ENV === "test") { BuildServer.__internals__ = { - SOURCES_CHANGED_SETTLE_MS + SOURCES_CHANGED_SETTLE_MS, + BUILD_REQUEST_DEBOUNCE_MS, + ABORTED_BUILD_RESTART_SETTLE_MS }; } diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 1e80bef5725..39e454c9d52 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -309,6 +309,60 @@ test.serial( `Expected stale after build-done; got: ${seq.join(", ")}`); }); +test.serial( + "abort-restart: a source-change-aborted build waits out the settle window before restarting", + async (t) => { + const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; + const {ABORTED_BUILD_RESTART_SETTLE_MS, BUILD_REQUEST_DEBOUNCE_MS} = + BuildServer.__internals__; + makeStatusRecorder(t); + + // Each build() invocation parks a resolver. Settling checks the abort signal and, when + // aborted, rejects with an AbortError — mirroring the real ProjectBuilder so BuildServer + // takes its abort re-queue path (which is what schedules the deferred restart). + const resolvers = []; + projectBuilder.build = sinon.stub().callsFake((opts, cb) => new Promise((resolve, reject) => { + resolvers.push(() => { + if (opts.signal?.aborted) { + const err = new Error("Build aborted"); + err.name = "AbortError"; + reject(err); + return; + } + cb("root.project", {getReader: () => ({fakeReader: true})}); + resolve(["root.project"]); + }); + })); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.context.buildServer = buildServer; + await clock.tickAsync(BUILD_REQUEST_DEBOUNCE_MS); + t.is(projectBuilder.build.callCount, 1, "initial build started"); + + // Mid-build source change aborts the running build, then let it settle. + buildServer._projectResourceChanged(rootProject, "/a.js", false); + resolvers[0](); + await clock.tickAsync(0); + t.is(projectBuilder.build.callCount, 1, "no restart yet — the settle window is open"); + + // Well within the settle window: still no restart. + await clock.tickAsync(ABORTED_BUILD_RESTART_SETTLE_MS - 50); + t.is(projectBuilder.build.callCount, 1, "restart still held mid-window"); + + // A further change resets the window — the restart must not fire at the original deadline. + buildServer._projectResourceChanged(rootProject, "/b.js", false); + await clock.tickAsync(ABORTED_BUILD_RESTART_SETTLE_MS - 50); + t.is(projectBuilder.build.callCount, 1, "second change reset the window; still no restart"); + + // Past the reset window: exactly one restart for the whole burst. + await clock.tickAsync(50); + t.is(projectBuilder.build.callCount, 2, "burst collapsed to a single restarted build"); + + // Settle the restarted build so no promise is left dangling. + resolvers[1]?.(); + await clock.tickAsync(0); + }); + test.serial("serve-status: build failure emits serveError and no orphan building", async (t) => { const {BuildServer, graph, projectBuilder, sinon, clock} = t.context; const statusEvents = makeStatusRecorder(t); From 3f3c34addf35fccee34c7b57a8691ac5c7961061 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:08 +0200 Subject: [PATCH 08/16] refactor(project): Add SETTLING server state for deferred rebuilds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During a `git checkout` the serve status could stay on `building` or flip to red `error` while nothing ran, until the checkout finished. The banner claimed a build was in progress when #activeBuild was already null. The deferred post-abort restart had no state to report: during its settle window the server is waiting for changes to stop, but SERVER_STATES had no value meaning that, so it showed whatever the last transition left. Add a SETTLING lifecycle state: changes seen, rebuild pending, holding until changes go quiet. It sits between STALE and BUILDING with no active build (#activeBuild === null); the deferred timer moves it to BUILDING when the window elapses. Route both deferral sites through it: - Post-abort restart reports SETTLING instead of leaving the banner on `building`. - Failure-with-pending-changes (the signal.aborted || #resourceChangeQueue.size > 0 branch) now defers and reports like the abort path instead of parking on the failed build's state. This fixes the `git checkout` case: the doomed first build reports SETTLING and retries once the tree is quiet instead of a spurious serve-error. Genuine non-transient failures still latch ERRORED. BUILDING -> SETTLING skips the buildDone emission (like BUILDING -> ERROR), and #reconcileServerState bails on SETTLING so the deferred-restart timer owns the SETTLING -> BUILDING transition. ServeLogger gains settling(pendingProjects) emitting serve-settling, and the interactive banner renders a spinning "settling · waiting for changes to settle" line. --- packages/logger/lib/loggers/Serve.js | 10 +- .../logger/lib/writers/InteractiveConsole.js | 4 + .../lib/writers/interactiveConsole/render.js | 5 + .../writers/interactiveConsole/state/build.js | 6 +- packages/logger/test/lib/loggers/Serve.js | 32 ++++++ .../test/lib/writers/InteractiveConsole.js | 18 ++++ .../lib/writers/interactiveConsole/render.js | 7 ++ packages/project/lib/build/BuildServer.js | 73 +++++++++---- .../test/lib/build/BuildServer.integration.js | 40 +++++++ .../project/test/lib/build/BuildServer.js | 102 ++++++++++++++++-- 10 files changed, 263 insertions(+), 34 deletions(-) diff --git a/packages/logger/lib/loggers/Serve.js b/packages/logger/lib/loggers/Serve.js index 2146365ddc0..edac075bde8 100644 --- a/packages/logger/lib/loggers/Serve.js +++ b/packages/logger/lib/loggers/Serve.js @@ -3,7 +3,7 @@ import Logger from "./Logger.js"; /** * Logger for emitting status events on the lifecycle of a UI5 development server - * (idle / stale / building / build-done / error). + * (idle / stale / settling / building / build-done / error). *

* Emits ui5.log and ui5.serve-status events on the * [process]{@link https://nodejs.org/api/process.html} object, which can be handled @@ -46,6 +46,14 @@ class Serve extends Logger { `Sources changed in: ${changedProjects.join(", ")}`); } + settling(pendingProjects) { + if (!pendingProjects || !Array.isArray(pendingProjects)) { + throw new Error("loggers/Serve#settling: Missing or incorrect pendingProjects parameter"); + } + this.#emitStatus("serve-settling", {pendingProjects}, + `Waiting for changes to settle in: ${pendingProjects.join(", ")}`); + } + validating(validatingProjects) { if (!validatingProjects || !Array.isArray(validatingProjects)) { throw new Error( diff --git a/packages/logger/lib/writers/InteractiveConsole.js b/packages/logger/lib/writers/InteractiveConsole.js index d7d6441b5d0..3fa34001ab4 100644 --- a/packages/logger/lib/writers/InteractiveConsole.js +++ b/packages/logger/lib/writers/InteractiveConsole.js @@ -354,6 +354,10 @@ class InteractiveConsole { this.#buildState.changedProjects = evt.changedProjects || []; this.#transitionTo(STATES.STALE); break; + case "serve-settling": + this.#buildState.pendingProjects = evt.pendingProjects || []; + this.#transitionTo(STATES.SETTLING); + break; case "serve-building": beginBuild(this.#buildState, this.#buildState.projectOrder); this.#transitionTo(STATES.BUILDING); diff --git a/packages/logger/lib/writers/interactiveConsole/render.js b/packages/logger/lib/writers/interactiveConsole/render.js index 615aadbf0d0..b189ba617bf 100644 --- a/packages/logger/lib/writers/interactiveConsole/render.js +++ b/packages/logger/lib/writers/interactiveConsole/render.js @@ -150,6 +150,11 @@ function renderStatusLine(state) { case STATES.STALE: return `${label}${chalk.yellow(figures.circle)} ${chalk.yellow(pad("stale"))} ` + `${chalk.dim("· files changed, rebuild on next request")}`; + case STATES.SETTLING: { + const frame = SPINNER_FRAMES[state.spinFrame % SPINNER_FRAMES.length]; + return `${label}${chalk.yellow(frame)} ${chalk.yellow(pad("settling"))} ` + + `${chalk.dim("· waiting for changes to settle")}`; + } case STATES.VALIDATING: { const frame = SPINNER_FRAMES[state.spinFrame % SPINNER_FRAMES.length]; const parts = [ diff --git a/packages/logger/lib/writers/interactiveConsole/state/build.js b/packages/logger/lib/writers/interactiveConsole/state/build.js index 182b5e7cfc3..51b504112c2 100644 --- a/packages/logger/lib/writers/interactiveConsole/state/build.js +++ b/packages/logger/lib/writers/interactiveConsole/state/build.js @@ -6,6 +6,7 @@ export const STATES = Object.freeze({ STARTING: "starting", // pre-populated placeholder before the first real state arrives READY: "ready", STALE: "stale", + SETTLING: "settling", // changes seen, rebuild deferred until they quiesce BUILDING: "building", VALIDATING: "validating", ERROR: "error", @@ -14,7 +15,7 @@ export const STATES = Object.freeze({ // States that animate a spinner. Consulted by both the tick scheduler in the // interactive console writer and the status-line renderer, so a state either // spins in both places or in neither. -export const SPINNING_STATES = new Set([STATES.BUILDING, STATES.VALIDATING]); +export const SPINNING_STATES = new Set([STATES.SETTLING, STATES.BUILDING, STATES.VALIDATING]); export function createBuildState() { return { @@ -31,6 +32,9 @@ export function createBuildState() { // Names of projects collected via `serve-validating` payloads — used to // label the validating state if/when the renderer wants to. validatingProjects: [], + // Names of projects collected via `serve-settling` payloads — used to + // label the settling state if/when the renderer wants to. + pendingProjects: [], // Frame counter for the spinner (incremented by the tick loop). spinFrame: 0, // Most recent error captured by `serve-error`. diff --git a/packages/logger/test/lib/loggers/Serve.js b/packages/logger/test/lib/loggers/Serve.js index 70124d3ec20..2c6fcebcf27 100644 --- a/packages/logger/test/lib/loggers/Serve.js +++ b/packages/logger/test/lib/loggers/Serve.js @@ -56,6 +56,26 @@ test.serial("stale: Missing parameter", (t) => { }, "Threw with expected error message"); }); +test.serial("settling emits serve-settling", (t) => { + const {serveLogger, statusHandler} = t.context; + serveLogger.settling(["project.a", "project.b"]); + t.is(statusHandler.callCount, 1, "One serve-status event emitted"); + t.deepEqual(statusHandler.getCall(0).args[0], { + level: "info", + status: "serve-settling", + pendingProjects: ["project.a", "project.b"], + }, "Status event has expected payload"); +}); + +test.serial("settling: Missing parameter", (t) => { + const {serveLogger} = t.context; + t.throws(() => { + serveLogger.settling(); + }, { + message: "loggers/Serve#settling: Missing or incorrect pendingProjects parameter", + }, "Threw with expected error message"); +}); + test.serial("validating emits serve-validating", (t) => { const {serveLogger, statusHandler} = t.context; serveLogger.validating(["library.a", "library.b"]); @@ -161,6 +181,18 @@ test.serial("No event listener: validating", (t) => { t.is(logHandler.callCount, 0, "No log event emitted"); }); +test.serial("No event listener: settling", (t) => { + const {serveLogger, statusHandler, logHandler, logStub} = t.context; + process.off(ServeLogger.SERVE_STATUS_EVENT_NAME, statusHandler); + serveLogger.settling(["project.a", "project.b"]); + t.is(logStub.callCount, 1, "_log got called once"); + t.is(logStub.getCall(0).args[0], "info", "Logged with expected log-level"); + t.is(logStub.getCall(0).args[1], + "Waiting for changes to settle in: project.a, project.b", + "Logged expected message"); + t.is(logHandler.callCount, 0, "No log event emitted"); +}); + test.serial("No event listener: building", (t) => { const {serveLogger, statusHandler, logHandler, logStub} = t.context; process.off(ServeLogger.SERVE_STATUS_EVENT_NAME, statusHandler); diff --git a/packages/logger/test/lib/writers/InteractiveConsole.js b/packages/logger/test/lib/writers/InteractiveConsole.js index c15a8a50751..6762e170dbc 100644 --- a/packages/logger/test/lib/writers/InteractiveConsole.js +++ b/packages/logger/test/lib/writers/InteractiveConsole.js @@ -535,6 +535,24 @@ test.serial("serve-status: serve-stale without a payload falls back to an empty writer.disable(); }); +test.serial("serve-status: serve-settling records pendingProjects and transitions to SETTLING", (t) => { + const {writer, stderr} = createWriter(); + process.emit("ui5.serve-status", {status: "serve-settling", pendingProjects: ["my.app"]}); + const state = writer._getStateForTest().build; + t.is(state.state, STATES.SETTLING); + t.deepEqual(state.pendingProjects, ["my.app"]); + const tail = stripAnsi(stderr.writes.slice(-5).join("")); + t.regex(tail, /settling/); + writer.disable(); +}); + +test.serial("serve-status: serve-settling without a payload falls back to an empty list", (t) => { + const {writer} = createWriter(); + process.emit("ui5.serve-status", {status: "serve-settling"}); + t.deepEqual(writer._getStateForTest().build.pendingProjects, []); + writer.disable(); +}); + test.serial("serve-status: serve-validating records projects and transitions to VALIDATING", (t) => { const {writer, stderr} = createWriter(); process.emit("ui5.serve-status", { diff --git a/packages/logger/test/lib/writers/interactiveConsole/render.js b/packages/logger/test/lib/writers/interactiveConsole/render.js index 64b001d8fae..2bf541e1102 100644 --- a/packages/logger/test/lib/writers/interactiveConsole/render.js +++ b/packages/logger/test/lib/writers/interactiveConsole/render.js @@ -253,6 +253,13 @@ test("renderBuildRegion: stale state includes 'files changed' hint", (t) => { t.regex(plain, /Status\s+.+?\s+stale\s+·\s+files changed/); }); +test("renderBuildRegion: settling state includes 'waiting for changes to settle' hint", (t) => { + const state = createBuildState(); + transitionTo(state, STATES.SETTLING); + const plain = renderBuildRegion(state).map(stripAnsi).join("\n"); + t.regex(plain, /Status\s+.+?\s+settling\s+·\s+waiting for changes to settle/); +}); + test("renderBuildRegion: validating state includes 'checking dependency caches' hint", (t) => { const state = createBuildState(); transitionTo(state, STATES.VALIDATING); diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 894d221de31..9f08af398c0 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -47,9 +47,12 @@ const WATCHER_RECOVERY_MAX_ATTEMPTS = 5; const WATCHER_RECOVERY_WINDOW_MS = 60000; // The server's lifecycle state. Mutated exclusively through #setState. +// Ordering intent: IDLE → STALE → SETTLING → BUILDING. const SERVER_STATES = Object.freeze({ IDLE: "idle", // No pending requests, no recent changes, no unvalidated caches. STALE: "stale", // Pending changes / pending requests, queue not yet flushed. + SETTLING: "settling", // Rebuild pending, deferred until changes quiesce. No build is active + // (#activeBuild === null); the deferred timer moves it to BUILDING. BUILDING: "building", // A build is in flight. VALIDATING: "validating", // A background cache-validation pass is in flight. ERROR: "error", // Last build cycle failed. @@ -562,9 +565,10 @@ class BuildServer extends EventEmitter { this.#setState(SERVER_STATES.STALE); } - // Reschedule a pending post-abort restart so its settle window measures quiet from this - // change, not from the abort. Only fires while a restart is waiting (no active build); - // #triggerRequestQueue clears the armed timer and re-arms it at the settle delay. + // Reschedule a pending post-abort/transient restart so its settle window measures quiet + // from this change, not from the abort. Only fires while a restart is waiting (no active + // build); the state is already SETTLING and stays there. #triggerRequestQueue clears the + // armed timer and re-arms it at the settle delay. if (this.#pendingAbortRestart) { this.#triggerRequestQueue(ABORTED_BUILD_RESTART_SETTLE_MS); } @@ -717,6 +721,7 @@ class BuildServer extends EventEmitter { // Set active build to prevent concurrent builds let buildError = null; + let transientFailure = false; const buildPromise = this.#activeBuild = this.#projectBuilder.build({ includeRootProject: buildRootProject, includedDependencies: dependenciesToBuild, @@ -739,13 +744,16 @@ class BuildServer extends EventEmitter { } } } else if (signal.aborted || this.#resourceChangeQueue.size > 0) { - // Task threw while sources were changing (e.g. mid-`git pull`). The build state - // is untrustworthy but the error itself is very likely spurious — a fresh build - // against the settled tree will typically succeed. Silently re-queue affected - // projects and leave the reader-request queue intact so held requests resolve - // on the retry. + // Task threw while sources were changing (e.g. mid-`git checkout`). The build + // state is untrustworthy but the error itself is very likely spurious — a fresh + // build against the settled tree will typically succeed. Re-queue affected + // projects and leave the reader-request queue intact so held requests resolve on + // the retry, then route through the same defer-and-report path as an abort (see + // below): the doomed build must not park the server on `building`/`error`; it + // reports SETTLING and retries once the tree is quiet. log.warn( `Build failed during concurrent source change — treating as transient: ${err.message}`); + transientFailure = true; for (const projectName of projectsToBuild) { const projectBuildStatus = this.#projectBuildStatus.get(projectName); if (!projectBuildStatus.isFresh()) { @@ -786,17 +794,24 @@ class BuildServer extends EventEmitter { continue; } this.emit("buildFinished", builtProjects); - if (signal.aborted) { - log.verbose(`Build aborted for projects: ${projectsToBuild.join(", ")}`); - // A source change aborted this build. Re-trigger processing so the re-queued - // projects rebuild — but hold the restart until changes settle rather than - // restarting on the snappy request debounce. A burst (git checkout, save-all) - // arrives as watcher batches up to MAX_WAIT_TIME apart; restarting between - // batches would spawn a build per batch, each aborted by the next. The settle - // delay collapses the burst into a single rebuild against the settled tree. - // Each further source change reschedules this restart (see #_projectResourceChanged), - // so the window measures quiet from the last change, not from the abort. + if (signal.aborted || transientFailure) { + log.verbose(`Build ${signal.aborted ? "aborted" : "failed transiently"} ` + + `for projects: ${projectsToBuild.join(", ")}`); + // A source change aborted this build, or the build failed while sources were still + // A source change aborted this build, or the build failed while sources were still + // changing. Re-trigger processing so the re-queued projects rebuild — but hold the + // restart until changes settle rather than restarting on the short request debounce. + // A burst (git checkout, save-all) arrives as watcher batches up to MAX_WAIT_TIME + // apart; restarting between batches would spawn a build per batch, each aborted by + // the next. The settle delay collapses the burst into a single rebuild against the + // settled tree. Each further source change reschedules this restart (see + // #_projectResourceChanged), so the window measures quiet from the last change. + // + // Report SETTLING for the duration of the window: the server is waiting for changes + // to quiesce before rebuilding, so leaving the banner on `building` (abort) or + // flipping it to `error` (transient failure) would misdescribe what's happening. this.#pendingAbortRestart = true; + this.#setState(SERVER_STATES.SETTLING, {pendingProjects: this.#getStaleProjectNames()}); this.#triggerRequestQueue(ABORTED_BUILD_RESTART_SETTLE_MS); return; } @@ -876,6 +891,11 @@ class BuildServer extends EventEmitter { * - #destroyed — server shutting down; state is irrelevant. * - #serverState === ERROR — a producer already settled us on ERROR; * don't paint over it with a successful cycle close. + * - #serverState === SETTLING — a deferred restart is armed; the timer owns + * the SETTLING → BUILDING transition. Every SETTLING entry leaves + * #pendingBuildRequest non-empty (so the check below already bails), but the + * explicit guard keeps a stray reconcile from flipping SETTLING to IDLE/STALE should that + * invariant ever break. * - #activeBuild || #pendingBuildRequest.size > 0 — a build cycle owns * the next transition. Notably fires when this call comes from the post-validation * finally and releaseValidating's onBuildRequired callback @@ -895,7 +915,8 @@ class BuildServer extends EventEmitter { * schedule a background validation pass. Post-build → true, post-validation → false. */ #reconcileServerState({hrtime, mayValidate = false} = {}) { - if (this.#destroyed || this.#serverState === SERVER_STATES.ERROR) { + if (this.#destroyed || this.#serverState === SERVER_STATES.ERROR || + this.#serverState === SERVER_STATES.SETTLING) { return; } if (this.#activeBuild || this.#pendingBuildRequest.size > 0) { @@ -1045,9 +1066,13 @@ class BuildServer extends EventEmitter { *
  • BUILDING → IDLE: buildDone(hrtime) then ready()
    • *
  • BUILDING → STALE: buildDone(hrtime) then stale(names)
    • *
  • BUILDING → VALIDATING: buildDone(hrtime) then validating(names)
    • + *
  • BUILDING → SETTLING: settling(names) only — no buildDone, since no successful + * cycle closed (mirrors the BUILDING → ERROR carve-out)
    • *
  • BUILDING → ERROR: serveError(err) only — buildDone is skipped so * consumers don't see a successful cycle close before the error
    • *
  • VALIDATING → IDLE/STALE: ready()/stale() — no buildDone (no build happened)
    • + *
  • * → SETTLING: settling(names)
    • + *
  • SETTLING → BUILDING: building()
    • *
  • any → ERROR: serveError(err)
    • *
  • * → IDLE/STALE/BUILDING/VALIDATING: ready()/stale()/building()/validating() as appropriate
    • * @@ -1058,18 +1083,21 @@ class BuildServer extends EventEmitter { * tuple produced by process.hrtime(start); required when leaving * BUILDING for IDLE/STALE/VALIDATING. * @param {string[]} [opts.staleProjects] Stale project names; required when transitioning to STALE. + * @param {string[]} [opts.pendingProjects] Names of projects awaiting the deferred rebuild; + * used when transitioning to SETTLING. Defaults to the stale project set. * @param {string[]} [opts.validatingProjects] Names of projects undergoing background cache * validation; required when transitioning to VALIDATING. * @param {Error} [opts.error] Error instance; required when transitioning to ERROR. */ - #setState(next, {hrtime, staleProjects, validatingProjects, error} = {}) { + #setState(next, {hrtime, staleProjects, pendingProjects, validatingProjects, error} = {}) { if (this.#serverState === next) { return; } const previous = this.#serverState; this.#serverState = next; - if (previous === SERVER_STATES.BUILDING && next !== SERVER_STATES.ERROR) { + if (previous === SERVER_STATES.BUILDING && + next !== SERVER_STATES.ERROR && next !== SERVER_STATES.SETTLING) { this.#serveLogger.buildDone(hrtime ?? [0, 0]); } @@ -1080,6 +1108,9 @@ class BuildServer extends EventEmitter { case SERVER_STATES.STALE: this.#serveLogger.stale(staleProjects ?? this.#getStaleProjectNames()); break; + case SERVER_STATES.SETTLING: + this.#serveLogger.settling(pendingProjects ?? this.#getStaleProjectNames()); + break; case SERVER_STATES.BUILDING: this.#serveLogger.building(); break; diff --git a/packages/project/test/lib/build/BuildServer.integration.js b/packages/project/test/lib/build/BuildServer.integration.js index 2018762e531..a450d5a037c 100644 --- a/packages/project/test/lib/build/BuildServer.integration.js +++ b/packages/project/test/lib/build/BuildServer.integration.js @@ -164,6 +164,46 @@ test.serial("Serve application.a, initial file changes", async (t) => { t.true(servedFileContent.includes(`test("third change");`), "Resource contains third changed file content"); }); +// Complements the unit-level transient-failure coverage with a real-timer end-to-end pass: a +// burst of rapid watcher events arrives while a reader request is parked. The extra first-build +// (100 ms) and post-abort/transient (550 ms) settle windows must not hang the request, and the +// transient aborts within the burst must never surface a `serve-error` on the status feed — the +// server reports `serve-settling` while holding, then resolves on the single settled-tree rebuild. +test.serial("Serve application.a, rapid change burst reports settling and never errors", async (t) => { + const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a"); + + const statusEvents = []; + const statusHandler = (evt) => statusEvents.push(evt.status); + process.on("ui5.serve-status", statusHandler); + t.teardown(() => process.off("ui5.serve-status", statusHandler)); + + await fixtureTester.serveProject(); + + const changedFilePath = `${fixtureTester.fixturePath}/webapp/test.js`; + + // Park a request, then fire several rapid changes around it — the shape of an editor save-all + // or a `git checkout` landing while a build is in flight. + await fs.appendFile(changedFilePath, `\ntest("burst 1");\n`); + await fixtureTester.fireWatcherEvent("update", changedFilePath); + + const resourceRequestPromise = fixtureTester.requestResource({resource: "/test.js"}); + + await fs.appendFile(changedFilePath, `\ntest("burst 2");\n`); + await fixtureTester.fireWatcherEvent("update", changedFilePath); + await fs.appendFile(changedFilePath, `\ntest("burst 3");\n`); + await fixtureTester.fireWatcherEvent("update", changedFilePath); + + await resourceRequestPromise; + + const resource = await fixtureTester.requestResource({resource: "/test.js"}); + const servedFileContent = await resource.getString(); + t.true(servedFileContent.includes(`test("burst 1");`), "Resource reflects the first burst change"); + t.true(servedFileContent.includes(`test("burst 3");`), "Resource reflects the final burst change"); + + t.false(statusEvents.includes("serve-error"), + `No serve-error surfaced during the transient burst; got: ${statusEvents.join(", ")}`); +}); + test.serial("Serve application.a, request application resource", async (t) => { const fixtureTester = t.context.fixtureTester = await FixtureTester.create(t, "application.a"); diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 39e454c9d52..0f2e5c47b61 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -271,7 +271,7 @@ test.serial("serve-status: initial build cycle emits building → buildDone → }); test.serial( - "serve-status: source change mid-build emits exactly one stale at cycle end", async (t) => { + "serve-status: source change mid-build emits settling at cycle end (not stale)", async (t) => { const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; const statusEvents = makeStatusRecorder(t); @@ -285,6 +285,7 @@ test.serial( }); const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.teardown(() => buildServer.destroy()); await clock.tickAsync(10); t.true(projectBuilder.build.called, "build started"); @@ -298,15 +299,18 @@ test.serial( await clock.tickAsync(0); const seq = statusEvents.map((e) => e.status); - const staleCalls = seq.filter((s) => s === "serve-stale"); - t.is(staleCalls.length, 1, - `Expected exactly one stale emission at cycle end, got sequence: ${seq.join(", ")}`); - // And the emission must come AFTER serve-build-done (i.e. it's the - // end-of-cycle one, not the IDLE→STALE one). - const lastStaleIdx = seq.lastIndexOf("serve-stale"); - const lastBuildDoneIdx = seq.lastIndexOf("serve-build-done"); - t.true(lastStaleIdx > lastBuildDoneIdx, - `Expected stale after build-done; got: ${seq.join(", ")}`); + // The aborted cycle defers its restart, so the end-of-cycle state is SETTLING + // (rebuild pending, held until changes quiesce) rather than STALE. No buildDone + // is emitted on BUILDING → SETTLING — no successful cycle closed. + const settlingCalls = seq.filter((s) => s === "serve-settling"); + t.is(settlingCalls.length, 1, + `Expected exactly one settling emission at cycle end, got sequence: ${seq.join(", ")}`); + t.false(seq.includes("serve-stale"), + `No stale emission on the aborted cycle; got: ${seq.join(", ")}`); + t.false(seq.includes("serve-build-done"), + `No buildDone on BUILDING → SETTLING; got: ${seq.join(", ")}`); + t.is(seq[seq.length - 1], "serve-settling", + `SETTLING is the terminal state of the deferred cycle; got: ${seq.join(", ")}`); }); test.serial( @@ -315,7 +319,7 @@ test.serial( const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; const {ABORTED_BUILD_RESTART_SETTLE_MS, BUILD_REQUEST_DEBOUNCE_MS} = BuildServer.__internals__; - makeStatusRecorder(t); + const statusEvents = makeStatusRecorder(t); // Each build() invocation parks a resolver. Settling checks the abort signal and, when // aborted, rejects with an AbortError — mirroring the real ProjectBuilder so BuildServer @@ -344,6 +348,9 @@ test.serial( resolvers[0](); await clock.tickAsync(0); t.is(projectBuilder.build.callCount, 1, "no restart yet — the settle window is open"); + // The deferred restart reports SETTLING, not a stale/building banner. + t.is(statusEvents[statusEvents.length - 1].status, "serve-settling", + "state is settling while the restart is deferred"); // Well within the settle window: still no restart. await clock.tickAsync(ABORTED_BUILD_RESTART_SETTLE_MS - 50); @@ -357,12 +364,85 @@ test.serial( // Past the reset window: exactly one restart for the whole burst. await clock.tickAsync(50); t.is(projectBuilder.build.callCount, 2, "burst collapsed to a single restarted build"); + t.is(statusEvents[statusEvents.length - 1].status, "serve-building", + "SETTLING → BUILDING once the window closes and the restart fires"); // Settle the restarted build so no promise is left dangling. resolvers[1]?.(); await clock.tickAsync(0); }); +test.serial( + "serve-status: transient failure during a change burst reports settling, not error", async (t) => { + const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; + const {ABORTED_BUILD_RESTART_SETTLE_MS, BUILD_REQUEST_DEBOUNCE_MS} = + BuildServer.__internals__; + const statusEvents = makeStatusRecorder(t); + + // The first build throws a plain error *while a source change is still queued* — the + // "half-written tree during git checkout" shape. The retry (second invocation) succeeds. + const resolvers = []; + projectBuilder.build = sinon.stub().callsFake((_opts, cb) => new Promise((resolve, reject) => { + resolvers.push({resolve, reject, cb}); + })); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.context.buildServer = buildServer; + await clock.tickAsync(BUILD_REQUEST_DEBOUNCE_MS); + t.is(projectBuilder.build.callCount, 1, "initial build started"); + + // A source change lands (queued, not yet flushed), then the build throws. Because + // #resourceChangeQueue is non-empty, the failure is treated as transient: re-queue and + // defer the retry, reporting SETTLING rather than ERROR. + buildServer._projectResourceChanged(rootProject, "/a.js", false); + resolvers[0].reject(new Error("ENOENT: half-written tree")); + await clock.tickAsync(0); + + const midSeq = statusEvents.map((e) => e.status); + t.false(midSeq.includes("serve-error"), + `Transient failure must not surface serve-error; got: ${midSeq.join(", ")}`); + t.is(statusEvents[statusEvents.length - 1].status, "serve-settling", + `Transient failure reports settling; got: ${midSeq.join(", ")}`); + t.is(projectBuilder.build.callCount, 1, "retry held until the settle window elapses"); + + // Once quiet, a single rebuild fires and succeeds. + await clock.tickAsync(ABORTED_BUILD_RESTART_SETTLE_MS); + t.is(projectBuilder.build.callCount, 2, "single deferred rebuild after the tree settled"); + resolvers[1].cb("root.project", {getReader: () => ({fakeReader: true})}); + resolvers[1].resolve(["root.project"]); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-ready"); + + const seq = statusEvents.map((e) => e.status); + t.false(seq.includes("serve-error"), + `No serve-error anywhere in the transient-failure cycle; got: ${seq.join(", ")}`); + }); + +test.serial( + "serve-status: genuine build failure (no pending changes) still errors", async (t) => { + const {BuildServer, graph, projectBuilder, sinon, clock} = t.context; + const {BUILD_REQUEST_DEBOUNCE_MS} = BuildServer.__internals__; + const statusEvents = makeStatusRecorder(t); + + // Build fails with no queued source change and no aborted signal — the genuine-error + // branch. Regression guard for the Step 4 `signal.aborted || #resourceChangeQueue.size > 0` + // predicate: this must land on ERROR, not SETTLING. + const buildError = new Error("Build blew up"); + projectBuilder.build = sinon.stub().rejects(buildError); + + const buildServer = await BuildServer.create(graph, projectBuilder, true, [], []); + t.context.buildServer = buildServer; + buildServer.on("error", () => {}); + + await clock.tickAsync(BUILD_REQUEST_DEBOUNCE_MS); + await drainUntil(clock, statusEvents, (e) => e.status === "serve-error"); + + const seq = statusEvents.map((e) => e.status); + t.true(seq.includes("serve-error"), `Genuine failure surfaces serve-error; got: ${seq.join(", ")}`); + t.false(seq.includes("serve-settling"), + `Genuine failure must not report settling; got: ${seq.join(", ")}`); + t.is(seq[seq.length - 1], "serve-error", "serve-error is the terminal state of a genuine failure"); + }); + test.serial("serve-status: build failure emits serveError and no orphan building", async (t) => { const {BuildServer, graph, projectBuilder, sinon, clock} = t.context; const statusEvents = makeStatusRecorder(t); From a5225c806ae55f2a0d1b3fe687b446a2ffbd40c2 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:09 +0200 Subject: [PATCH 09/16] refactor(project): Add first-build settle window reporting SETTLING The first build after a quiet period fired on BUILD_REQUEST_DEBOUNCE_MS (10 ms). On a multi-file operation the tree is half-written when the first watcher event lands, so the build fires into an incomplete tree and fails transiently. Give the first speculative build a 100 ms settle window (FIRST_BUILD_SETTLE_MS) instead. This absorbs an editor's multi-file save fan-out; 100 ms sits well below @parcel/watcher's 500 ms cap and near its 50 ms floor, so single-edit latency is barely affected. The window reports SETTLING. Applies only when a build is already pending (a reader request queued but not started); laziness is preserved, since with nothing queued a source change still waits for a reader request. An explicit reader request supersedes the window by re-arming the queue at the snappy debounce, so serving a request is never delayed. Kept separate from the SETTLING reporting change so a regression in single-edit latency can be bisected cleanly. --- packages/project/lib/build/BuildServer.js | 51 ++++-- .../project/test/lib/build/BuildServer.js | 156 ++++++++++++++++++ 2 files changed, 193 insertions(+), 14 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 9f08af398c0..a364a74f37d 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -24,10 +24,23 @@ const log = getLogger("build:BuildServer"); const SOURCES_CHANGED_SETTLE_MS = 550; // Debounce for the request queue. A reader request enqueues a build and triggers the queue after -// this short delay so a batch of near-simultaneous requests builds together. Kept small so the -// first build after a quiet period starts promptly. +// this short delay so a batch of near-simultaneous requests builds together. Serving an explicit +// request must not wait, so this is kept small — it is not used for the speculative first build +// driven by a source change (see FIRST_BUILD_SETTLE_MS). const BUILD_REQUEST_DEBOUNCE_MS = 10; +// Settle window for the first speculative build driven by a source change from a quiet state. +// A multi-file operation — an editor's save-all, a `git checkout` — delivers its first watcher +// event while the tree is still half-written; building immediately on the snappy +// BUILD_REQUEST_DEBOUNCE_MS would fire into that half-written tree and fail on a transient error. +// Holding the first build for this short window absorbs an editor's own save fan-out with +// negligible single-edit cost: 100 ms sits far below @parcel/watcher's 500 ms coalescing cap and +// roughly at its 50 ms floor, so a lone edit still reaches the build promptly. This does not cover +// a multi-second `git checkout` on its own — full coverage of that case comes from routing the +// failure-with-pending-changes path through the deferred restart (the transient branch in +// #processBuildRequests). Reader-request-driven builds keep the snappy BUILD_REQUEST_DEBOUNCE_MS. +const FIRST_BUILD_SETTLE_MS = 100; + // Settle window for restarting a build that a source change aborted. When a change lands mid-build // the running build is aborted immediately, but the restart is held until changes have been quiet // for this long (each further change resets it). During a burst of changes — a `git checkout`, a @@ -571,6 +584,15 @@ class BuildServer extends EventEmitter { // armed timer and re-arms it at the settle delay. if (this.#pendingAbortRestart) { this.#triggerRequestQueue(ABORTED_BUILD_RESTART_SETTLE_MS); + } else if (!this.#activeBuild && this.#pendingBuildRequest.size > 0) { + // A reader-request-driven build is queued but has not started, and a source change just + // landed. Re-time it to the first-build settle window and report SETTLING: an editor's + // multi-file save fan-out then collapses into one build against the settled tree instead + // of firing into a half-written one. Laziness is preserved — with nothing queued, a + // change still waits for a reader request rather than provoking a speculative build. A + // subsequent reader request supersedes the settle by re-arming at the snappy debounce. + this.#setState(SERVER_STATES.SETTLING, {pendingProjects: this.#getStaleProjectNames()}); + this.#triggerRequestQueue(FIRST_BUILD_SETTLE_MS); } // Leading-edge emit with a trailing settle window. The first change of a quiet period @@ -606,24 +628,24 @@ class BuildServer extends EventEmitter { } /** - * Enqueues a project for building and returns a promise that resolves with its reader + * Enqueues a project for building and triggers the request queue at the short debounce. * - * If the project is already queued, returns the existing promise. Otherwise, creates - * a new promise, adds the project to the pending build queue, and triggers queue processing. + * Serving a request must not wait, so this always (re-)arms the queue at + * BUILD_REQUEST_DEBOUNCE_MS — even when the project is already queued. A reader + * request thereby supersedes a deferred settle window (the post-abort/transient restart at + * ABORTED_BUILD_RESTART_SETTLE_MS, or the first-build window at + * FIRST_BUILD_SETTLE_MS): the parked build is pulled forward to the short debounce + * rather than left waiting out the longer window. * * @param {string} projectName Name of the project to enqueue */ #enqueueBuild(projectName) { - if (this.#pendingBuildRequest.has(projectName)) { - // Already queued - return; + if (!this.#pendingBuildRequest.has(projectName)) { + log.verbose(`Enqueuing project '${projectName}' for build`); + this.#pendingBuildRequest.add(projectName); } - - log.verbose(`Enqueuing project '${projectName}' for build`); - - // Add to pending build requests - this.#pendingBuildRequest.add(projectName); - + // Always re-arm at the default debounce: an explicit reader request supersedes any longer + // settle window an earlier source change may have armed. this.#triggerRequestQueue(); } @@ -1376,6 +1398,7 @@ if (process.env.NODE_ENV === "test") { BuildServer.__internals__ = { SOURCES_CHANGED_SETTLE_MS, BUILD_REQUEST_DEBOUNCE_MS, + FIRST_BUILD_SETTLE_MS, ABORTED_BUILD_RESTART_SETTLE_MS }; } diff --git a/packages/project/test/lib/build/BuildServer.js b/packages/project/test/lib/build/BuildServer.js index 0f2e5c47b61..c84b0a653a7 100644 --- a/packages/project/test/lib/build/BuildServer.js +++ b/packages/project/test/lib/build/BuildServer.js @@ -443,6 +443,162 @@ test.serial( t.is(seq[seq.length - 1], "serve-error", "serve-error is the terminal state of a genuine failure"); }); +test.serial( + "serve-status: a pending build re-times to the first-build window on a source change", async (t) => { + const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; + const {FIRST_BUILD_SETTLE_MS, BUILD_REQUEST_DEBOUNCE_MS} = BuildServer.__internals__; + const statusEvents = makeStatusRecorder(t); + + const resolvers = []; + projectBuilder.build = sinon.stub().callsFake((_opts, cb) => new Promise((resolve) => { + resolvers.push({resolve, cb}); + })); + + // No initial build: the server starts IDLE. Capture the interface so we can enqueue a + // reader-request-driven build (which is what makes a build "pending" without an active one). + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + class FakeWatchHandler { + constructor() { + this.destroy = sinon.stub().resolves(); + this.on = sinon.stub(); + this.watch = sinon.stub().resolves(); + } + } + const CapturingBuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + const buildServer = await CapturingBuildServer.create(graph, projectBuilder, false, [], []); + t.teardown(() => buildServer.destroy()); + + // A reader request enqueues a build on the snappy debounce (10 ms) but doesn't fire yet. + const readerPromise = capturedInterface.getReaderForProject("root.project"); + readerPromise.catch(() => {}); + + // Before the debounce elapses, a source change lands. The pending (not-yet-started) build + // is re-timed to the 100 ms first-build window and the banner reports SETTLING. + buildServer._projectResourceChanged(rootProject, "/a.js", false); + t.is(statusEvents[statusEvents.length - 1].status, "serve-settling", + "source change on a pending build reports settling"); + + // The old 10 ms debounce must NOT fire the build — the window was pushed out to 100 ms. + await clock.tickAsync(BUILD_REQUEST_DEBOUNCE_MS); + t.is(projectBuilder.build.callCount, 0, "build held past the snappy debounce by the 100 ms window"); + + await clock.tickAsync(FIRST_BUILD_SETTLE_MS - BUILD_REQUEST_DEBOUNCE_MS); + t.is(projectBuilder.build.callCount, 1, "build fires once the 100 ms first-build window elapses"); + + resolvers[0].cb("root.project", {getReader: () => ({fakeReader: true})}); + resolvers[0].resolve(["root.project"]); + await readerPromise; + }); + +test.serial( + "serve-status: a reader request supersedes the first-build settle window", async (t) => { + const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; + const {FIRST_BUILD_SETTLE_MS, BUILD_REQUEST_DEBOUNCE_MS} = BuildServer.__internals__; + makeStatusRecorder(t); + + const resolvers = []; + projectBuilder.build = sinon.stub().callsFake((_opts, cb) => new Promise((resolve) => { + resolvers.push({resolve, cb}); + })); + + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + class FakeWatchHandler { + constructor() { + this.destroy = sinon.stub().resolves(); + this.on = sinon.stub(); + this.watch = sinon.stub().resolves(); + } + } + const CapturingBuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + const buildServer = await CapturingBuildServer.create(graph, projectBuilder, false, [], []); + t.teardown(() => buildServer.destroy()); + + // Enqueue a build, then re-time it to the 100 ms window via a source change. + const firstReq = capturedInterface.getReaderForProject("root.project"); + firstReq.catch(() => {}); + buildServer._projectResourceChanged(rootProject, "/a.js", false); + + // A fresh reader request must pull the build forward to the snappy debounce, superseding + // the 100 ms window. + const secondReq = capturedInterface.getReaderForProject("root.project"); + secondReq.catch(() => {}); + await clock.tickAsync(BUILD_REQUEST_DEBOUNCE_MS); + t.is(projectBuilder.build.callCount, 1, + "reader request superseded the 100 ms window; build ran at the snappy debounce"); + + // (Sanity) the 100 ms window would not have fired the build this early on its own. + t.true(BUILD_REQUEST_DEBOUNCE_MS < FIRST_BUILD_SETTLE_MS); + + resolvers[0].cb("root.project", {getReader: () => ({fakeReader: true})}); + resolvers[0].resolve(["root.project"]); + await Promise.all([firstReq, secondReq]); + }); + +test.serial( + "serve-status: the first-build window resets on each further source change", async (t) => { + const {BuildServer, graph, projectBuilder, rootProject, sinon, clock} = t.context; + const {FIRST_BUILD_SETTLE_MS} = BuildServer.__internals__; + makeStatusRecorder(t); + + const resolvers = []; + projectBuilder.build = sinon.stub().callsFake((_opts, cb) => new Promise((resolve) => { + resolvers.push({resolve, cb}); + })); + + let capturedInterface; + class CapturingBuildReader { + constructor(_name, _projects, buildServerInterface) { + capturedInterface = buildServerInterface; + } + } + class FakeWatchHandler { + constructor() { + this.destroy = sinon.stub().resolves(); + this.on = sinon.stub(); + this.watch = sinon.stub().resolves(); + } + } + const CapturingBuildServer = (await esmock("../../../lib/build/BuildServer.js", { + "../../../lib/build/BuildReader.js": CapturingBuildReader, + "../../../lib/build/helpers/WatchHandler.js": FakeWatchHandler, + })).default; + const buildServer = await CapturingBuildServer.create(graph, projectBuilder, false, [], []); + t.teardown(() => buildServer.destroy()); + + const readerPromise = capturedInterface.getReaderForProject("root.project"); + readerPromise.catch(() => {}); + buildServer._projectResourceChanged(rootProject, "/a.js", false); + + // Just short of the window, a further change resets it. + await clock.tickAsync(FIRST_BUILD_SETTLE_MS - 10); + buildServer._projectResourceChanged(rootProject, "/b.js", false); + await clock.tickAsync(FIRST_BUILD_SETTLE_MS - 10); + t.is(projectBuilder.build.callCount, 0, "second change reset the window; still no build"); + + await clock.tickAsync(10); + t.is(projectBuilder.build.callCount, 1, "build fires once after the final quiet period"); + + resolvers[0].cb("root.project", {getReader: () => ({fakeReader: true})}); + resolvers[0].resolve(["root.project"]); + await readerPromise; + }); + test.serial("serve-status: build failure emits serveError and no orphan building", async (t) => { const {BuildServer, graph, projectBuilder, sinon, clock} = t.context; const statusEvents = makeStatusRecorder(t); From 04ffbbf4e3358af04214df58374220c12606824c Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:09 +0200 Subject: [PATCH 10/16] refactor(logger): Drop redundant spinFrame reset on serve-validating transitionTo() in state/build.js resets spinFrame to 0 on every transition, and no other serve-* branch sets it at the call site. Drop the redundant assignment and rely on the helper. --- packages/logger/lib/writers/InteractiveConsole.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/logger/lib/writers/InteractiveConsole.js b/packages/logger/lib/writers/InteractiveConsole.js index 3fa34001ab4..f9b7f60a672 100644 --- a/packages/logger/lib/writers/InteractiveConsole.js +++ b/packages/logger/lib/writers/InteractiveConsole.js @@ -364,7 +364,6 @@ class InteractiveConsole { break; case "serve-validating": this.#buildState.validatingProjects = evt.validatingProjects || []; - this.#buildState.spinFrame = 0; this.#transitionTo(STATES.VALIDATING); break; case "serve-build-done": From 7b2380c655e9b536cd7d64ceddc701d95e9cd9a5 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:10 +0200 Subject: [PATCH 11/16] docs(project): Sync incremental-build skill with BuildServer state machines architecture.md had fallen behind BuildServer.js. Rewrite the affected sections to match the implementation: - Replace the 3-state per-project sketch (INITIAL / INVALIDATED / FRESH) with the actual 6-state ProjectBuildStatus diagram, adding VALIDATING, BUILDING, and ERRORED, and note which transitions each state accepts. - Add a Server lifecycle section for SERVER_STATES (IDLE / STALE / BUILDING / VALIDATING / ERROR) driven by #setState and #reconcileServerState, including SETTLING between STALE and BUILDING and the BUILDING -> SETTLING edge. - Document background cache validation (#scheduleBackgroundValidation / #runBackgroundValidation) promoting INITIAL projects to FRESH, and the reader-request wait-on-validation path in #getReaderForProject. - Add a Startup section for BuildServer.create() awaiting WatchHandler readiness (the Windows ReadDirectoryChangesW race). - Expand Build Request Flow with the ERRORED short-circuit, the VALIDATING wait, #stopActiveValidation, markBuilding, the transient-vs-non-transient failure split, and FIRST_BUILD_SETTLE_MS. - Expand File Watch and Abort with fileAddedOrRemoved reader eviction, leading-edge sourcesChanged (SOURCES_CHANGED_SETTLE_MS), the deferred post-abort restart (ABORTED_BUILD_RESTART_SETTLE_MS), the first-build 100 ms window, and error clearing on invalidate. - Correct the WatchHandler entry from chokidar to @parcel/watcher and record its 50 ms / 500 ms coalescing wait behind the debounce values. - Add an Error gating section for ERRORED, #lastError, and the transient-error carve-out. - Point the BuildServer row in the component table at ProjectBuildStatus and SERVER_STATES. --- .../skills/incremental-build/architecture.md | 132 +++++++++++++++--- 1 file changed, 111 insertions(+), 21 deletions(-) diff --git a/.claude/skills/incremental-build/architecture.md b/.claude/skills/incremental-build/architecture.md index 54531e5776d..fc9d686b027 100644 --- a/.claude/skills/incremental-build/architecture.md +++ b/.claude/skills/incremental-build/architecture.md @@ -32,13 +32,13 @@ Use this table to locate source files. ALWAYS read the relevant source file befo | Component | Location | Role | |-----------|----------|------| -| `BuildServer` | `lib/build/BuildServer.js` | Development server, file watching, build orchestration | +| `BuildServer` | `lib/build/BuildServer.js` | Development server, file watching, build orchestration. Also defines `ProjectBuildStatus` (per-project state machine, reader-request queue, error latching) and the outer `SERVER_STATES` reconciler | | `BuildReader` | `lib/build/BuildReader.js` | Reader exposed by BuildServer; routes resource requests to per-project readers via namespace map | | `ProjectBuilder` | `lib/build/ProjectBuilder.js` | Builds projects in dependency order | | `BuildContext` | `lib/build/helpers/BuildContext.js` | Global build config, project context cache | | `getBuildSignature` | `lib/build/helpers/getBuildSignature.js` | Build signature computation: `BUILD_SIG_VERSION` + build config + project config | | `ProjectBuildContext` | `lib/build/helpers/ProjectBuildContext.js` | Per-project bridge between builder, tasks, and cache | -| `WatchHandler` | `lib/build/helpers/WatchHandler.js` | chokidar-based source path watcher; emits change events to BuildServer | +| `WatchHandler` | `lib/build/helpers/WatchHandler.js` | `@parcel/watcher`-based source path watcher; emits change events to BuildServer. The watcher coalesces its own events with a 50 ms min / 500 ms max wait, so a continuous operation is delivered as batches up to 500 ms apart | | `TaskRunner` | `lib/build/TaskRunner.js` | Task composition, execution loop, abort handling | | `Cache` enum | `lib/build/cache/Cache.js` | Cache mode constants: `Default`, `Force`, `ReadOnly`, `Off` (CLI `--cache` option) | | `ProjectBuildCache` | `lib/build/cache/ProjectBuildCache.js` | Cache orchestration per project: index management, stage lookup, result recording | @@ -61,40 +61,130 @@ Use this table to locate source files. ALWAYS read the relevant source file befo ## Key Flows +### Startup + +`BuildServer.create()` awaits `WatchHandler` readiness before enqueueing initial builds. This closes a race — most visible on Windows' `ReadDirectoryChangesW` backend — where source changes made immediately after `graph.serve()` resolves would otherwise be missed. + ### Build Request Flow ``` reader.byPath("/test.js") - -> BuildServer checks ProjectBuildStatus - -> If not fresh: #enqueueBuild(projectName) - -> Debounced (10ms): #processBuildRequests() - -> Batch all pending projects + -> BuildServer #getReaderForProject(projectName) + -> If ProjectBuildStatus.isFresh(): return cached reader + -> If getError() returns a captured error: throw it (ERRORED gate, + see "Error gating" below) + -> Queue {resolve, reject} on the status via addReaderRequest() + -> If isValidating(): wait on the running validation pass + -> Otherwise #enqueueBuild(projectName) + -> Debounced (`BUILD_REQUEST_DEBOUNCE_MS` = 10ms): #processBuildRequests() + -> Any in-flight background validation is aborted first + (#stopActiveValidation) + -> Batch all pending projects; markBuilding() on each -> projectBuilder.build({projects, signal}) - -> On success: setReader(project.getReader({style: "runtime"})) - -> Resolve queued reader promises + -> On success: setReader(project.getReader({style: "runtime"})) — this + also drains the queued reader requests + -> On non-abort failure: rejectReaderRequests(err) latches ERRORED + -> On abort or concurrent source change: re-queue affected projects, + leave reader queue intact so they resolve on the retry ``` ### File Watch and Abort When a source file changes: -1. `WatchHandler` emits change event with project name and resource path -2. `_projectResourceChanged()` queues the change and calls `ProjectBuildStatus.invalidate()` on the affected project and all its dependents -3. `invalidate()` triggers the project's `AbortController`, which aborts the running build via `AbortSignal` -4. The build loop catches `AbortBuildError` and re-enqueues projects that aren't fresh -5. Queued resource changes are flushed via `#flushResourceChanges()` before the next build starts +1. `WatchHandler` emits change event with project name, resource path, and event type +2. `_projectResourceChanged()` walks `traverseDependents()` and calls `ProjectBuildStatus.invalidate({reason, fileAddedOrRemoved})` on the affected project and every dependent. Change is queued in `#resourceChangeQueue` +3. `invalidate()` clears any latched error (lifting the ERRORED gate), aborts the running build via `AbortSignal`, and rotates the `AbortController` +4. `fileAddedOrRemoved=true` (create/delete events) additionally evicts the cached reader on the status. Pure modifies keep the reader so callers already holding its promise still resolve +5. The build loop catches `AbortBuildError`, distinguishes abort from concurrent-change failure, and re-enqueues projects that aren't fresh. Both the source-change-aborted build and a build that *failed* while sources were still changing (the transient branch, `signal.aborted || #resourceChangeQueue.size > 0`) defer their restart until changes settle (`ABORTED_BUILD_RESTART_SETTLE_MS` = 550 ms, reset by each further change) rather than firing on the snappy request debounce — a burst delivered as multiple watcher batches then collapses into one rebuild against the settled tree instead of a build-abort cycle per batch. Both report `SETTLING` for the window's duration: the doomed build no longer parks the banner on `building` (abort) or flips it to `error` (transient failure); it reports "waiting for changes to settle" and retries once the tree is quiet. A reader request supersedes the deferred restart by enqueueing on the normal `BUILD_REQUEST_DEBOUNCE_MS` (10 ms), so serving a request is not delayed. A genuine, non-transient failure still latches ERRORED. +6. The first speculative build after a source change from a quiet state is held for a short first-build window (`FIRST_BUILD_SETTLE_MS` = 100 ms, also reported as `SETTLING`) rather than the snappy debounce — this absorbs an editor's own multi-file save fan-out (100 ms sits far below the watcher's 500 ms coalescing cap, roughly at its 50 ms floor) so a save-all doesn't fire a build into a half-written tree. It applies only to a build that is already pending (a reader request queued but not yet started); laziness is preserved — with nothing queued, a change still waits for a reader request. On its own it does not cover a multi-second `git checkout`; full coverage of that comes from the transient-failure deferral above. +7. Queued resource changes are flushed via `#flushResourceChanges()` before the next build starts (must happen before `projectBuilder.build`) + +The server also emits a `sourcesChanged` event to drive live-reload notifications. Emission is **leading-edge**: the first change of a quiet period notifies immediately (a lone edit reaches clients at the watcher's own ~50 ms latency floor with no debounce added), and a trailing settle window (`SOURCES_CHANGED_SETTLE_MS` = 550 ms, above the watcher's 500 ms cap) coalesces the remainder of a burst into one further emit. Because emission is leading-edge, the window size does not affect single-edit latency — it only controls burst coalescing. ### State Machine (per project) +`ProjectBuildStatus` (defined at the bottom of `BuildServer.js`) has six states: + ``` -INITIAL -> (first build requested, invalidate()) -> INVALIDATED -> (build completes, setReader()) -> FRESH - | - (file change detected) - v - INVALIDATED - (abort + re-enqueue) + +------------------------------------------+ + | | + v | + INITIAL --(reader request)--> INVALIDATED --(markBuilding)--> BUILDING + | ^ | + | | | setReader() + | (background validation) | v + | markValidating() | FRESH + v | | + VALIDATING ---(cache stale, | | + | releaseValidating) | (file change + | + | -----> INITIAL | invalidate()) | + | +------------------------------+ + | (cache fresh, setReader) | + +--> FRESH | + | + (build fails, non-transient) + v + ERRORED + | + (any invalidation + clears #lastError) + v + INVALIDATED ``` -Note: There is no separate `BUILDING` state; `INVALIDATED` covers both "needs build" and "building in progress". The abort controller on `ProjectBuildStatus` cancels the running build on re-invalidation. +- **INITIAL** — never built; eligible for background cache validation. +- **INVALIDATED** — needs a build. Set by `invalidate()` (source change, dependency change) and by the first reader request from INITIAL. +- **VALIDATING** — a background pass is checking cache validity for this project. Reader requests skip `#enqueueBuild` and wait on the pass. Only reachable from INITIAL via `markValidating()`. +- **BUILDING** — a real build cycle owns the project. Reached unconditionally from any prior state via `markBuilding()` (the caller has already claimed it from `#pendingBuildRequest`). +- **FRESH** — reader available. Set by `setReader()`, which only accepts BUILDING or VALIDATING as prior states — a late-arriving reader for a project re-invalidated mid-build is dropped. +- **ERRORED** — last build failed with a non-transient error. Held until an invalidation lifts the gate. See below. + +### Error gating + +Deterministic builds don't recover without an input change, so `rejectReaderRequests(err)` latches the project into ERRORED and captures the error on `#lastError`. Subsequent reader requests short-circuit via `getError()` and throw the captured error immediately — no rebuild loop against a broken tree. Any `invalidate()` (direct source change or a change in a transitive dependency) clears `#lastError` before flipping the state, so the next request enqueues a fresh build. + +Abort errors and errors during concurrent source changes are treated as transient: the reader queue is left intact and the affected projects are re-queued. The user sees a warn-level log, not a rejection. + +### Server lifecycle state + +BuildServer also maintains an outer state machine over all projects, mutated exclusively through `#setState` and emitted to the `ServeLogger`: + +``` +IDLE --(source change / reader request)--> STALE --(#triggerRequestQueue)--> BUILDING + ^ ^ | + | | v + | SETTLING <----------+------(abort / transient | + | | (rebuild deferred failure mid-cycle)------+ + | | until quiet) | + | +--(settle window elapses)--------------------> BUILDING + | | + +---------- VALIDATING <--(post-build, INITIAL projects remain)-------------+ + | | | + | +--(cache hit for all)-----------------------------------> IDLE + | | + | +--(cache miss, still non-FRESH) -----------------------> STALE + | + any --(unrecoverable failure)--> ERROR --(any invalidation)--> STALE +``` + +- `#reconcileServerState({mayValidate})` is the single point of truth for terminal transitions at the end of a build cycle or validation pass. It picks IDLE / STALE / VALIDATING based on `#getStaleProjectNames()`, `#activeBuild`, `#pendingBuildRequest`, and the `mayValidate` flag. It bails on `SETTLING` (as it does on `ERROR`) so the deferred-restart timer owns the SETTLING → BUILDING transition. +- **SETTLING** means "changes seen, a rebuild is pending, holding until changes go quiet." It sits between STALE and BUILDING and is entered from three deferral sites, all reporting `serve-settling`: the post-abort restart, the failure-with-pending-changes (transient) path, and a source change re-timing an already-pending build to the first-build window. No build is active while in SETTLING (`#activeBuild === null`); the armed timer moves it to BUILDING when the settle window elapses. BUILDING → SETTLING skips the `buildDone` emission — no successful cycle closed (mirrors BUILDING → ERROR). +- A genuine, non-transient build failure (no pending changes, signal not aborted) still goes to ERROR — the distinction is the `signal.aborted || #resourceChangeQueue.size > 0` predicate. +- Errored projects are NOT counted as "stale" — their rebuild is gated on input change, so surfacing them under STALE would understate the situation. +- BUILDING → ERROR skips the `buildDone` emission so consumers don't see a successful cycle close before the error. + +### Background cache validation + +After a build cycle ends with some projects still in INITIAL (e.g. dependencies never requested yet), `#scheduleBackgroundValidation` picks them up and calls `projectBuilder.validateCaches()` in a fire-and-forget pass: + +1. `willValidate(projectName)` claims the project via `markValidating()` — a no-op if the state moved on. +2. Per project, `validateCaches` invokes the callback with `usesCache`: + - `usesCache=true` → `setReader()` promotes the project to FRESH without executing any tasks. + - `usesCache=false` → `releaseValidating()` reverts VALIDATING → INITIAL. If reader requests are queued, `onBuildRequired` fires `#enqueueBuild` so the waiting callers eventually resolve. +3. A source change during the pass aborts the composite signal (validation abort + per-project abort) and cancels validation for the affected projects. +4. The `finally` clause guarantees no project is left stuck in VALIDATING regardless of how the pass ended. + +A build request preempts an in-flight pass: `#triggerRequestQueue` awaits `#stopActiveValidation` before claiming the builder's `buildIsRunning` lock. The pass's `finally` re-invokes `#reconcileServerState({mayValidate: false})` — `mayValidate=false` prevents stack recursion into another validation pass over the projects the previous one just released. ## Caching Architecture @@ -362,7 +452,7 @@ Stage metadata stored on disk includes: ## Key Architectural Patterns 1. **Lazy building**: Projects built on-demand when readers are requested -2. **Request batching**: Multiple pending build requests processed in single batch (10ms debounce) +2. **Request batching**: Multiple pending build requests processed in single batch (`BUILD_REQUEST_DEBOUNCE_MS` = 10ms debounce). A source-change-driven first build is held on a short settle window (`FIRST_BUILD_SETTLE_MS` = 100ms) to absorb editor save fan-out, and a source-change-aborted or transiently-failed build restarts on a longer window (`ABORTED_BUILD_RESTART_SETTLE_MS` = 550ms) so a burst collapses into one rebuild. Both windows report the `SETTLING` state; a reader request supersedes them at the snappy 10ms debounce. 3. **Abort/retry**: File changes abort running builds; projects re-queued automatically 4. **Structural sharing**: Derived hash trees share unchanged subtrees, reducing memory 5. **Content-addressed storage**: Resources deduplicated via integrity hashes in custom CAS (synchronous path resolution, gzip-compressed) From 6c1fd3799957cc743b279a8975edb6a7986e7c50 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:11 +0200 Subject: [PATCH 12/16] refactor(server): Serve build-error page with live-reload on HTML navigation While the build server is errored, a top-level HTML navigation now renders an error page with the live-reload client attached instead of a plain-text stack trace, so the tab reloads itself once the source is fixed. The terminal error handler previously answered every failure with plain text, losing the live-reload script serveResources.js injects, so a fixed source emitted sourcesChanged but the tab had no client listening. Content-negotiate in createErrorHandler: for a top-level HTML navigation, render errorPage.html with the error message and stack HTML-escaped, and embed INJECT_SCRIPT_TAG when liveReload.active. Non-HTML requests keep the text/plain stack response. The script tag reuses the constant serveResources.js injects, so there is one source of truth. On the next sourcesChanged the client calls location.reload(), landing on the recovered build or a fresh error page. Negotiate on Sec-Fetch-Dest and an explicit text/html Accept rather than req.accepts("html"): a wildcard Accept returns "html", which would serve the HTML page to every failing subresource load and leave a "); + const res = mockRes(); + + handler(err, mockReq({"sec-fetch-dest": "document"}), res, () => t.fail("next")); + + t.false(res.state.body.includes("