From d33abaca8234591b60b3560537fac0ae32a1379d Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:00 +0200 Subject: [PATCH 01/18] refactor(project): Handle recoverable build errors without crash Classify caught task errors into three branches: abort, transient (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 are reported 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 df564220c09d89ed3ee6170ffb37cda26cf1ca7d Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:02 +0200 Subject: [PATCH 02/18] 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 reproduce the same failure, so hold the gate until real signal arrives. Three signals lift the gate: - 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 captured error forever. Correct the banner for the error path: _projectResourceChanged now also 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 | 135 ++++++- .../test/lib/build/BuildServer.integration.js | 34 ++ .../project/test/lib/build/BuildServer.js | 340 ++++++++++++++++++ 3 files changed, 501 insertions(+), 8 deletions(-) diff --git a/packages/project/lib/build/BuildServer.js b/packages/project/lib/build/BuildServer.js index 745a36d5d6a..d5116f30866 100644 --- a/packages/project/lib/build/BuildServer.js +++ b/packages/project/lib/build/BuildServer.js @@ -263,6 +263,17 @@ class BuildServer extends EventEmitter { if (projectBuildStatus.isFresh()) { return projectBuildStatus.getReader(); } + + // Last build failed and nothing has changed since. Rebuilding would just + // re-produce the same error (builds are deterministic), so short-circuit + // with the captured error. The gate lifts as soon as any 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 +354,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 +380,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 +493,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 +603,44 @@ class BuildServer extends EventEmitter { this.#triggerRequestQueue(); return; } + // A successful build cycle proves the environment can build. Lift ERRORED + // gates on any *other* project (Fix 2 in _projectResourceChanged handles + // the graph-local case; this broadens it to unrelated projects that a + // dependent-change never reaches). The next reader request for such a + // project will re-enqueue 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 +900,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 +915,7 @@ class ProjectBuildStatus { #reader; #abortController = new AbortController(); #onBuildRequired; + #lastError = null; /** * @param {Function} [onBuildRequired] Invoked when the status leaves the VALIDATING @@ -869,6 +947,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 +1041,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 +1098,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 7133d514a0f070be69a2baecfd0daedcf98d358b Mon Sep 17 00:00:00 2001 From: Matthias Osswald Date: Wed, 1 Jul 2026 15:12:25 +0200 Subject: [PATCH 03/18] refactor(server): Add terminal Express error handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every next(err) in the middleware chain now flows through a custom 4-argument handler that responds with HTTP 500, text/plain content type, and the error stack (falling back to the message or String(err)) as the body. Registered last so it catches errors from every earlier middleware, including third-party custom middleware where we cannot wrap a try/catch. If the response has already started, delegate to Express's default handler which closes the connection. Console-log routing (differentiating gated build errors from unexpected ones) stays out of this handler for now — that 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 cee67f08326..e4e197d3041 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"; @@ -238,6 +239,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 c1e35ca5e32c2dac78400eda230a1232199d9209 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:03 +0200 Subject: [PATCH 04/18] refactor(project): Drop resurrected resources from delta merge On a delta signature transition whose changedProjectResourcePaths lists a path the delta task did not re-emit, the merge loop in recordTaskResult replayed the previous stage's copy of that path into the new stage writer. When a source file is deleted between builds, the delta task emits nothing for it and the merge resurrects the pre-deletion output, so downstream readers keep serving the file after its source is gone. Affects every differential task (replaceCopyright, replaceVersion, replaceBuildtime, minify). Filter the merge by cacheInfo.changedProjectResourcePaths: if a path was flagged changed and the task did not write it, drop the stale copy instead of replaying it. Paths outside the delta merge as before. Track the dropped count alongside merged count in the perf log. Add four fail-then-succeed test buckets covering invariants across a failed-attempt / retry boundary on one ProjectBuildCache instance: 1. #writtenResultResourcePaths accumulates across failed attempts, because allTasksCompleted (which clears it) never runs on a thrown build. The worst outcome is redundant I/O on the retry, not wrong output: the retry re-hashes each leaked path against its fresh reader, so unchanged content yields the same signature and changed content invalidates correctly. The test pins this behavior. Clearing the set cleanly needs a separate fail/abort hook, since #initSourceIndex legitimately seeds the array with delta paths on the first build after loading from persistent cache. 2. #currentStageSignatures reflect the retry's stages after a full retry. 3. The delta merge does not resurrect a deleted-source resource across the boundary. 4. #frozenSourceReader is nulled by the retry's initStages call before further task inputs are computed. --- .../lib/build/cache/ProjectBuildCache.js | 22 +- .../test/lib/build/cache/ProjectBuildCache.js | 337 ++++++++++++++++++ 2 files changed, 355 insertions(+), 4 deletions(-) diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index 30dd53d3268..85ef299378f 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -896,21 +896,35 @@ export default class ProjectBuildCache { reader = cacheInfo.previousStageCache.stage.getWriter() ?? cacheInfo.previousStageCache.stage.getCachedWriter(); } + // Paths the delta task was told changed but did not re-emit. Their source + // is gone (or intentionally excluded); replaying the previous stage's + // copy would resurrect content that no longer belongs in the output. + const changedProjectResourcePaths = new Set(cacheInfo.changedProjectResourcePaths ?? []); const mergeStart = performance.now(); const previousWrittenResources = await reader.byGlob("/**/*"); let mergedCount = 0; + let droppedCount = 0; for (const res of previousWrittenResources) { - if (!writtenResourcePaths.includes(res.getOriginalPath())) { - await stageWriter.write(res); - mergedCount++; + const path = res.getOriginalPath(); + if (writtenResourcePaths.includes(path)) { + continue; // Delta re-emitted this path; skip } + if (changedProjectResourcePaths.has(path)) { + // Delta was told this path changed but did not write it back. + // Drop the stale copy from the merge. + droppedCount++; + continue; + } + await stageWriter.write(res); + mergedCount++; } if (log.isLevelEnabled("perf")) { log.perf( `recordTaskResult delta merge for task ${taskName} ` + `in project ${this.#project.getName()} completed in ` + `${(performance.now() - mergeStart).toFixed(2)} ms ` + - `(${previousWrittenResources.length} previous, ${mergedCount} merged)`); + `(${previousWrittenResources.length} previous, ${mergedCount} merged, ` + + `${droppedCount} dropped)`); } } else { // Calculate signature for executed task diff --git a/packages/project/test/lib/build/cache/ProjectBuildCache.js b/packages/project/test/lib/build/cache/ProjectBuildCache.js index 82929a02ab7..e41cac036f8 100644 --- a/packages/project/test/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/test/lib/build/cache/ProjectBuildCache.js @@ -1873,3 +1873,340 @@ test("validateCache: Cache.Force throws when source changes are detected", async ); t.regex(err.message, /Force.*mode.*stale/i, "Error message mentions Force mode and stale cache"); }); + +// ===== FAIL-THEN-SUCCEED (BuildServer error-recovery) REPRO TESTS ===== +// +// These tests exercise the state that persists across build attempts on the same +// ProjectBuildCache instance — the shape a long-running `ui5 serve` walks when a +// task throws mid-build (e.g. LESS syntax error) and a subsequent source edit +// heals the failure. The recorded symptom is "server serves corrupted resources +// from the failed build after fix". `StageCache#discardPending` (called from +// `validateCache({prepareForBuild:true})`) already scrubs pending stage cache +// entries the failed attempt added; the remaining risk is state that outlives +// that scrub — `#writtenResultResourcePaths`, `#currentStageSignatures`, the +// delta-merge input, and the frozen source reader on ProjectResources. +// +// Each test drives an initial successful build, a second attempt where task B +// "throws" (modeled by omitting its `recordTaskResult` call), and a retry. +// Assertions inspect the observable arguments passed to the taskCache / +// projectResources mocks on the retry. + +// Helper: after task A has recorded, model task B throwing by leaving the stage +// in place without calling recordTaskResult. Then simulate the retry's fresh +// validateCache({prepareForBuild:true}) call. +async function driveFailedBuildThenRetry(cache, mockDependencyReader) { + // Retry: fresh validateCache({prepareForBuild:true}) — this is the entry point + // BuildServer takes for a rebuilt project. discardPending fires here. + return cache.validateCache(mockDependencyReader, {prepareForBuild: true}); +} + +test("Fail-then-succeed: #writtenResultResourcePaths accumulates across failed attempts (documented behavior)", + async (t) => { + // After taskA records in a failed build and taskB throws, the failed attempt + // leaves /a.js in #writtenResultResourcePaths. On retry, + // prepareTaskExecutionAndValidateCache calls + // taskCache.updateProjectIndices(reader, #writtenResultResourcePaths). + // + // This is a benign leak in practice: updateProjectIndices re-fetches each + // path through the retry's fresh reader and re-hashes; unchanged content + // produces the same signature and no invalidation, changed content + // invalidates correctly. The observable impact is redundant I/O on the + // retry, not incorrect output. This test pins the current behavior so + // a future refactor that reduces or eliminates the leak can update it. + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + + const initialA = createMockResource("/a.js", "hash-a", 1000, 100, 1); + const initialB = createMockResource("/b.js", "hash-b", 1000, 100, 2); + project.getSourceReader.callsFake(() => ({ + byGlob: sinon.stub().resolves([initialA, initialB]), + byPath: sinon.stub().callsFake((p) => { + return Promise.resolve(p === "/a.js" ? initialA : p === "/b.js" ? initialB : null); + }) + })); + + const cache = await ProjectBuildCache.create(project, "sig", cacheManager); + await cache.initSourceIndex(); + + const mockDependencyReader = { + byGlob: sinon.stub().resolves([]), + byPath: sinon.stub().resolves(null), + }; + + // Failed build attempt: taskA runs successfully and writes /a.js. + cache.setTasks(["taskA", "taskB"]); + await cache.prepareTaskExecutionAndValidateCache("taskA"); + + const writtenA = createMockResource("/a.js", "hash-a-built", 2000, 200, 1); + project.getProjectResources().getStage.returns({ + getId: () => "task/taskA", + getWriter: sinon.stub().returns({ + byGlob: sinon.stub().resolves([writtenA]) + }) + }); + await cache.recordTaskResult( + "taskA", {paths: new Set(), patterns: new Set()}, + {paths: new Set(), patterns: new Set()}, null, false); + + // taskB "throws" — no recordTaskResult call. The failed build leaves + // #writtenResultResourcePaths containing ["/a.js"] since allTasksCompleted + // (which would clear it) never runs. + + // Retry: BuildServer calls validateCache({prepareForBuild:true}) again. + await driveFailedBuildThenRetry(cache, mockDependencyReader); + + // The retry claims taskA again. Currently /a.js is passed as a "changed" + // path to updateProjectIndices even though it did not change on disk. + cache.setTasks(["taskA", "taskB"]); + const updateProjectIndicesStub = sinon.stub( + cache.getTaskCache("taskA"), "updateProjectIndices").resolves(); + + project.getProjectResources().getStage.returns({ + getId: () => "task/taskA", + getWriter: sinon.stub().returns({ + byGlob: sinon.stub().resolves([]) + }) + }); + await cache.prepareTaskExecutionAndValidateCache("taskA"); + + t.true(updateProjectIndicesStub.called, + "updateProjectIndices is called on retry with the leaked paths"); + const changedPaths = updateProjectIndicesStub.firstCall.args[1]; + t.true(changedPaths.includes("/a.js"), + `Documented behavior: retry passes /a.js to updateProjectIndices even ` + + `though it did not change between attempts (changed paths: ${JSON.stringify(changedPaths)}). ` + + `Re-hashing through the retry's fresh reader means this does not produce ` + + `incorrect output, only redundant work.`); + }); + +test("Fail-then-succeed: #currentStageSignatures from failed attempt does not linger", + async (t) => { + // After a failed build, #currentStageSignatures contains the entry the + // completed task wrote. On retry, if the source signature happens to match + // what the failed build indexed at that stage, #findResultCache / + // #getResultStageSignature could combine the stale entry with the fresh one + // and either short-circuit unnecessarily or produce a spurious cache hit. + // Assert #getResultStageSignature (via allTasksCompleted → cache write) + // reflects only what the *successful* retry recorded. + 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(); + + const mockDependencyReader = { + byGlob: sinon.stub().resolves([]), + byPath: sinon.stub().resolves(null), + }; + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); + + // Failed attempt: taskA records with a distinctive signature. + cache.setTasks(["taskA", "taskB"]); + await cache.prepareTaskExecutionAndValidateCache("taskA"); + project.getProjectResources().getStage.returns({ + getId: () => "task/taskA", + getWriter: sinon.stub().returns({ + byGlob: sinon.stub().resolves([]) + }) + }); + await cache.recordTaskResult( + "taskA", {paths: new Set(), patterns: new Set()}, + {paths: new Set(), patterns: new Set()}, null, false); + + // taskB "throws" — build fails. + + // Retry. + await driveFailedBuildThenRetry(cache, mockDependencyReader); + cache.setTasks(["taskA", "taskB"]); + + await cache.prepareTaskExecutionAndValidateCache("taskA"); + project.getProjectResources().getStage.returns({ + getId: () => "task/taskA", + getWriter: sinon.stub().returns({ + byGlob: sinon.stub().resolves([]) + }) + }); + await cache.recordTaskResult( + "taskA", {paths: new Set(), patterns: new Set()}, + {paths: new Set(), patterns: new Set()}, null, false); + + await cache.prepareTaskExecutionAndValidateCache("taskB"); + project.getProjectResources().getStage.returns({ + getId: () => "task/taskB", + getWriter: sinon.stub().returns({ + byGlob: sinon.stub().resolves([]) + }) + }); + await cache.recordTaskResult( + "taskB", {paths: new Set(), patterns: new Set()}, + {paths: new Set(), patterns: new Set()}, null, false); + + await cache.allTasksCompleted(); + await cache.writeCache(); + + // Inspect the result metadata that was written. It must reference exactly + // the two stages the successful retry recorded — not more, not less. + const resultMetadataCalls = cacheManager.writeResultMetadata.getCalls(); + t.is(resultMetadataCalls.length, 1, "One result metadata write"); + const {stageSignatures} = resultMetadataCalls[0].args[3]; + const stageNames = Object.keys(stageSignatures); + t.deepEqual(stageNames.sort(), ["task/taskA", "task/taskB"], + "Result metadata references exactly the two retry stages, no lingering entries"); + }); + +test("Fail-then-succeed: delta merge does not resurrect resources from a stage a failed attempt wrote", + async (t) => { + // In build 1, taskA records a stage that contains /a.js and /b.js. + // Build 1 completes; result cache is persisted. + // Build 2 (failed): source /b.js is deleted; taskA runs, taskB throws. + // Build 3 (retry): source /b.js is still gone. taskA runs in delta mode + // with cacheInfo.previousStageCache pointing at build 1's stage entry. + // The delta merge at recordTaskResult (line 900-907) reads previousStageCache + // and writes every resource NOT overlaid by the current delta. If it merges + // the stale /b.js, /b.js becomes visible in the retry's output even though + // the source file no longer exists. + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + const cache = await ProjectBuildCache.create(project, "sig", cacheManager); + await cache.initSourceIndex(); + + cache.setTasks(["deltaTask"]); + await cache.prepareTaskExecutionAndValidateCache("deltaTask"); + + // The retry's delta task writes only the changed /a.js. + const retryA = createMockResource("/a.js", "hash-a-new", 3000, 300, 1); + const writeStub = sinon.stub().resolves(); + project.getProjectResources().getStage.returns({ + getId: () => "task/deltaTask", + getWriter: sinon.stub().returns({ + byGlob: sinon.stub().resolves([retryA]), + write: writeStub, + }) + }); + + // previousStageCache reflects the state before the failed build: /a.js AND /b.js. + // The retry drops /b.js (source deleted). If the delta merge blindly + // replays every previous-stage resource, /b.js survives in the retry output. + const prevA = createMockResource("/a.js", "hash-a-old", 1000, 100, 1); + const prevB = createMockResource("/b.js", "hash-b-old", 1000, 100, 2); + const cacheInfo = { + previousStageCache: { + signature: "prev-proj-prev-dep", + stage: { + byGlob: sinon.stub().resolves([prevA, prevB]), + }, + writtenResourcePaths: ["/a.js", "/b.js"], + projectTagOperations: undefined, + buildTagOperations: undefined, + }, + newSignature: "new-proj-new-dep", + // changedProjectResourcePaths tells the delta task WHICH paths changed. + // /b.js was deleted, so it changed — but the delta task didn't write + // its replacement (it can't; the file is gone). + changedProjectResourcePaths: ["/a.js", "/b.js"], + changedDependencyResourcePaths: [], + }; + + await cache.recordTaskResult( + "deltaTask", {paths: new Set(), patterns: new Set()}, + {paths: new Set(), patterns: new Set()}, cacheInfo, true); + + // The merge currently writes every previous resource whose path is not in + // the delta's writtenResourcePaths. /b.js is not overlaid → gets written. + // This test documents the observed behavior: the deleted file is + // resurrected. This IS the bug shape reported by the user. + const writtenPaths = writeStub.getCalls().map((c) => c.args[0].getOriginalPath()); + t.false(writtenPaths.includes("/b.js"), + `LEAK: /b.js was merged into the retry stage from the previous stage cache ` + + `even though its source is gone. Written paths: ${JSON.stringify(writtenPaths)}`); + }); + +test("Fail-then-succeed: #frozenSourceReader from a prior build does not shadow the retry's fresh source reads", + async (t) => { + // After build N succeeds, ProjectResources.#frozenSourceReader points at a + // CAS-backed snapshot of the untransformed sources. That reader has HIGHER + // priority than the on-disk source reader (see ProjectResources#getReaders). + // On build N+1 the reader is nulled by initStages → #initStageMetadata, + // but only when initStages is actually invoked. The BuildServer path + // calls validateCache({prepareForBuild:true}) → this.#project.getReader() + // BEFORE setTasks/initStages. If any code path reads through the reader + // returned by that call, it sees the stale frozen snapshot. + // + // Model this by driving a full retry and asserting that setFrozenSourceReader + // is called with null (or that initStages is called before any downstream + // task consults getReader on ProjectResources). The current mock's + // initStages does nothing — we upgrade it here to model the real reset, + // then verify the invariant that the retry does not observe the previous + // build's frozen reader. + const project = createMockProject(); + const cacheManager = createMockCacheManager(); + + // Track invocations to sequence them. + const events = []; + const projectResources = project.getProjectResources(); + let frozenReader = null; + projectResources.setFrozenSourceReader.callsFake((r) => { + events.push({op: "setFrozenSourceReader", value: r ? "reader" : null}); + frozenReader = r; + }); + // Upgrade the initStages stub to model the real ProjectResources behavior: + // #initStageMetadata resets #frozenSourceReader = null. + projectResources.initStages.callsFake(() => { + events.push({op: "initStages", frozenBefore: frozenReader ? "reader" : null}); + frozenReader = null; + }); + + 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(); + + const mockDependencyReader = { + byGlob: sinon.stub().resolves([]), + byPath: sinon.stub().resolves(null), + }; + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); + + // Successful build 1: run taskA and allTasksCompleted (which invokes freeze). + cache.setTasks(["taskA"]); + await cache.prepareTaskExecutionAndValidateCache("taskA"); + project.getProjectResources().getStage.returns({ + getId: () => "task/taskA", + getWriter: sinon.stub().returns({ + byGlob: sinon.stub().resolves([]) + }) + }); + await cache.recordTaskResult( + "taskA", {paths: new Set(), patterns: new Set()}, + {paths: new Set(), patterns: new Set()}, null, false); + await cache.allTasksCompleted(); // sets a frozen reader + + // The frozen reader must be set by this point. + t.true(events.some((e) => e.op === "setFrozenSourceReader" && e.value === "reader"), + "Build 1 set a frozen source reader"); + + // Now simulate BuildServer retry after a failed build attempt: the fresh + // validateCache({prepareForBuild:true}) captures #currentProjectReader via + // project.getReader(), THEN setTasks runs and calls initStages. + // Assert initStages fires before any subsequent recordTaskResult so the + // stale frozen reader cannot leak through to the retry's task inputs. + await cache.validateCache(mockDependencyReader, {prepareForBuild: true}); + cache.setTasks(["taskA"]); + + const initStagesIdx = events.map((e) => e.op).lastIndexOf("initStages"); + t.true(initStagesIdx >= 0, "initStages fires on retry"); + // After initStages, frozenReader is null — future getReader() calls no + // longer include the stale frozen snapshot. + t.is(frozenReader, null, "Frozen source reader is dropped by initStages on retry"); + }); + From c6c7e1ad730bbd837cb3ce72f7abec8a30129c47 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:03 +0200 Subject: [PATCH 05/18] refactor(project): Handle unresolved reads in ResourceRequestManager MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit MonitoredReader records every byPath/byGlob call regardless of result, so a task probing an optional file or using a glob that matches nothing still contributes those requests. On #addRequestSet, when findExactMatch misses and findBestParent picks a proper-subset parent, the delta can contain only requests that resolve to zero resources. That threw "Unexpected empty added resources for request set ID ...", reproduced by branch-switching in OpenUI5. These reads are not benign: whether a probed file exists can change task output, so collapsing an empty-delta node onto its parent's signature would serve stale results after the file's existence changes. Replace the throw with a marker scheme: - On an empty-resolving delta, derive an empty tree from the parent (deriveTree([])) and record the added-request keys on metadata.unresolvedKeys. For an all-empty root recording, keep the empty ResourceIndex and populate unresolvedKeys with every request. Partially-resolving deltas record only the unresolved subset. - Route getIndexSignatures() and #addRequestSet's returned signature through a new #computeNodeSignature: the pure tree hash when unresolvedKeys is empty, otherwise sha256(treeHash \0 sorted keys). - In updateIndices, drop keys from unresolvedKeys as their request resolves to a real resource. Once drained, the signature collapses to the pure tree hash — matching a same-shape recording with the resource present. - Persist unresolvedKeys on root and delta entries through toCacheObject/fromCache. SharedHashTree is untouched: deriveTree([]) already produces a distinct ResourceIndex whose tree hashes to the parent's, so the tree-identity map keys stay distinct across nodes. Rewrite the three repro tests to assert no-throw plus signature distinctness; add convergence, cache round-trip, empty-root discrimination, and BuildTaskCache-shape coverage. --- .../lib/build/cache/ResourceRequestManager.js | 188 +++++++++++++++-- .../test/lib/build/cache/BuildTaskCache.js | 32 +++ .../lib/build/cache/ResourceRequestManager.js | 197 ++++++++++++++++++ 3 files changed, 395 insertions(+), 22 deletions(-) diff --git a/packages/project/lib/build/cache/ResourceRequestManager.js b/packages/project/lib/build/cache/ResourceRequestManager.js index f7a211f1e4b..ac7ae76a571 100644 --- a/packages/project/lib/build/cache/ResourceRequestManager.js +++ b/packages/project/lib/build/cache/ResourceRequestManager.js @@ -1,3 +1,4 @@ +import crypto from "node:crypto"; import micromatch from "micromatch"; import ResourceRequestGraph, {Request} from "./ResourceRequestGraph.js"; import ResourceIndex from "./index/ResourceIndex.js"; @@ -73,15 +74,18 @@ class ResourceRequestManager { projectName, taskName, useDifferentialUpdate, requestGraph, unusedAtLeastOnce); const registries = new Map(); // Restore root resource indices - for (const {nodeId, resourceIndex: serializedIndex} of rootIndices) { + for (const {nodeId, resourceIndex: serializedIndex, unresolvedKeys} of rootIndices) { const metadata = requestGraph.getMetadata(nodeId); const registry = resourceRequestManager.#newTreeRegistry(); registries.set(nodeId, registry); metadata.resourceIndex = ResourceIndex.fromCacheShared(serializedIndex, registry); + if (unresolvedKeys && unresolvedKeys.length) { + metadata.unresolvedKeys = new Set(unresolvedKeys); + } } // Restore delta resource indices if (deltaIndices) { - for (const {nodeId, addedResourceIndex} of deltaIndices) { + for (const {nodeId, addedResourceIndex, unresolvedKeys} of deltaIndices) { const node = requestGraph.getNode(nodeId); const {resourceIndex: parentResourceIndex} = requestGraph.getMetadata(node.getParentId()); const registry = registries.get(node.getParentId()); @@ -91,9 +95,11 @@ class ResourceRequestManager { } const resourceIndex = parentResourceIndex.deriveTreeWithIndex(addedResourceIndex); - requestGraph.setMetadata(nodeId, { - resourceIndex, - }); + const metadata = {resourceIndex}; + if (unresolvedKeys && unresolvedKeys.length) { + metadata.unresolvedKeys = new Set(unresolvedKeys); + } + requestGraph.setMetadata(nodeId, metadata); } } return resourceRequestManager; @@ -113,11 +119,11 @@ class ResourceRequestManager { getIndexSignatures() { const requestSetIds = this.#requestGraph.getAllNodeIds(); const signatures = requestSetIds.map((requestSetId) => { - const {resourceIndex} = this.#requestGraph.getMetadata(requestSetId); + const {resourceIndex, unresolvedKeys} = this.#requestGraph.getMetadata(requestSetId); if (!resourceIndex) { throw new Error(`Resource index missing for request set ID ${requestSetId}`); } - return resourceIndex.getSignature(); + return this.#computeNodeSignature(resourceIndex, unresolvedKeys); }); if (this.#unusedAtLeastOnce) { signatures.push("X"); // Signature for when no requests were made @@ -262,7 +268,8 @@ class ResourceRequestManager { // Phase 3: Process each request set from cache for (const requestSetId of matchingRequestSetIds) { - const {resourceIndex} = this.#requestGraph.getMetadata(requestSetId); + const metadata = this.#requestGraph.getMetadata(requestSetId); + const {resourceIndex} = metadata; if (!resourceIndex) { throw new Error(`Missing resource index for request set ID ${requestSetId}`); } @@ -284,6 +291,16 @@ class ResourceRequestManager { if (resourcesToUpdate.length) { await resourceIndex.upsertResources(resourcesToUpdate); } + // Drain unresolved markers for this node: any recorded added-request that now + // resolves to at least one resource is no longer unresolved. Only inspect + // added-requests (the ones this node contributes) — inherited requests are the + // parent's responsibility. + if (metadata.unresolvedKeys && metadata.unresolvedKeys.size && + resourcesToUpdate.length) { + this.#drainUnresolvedKeys( + metadata, this.#requestGraph.getNode(requestSetId).getAddedRequests(), + resourcesToUpdate.map((res) => res.getOriginalPath())); + } } let hasChanges; if (this.#useDifferentialUpdate) { @@ -555,10 +572,13 @@ class ResourceRequestManager { // Try to find an existing request set that we can reuse let setId = this.#requestGraph.findExactMatch(requests); let resourceIndex; + let unresolvedKeys; if (setId) { // Reuse existing resource index. // Note: This index has already been updated before the task executed, so no update is necessary here - resourceIndex = this.#requestGraph.getMetadata(setId).resourceIndex; + const existingMetadata = this.#requestGraph.getMetadata(setId); + resourceIndex = existingMetadata.resourceIndex; + unresolvedKeys = existingMetadata.unresolvedKeys; } else { // New request set, check whether we can create a delta const metadata = {}; // Will populate with resourceIndex below @@ -572,27 +592,143 @@ class ResourceRequestManager { const addedRequests = requestSet.getAddedRequests(); const resourcesToAdd = await this.#getResourcesForRequests(addedRequests, reader); - if (!resourcesToAdd.length) { - throw new Error(`Unexpected empty added resources for request set ID ${setId} ` + - `of task '${this.#taskName}' of project '${this.#projectName}'`); + if (resourcesToAdd.length) { + log.verbose(`Task '${this.#taskName}' of project '${this.#projectName}' ` + + `created derived resource index for request set ID ${setId} ` + + `based on parent ID ${parentId} with ${resourcesToAdd.length} additional resources`); + resourceIndex = await parentResourceIndex.deriveTree(resourcesToAdd); + // Some added requests may have resolved to no resource yet (e.g. a byPath + // probe for an optional file). Record the unresolved keys so the exposed + // signature stays distinct from the parent's until they resolve. + unresolvedKeys = this.#collectUnresolvedKeys(addedRequests, resourcesToAdd); + } else { + // Every added request resolved to nothing. This is legitimate: a task can + // probe files (byPath returning null, byGlob returning []) whose absence + // still influences task output. Derive an empty tree so the graph shape + // tracks the recording 1:1, and record every added request as unresolved + // so the exposed signature stays distinct from the parent's. + log.verbose(`Task '${this.#taskName}' of project '${this.#projectName}' ` + + `created derived resource index for request set ID ${setId} ` + + `based on parent ID ${parentId} with all added requests unresolved`); + resourceIndex = await parentResourceIndex.deriveTree([]); + unresolvedKeys = new Set(addedRequests.map((r) => r.toKey())); } - log.verbose(`Task '${this.#taskName}' of project '${this.#projectName}' ` + - `created derived resource index for request set ID ${setId} ` + - `based on parent ID ${parentId} with ${resourcesToAdd.length} additional resources`); - resourceIndex = await parentResourceIndex.deriveTree(resourcesToAdd); } else { const resourcesRead = await this.#getResourcesForRequests(requests, reader); resourceIndex = await ResourceIndex.createShared(resourcesRead, Date.now(), this.#newTreeRegistry()); + // Same handling for the root case: distinguish independent recordings that + // happen to resolve to zero resources today. + unresolvedKeys = this.#collectUnresolvedKeys(requests, resourcesRead); } metadata.resourceIndex = resourceIndex; + if (unresolvedKeys && unresolvedKeys.size) { + metadata.unresolvedKeys = unresolvedKeys; + } } return { setId, - signature: resourceIndex.getSignature(), + signature: this.#computeNodeSignature(resourceIndex, unresolvedKeys), }; } + /** + * Computes the exposed signature for a request-set node + * + * When the node has no unresolved requests, returns the underlying tree signature. + * When some recorded requests resolved to no resources (e.g. byPath probes for + * files that do not currently exist, or byGlob patterns matching nothing), those + * recorded reads still influence task output, so the exposed signature must stay + * distinct from the tree hash of an otherwise-identical index. The composite + * signature is a SHA-256 of the tree signature and the sorted unresolved keys. + * + * Once all unresolved keys have been drained (a later updateIndices upserts each + * probed resource as it appears), the composite falls back to the tree signature + * — matching what a fresh recording with the resource present would produce. + * + * @param {ResourceIndex} resourceIndex Resource index of the node + * @param {Set|undefined} unresolvedKeys Set of unresolved request keys + * @returns {string} Exposed signature + */ + #computeNodeSignature(resourceIndex, unresolvedKeys) { + const treeSignature = resourceIndex.getSignature(); + if (!unresolvedKeys || unresolvedKeys.size === 0) { + return treeSignature; + } + const sortedKeys = Array.from(unresolvedKeys).sort(); + return crypto.createHash("sha256") + .update(treeSignature) + .update("\0") + .update(sortedKeys.join("\0")) + .digest("hex"); + } + + /** + * Determines which recorded requests did not resolve to any resource + * + * A 'path' request is unresolved if no fetched resource has that exact path. + * A 'patterns' request is unresolved if no fetched resource path matches any + * pattern in the array. + * + * @param {Request[]} recordedRequests Requests as recorded by the MonitoredReader + * @param {Array} fetchedResources Resources actually returned + * @returns {Set} Set of unresolved request keys (Request.toKey() form) + */ + #collectUnresolvedKeys(recordedRequests, fetchedResources) { + if (!recordedRequests.length) { + return new Set(); + } + const fetchedPaths = fetchedResources.map((res) => res.getOriginalPath()); + const unresolved = new Set(); + for (const request of recordedRequests) { + if (request.type === "path") { + if (!fetchedPaths.includes(request.value)) { + unresolved.add(request.toKey()); + } + } else { + const matches = micromatch(fetchedPaths, request.value, {dot: true}); + if (!matches.length) { + unresolved.add(request.toKey()); + } + } + } + return unresolved; + } + + /** + * Drops keys from metadata.unresolvedKeys whose recorded request now resolves to + * at least one of the given paths. + * + * Called from updateIndices after upserting newly-appeared resources into a + * node's index. Once every key drains, the node's exposed signature collapses + * back to the pure tree hash, matching what a fresh recording with the + * resource present would have produced. + * + * @param {object} metadata Request-set node metadata (mutated in place) + * @param {Request[]} addedRequests Recorded added-requests for this node + * @param {string[]} resolvedPaths Paths that now resolve to a resource + */ + #drainUnresolvedKeys(metadata, addedRequests, resolvedPaths) { + for (const request of addedRequests) { + const key = request.toKey(); + if (!metadata.unresolvedKeys.has(key)) { + continue; + } + let resolvesNow; + if (request.type === "path") { + resolvesNow = resolvedPaths.includes(request.value); + } else { + resolvesNow = micromatch(resolvedPaths, request.value, {dot: true}).length > 0; + } + if (resolvesNow) { + metadata.unresolvedKeys.delete(key); + } + } + if (metadata.unresolvedKeys.size === 0) { + delete metadata.unresolvedKeys; + } + } + /** * Associates a request set from this manager with one from another manager * @@ -691,15 +827,19 @@ class ResourceRequestManager { const rootIndices = []; const deltaIndices = []; for (const {nodeId, parentId} of this.#requestGraph.traverseByDepth()) { - const {resourceIndex} = this.#requestGraph.getMetadata(nodeId); + const {resourceIndex, unresolvedKeys} = this.#requestGraph.getMetadata(nodeId); if (!resourceIndex) { throw new Error(`Missing resource index for node ID ${nodeId}`); } if (!parentId) { - rootIndices.push({ + const entry = { nodeId, resourceIndex: resourceIndex.toCacheObject(), - }); + }; + if (unresolvedKeys && unresolvedKeys.size) { + entry.unresolvedKeys = Array.from(unresolvedKeys); + } + rootIndices.push(entry); } else { const {resourceIndex: rootResourceIndex} = this.#requestGraph.getMetadata(parentId); if (!rootResourceIndex) { @@ -708,10 +848,14 @@ class ResourceRequestManager { // Store the metadata for all added resources. Note: Those resources might not be available // in the current tree. In that case we store an empty array. const addedResourceIndex = resourceIndex.getAddedResourceIndex(rootResourceIndex); - deltaIndices.push({ + const entry = { nodeId, addedResourceIndex, - }); + }; + if (unresolvedKeys && unresolvedKeys.size) { + entry.unresolvedKeys = Array.from(unresolvedKeys); + } + deltaIndices.push(entry); } } return { diff --git a/packages/project/test/lib/build/cache/BuildTaskCache.js b/packages/project/test/lib/build/cache/BuildTaskCache.js index 85f4052e3cf..2a73267c4c0 100644 --- a/packages/project/test/lib/build/cache/BuildTaskCache.js +++ b/packages/project/test/lib/build/cache/BuildTaskCache.js @@ -493,3 +493,35 @@ test("Handles non-existent resource paths", async (t) => { t.is(typeof projectSig, "string", "Still returns signature"); t.is(typeof depSig, "string", "Still returns dependency signature"); }); + +test("recordRequests with unresolved probe in delta position returns a distinct signature", async (t) => { + // Shape observed in OpenUI5 after a branch switch: the first recording anchors + // a resolvable parent request set, then a subsequent recording adds a byPath + // probe for a file that no longer exists. Used to throw + // "Unexpected empty added resources ...". + const cache = new BuildTaskCache("test.project", "testTask", false); + + const projectReader = createMockReader([ + createMockResource("/a.js"), + ]); + const dependencyReader = createMockReader([]); + + const firstRequests = { + paths: new Set(["/a.js"]), + patterns: new Set(), + }; + const [firstProjSig] = await cache.recordRequests( + firstRequests, undefined, projectReader, dependencyReader); + + const probingRequests = { + paths: new Set(["/a.js", "/optional.json"]), + patterns: new Set(), + }; + const [probingProjSig] = await cache.recordRequests( + probingRequests, undefined, projectReader, dependencyReader); + + t.is(typeof probingProjSig, "string", + "Probing recording completes without throwing"); + t.not(probingProjSig, firstProjSig, + "Probing recording gets a cache key distinct from the parent's — the probed absence matters for output"); +}); diff --git a/packages/project/test/lib/build/cache/ResourceRequestManager.js b/packages/project/test/lib/build/cache/ResourceRequestManager.js index 161b292ac2e..3be17bbd1b0 100644 --- a/packages/project/test/lib/build/cache/ResourceRequestManager.js +++ b/packages/project/test/lib/build/cache/ResourceRequestManager.js @@ -659,6 +659,203 @@ test("ResourceRequestManager: Serialization round-trip with multiple request set t.false(manager2.hasNewOrModifiedCacheEntries(), "Restored manager has no new entries"); }); +// ===== UNRESOLVED-READS TESTS ===== +// +// A task's MonitoredReader records every byPath/byGlob call, including calls that +// return null or []. When a later addRequests recording contains an added delta +// whose requests all resolve to zero resources against the current reader +// (typical after a branch switch that deletes probed files), the manager still +// needs to record the request set — its shape influences task output, so it +// cannot be silently collapsed onto the parent's cache key. +// +// The tests below assert: +// - #addRequestSet does not throw for empty-resolving deltas. +// - The exposed signature is distinct from the parent's and from any other +// empty-resolving delta. +// - Once a probed resource appears and updateIndices upserts it, the composite +// signature collapses to the pure tree hash, matching the signature a +// same-shape recording would have produced with the resource present. +// - The composite survives toCacheObject / fromCache round-trip. + +test("ResourceRequestManager: #addRequestSet with empty-resolving path delta produces distinct signature", + async (t) => { + const resourcesBefore = new Map([ + ["/a.js", createMockResource("/a.js", "hash-a")], + ]); + const reader = createMockReader(resourcesBefore); + + const manager = new ResourceRequestManager("test.project", "myTask", false); + // Parent recording: {/a.js} + const parent = await manager.addRequests({paths: ["/a.js"], patterns: []}, reader); + + // Probing recording: {/a.js, /c.js}. /c.js does not exist, but MonitoredReader + // still recorded the byPath call. + const probe = await manager.addRequests({ + paths: ["/a.js", "/c.js"], + patterns: [], + }, reader); + + t.truthy(probe.signature, "Empty-resolving delta yields a signature (no throw)"); + t.not(probe.signature, parent.signature, + "Delta signature is distinct from parent's — the probe influences task output"); + + const signatures = manager.getIndexSignatures(); + t.is(signatures.length, 2, "Both request sets are recorded"); + t.is(signatures[0], parent.signature); + t.is(signatures[1], probe.signature); + }); + +test("ResourceRequestManager: #addRequestSet with empty-resolving pattern delta produces distinct signature", + async (t) => { + const resourcesBefore = new Map([ + ["/src/a.js", createMockResource("/src/a.js", "hash-a")], + ]); + const reader = createMockReader(resourcesBefore); + + const manager = new ResourceRequestManager("test.project", "myTask", false); + const parent = await manager.addRequests({paths: ["/src/a.js"], patterns: []}, reader); + + // Recorded byGlob("/test/**/*.js") that matches nothing today. + const probe = await manager.addRequests({ + paths: ["/src/a.js"], + patterns: [["/test/**/*.js"]], + }, reader); + + t.truthy(probe.signature, "Empty-resolving pattern delta yields a signature"); + t.not(probe.signature, parent.signature, "Pattern probe changes cache identity"); + }); + +test("ResourceRequestManager: two independent empty-resolving deltas produce different signatures", + async (t) => { + const reader = createMockReader(new Map([ + ["/a.js", createMockResource("/a.js", "hash-a")], + ])); + + const manager = new ResourceRequestManager("test.project", "myTask", false); + await manager.addRequests({paths: ["/a.js"], patterns: []}, reader); + + const probeC = await manager.addRequests({ + paths: ["/a.js", "/c.js"], + patterns: [], + }, reader); + const probeD = await manager.addRequests({ + paths: ["/a.js", "/d.js"], + patterns: [], + }, reader); + + t.not(probeC.signature, probeD.signature, + "Different unresolved probes must map to different cache keys"); + }); + +test("ResourceRequestManager: signature collapses to tree hash once probed resource resolves", + async (t) => { + // A: full run against a state where /c.js exists — produces the natural + // derived-tree signature we want the recovering run to converge on. + const readerWithC = createMockReader(new Map([ + ["/a.js", createMockResource("/a.js", "hash-a")], + ["/c.js", createMockResource("/c.js", "hash-c")], + ])); + const managerA = new ResourceRequestManager("test.project", "myTask", false); + await managerA.addRequests({paths: ["/a.js"], patterns: []}, readerWithC); + const referenceRun = await managerA.addRequests({ + paths: ["/a.js", "/c.js"], + patterns: [], + }, readerWithC); + + // B: same recording against a reader where /c.js is missing, then + // updateIndices reintroduces it. + const readerWithoutC = createMockReader(new Map([ + ["/a.js", createMockResource("/a.js", "hash-a")], + ])); + const managerB = new ResourceRequestManager("test.project", "myTask", false); + await managerB.addRequests({paths: ["/a.js"], patterns: []}, readerWithoutC); + const probingRun = await managerB.addRequests({ + paths: ["/a.js", "/c.js"], + patterns: [], + }, readerWithoutC); + + t.not(probingRun.signature, referenceRun.signature, + "Probing recording differs while /c.js is missing"); + + // /c.js now appears. updateIndices upserts it into the probing node. + const readerNowWithC = createMockReader(new Map([ + ["/a.js", createMockResource("/a.js", "hash-a")], + ["/c.js", createMockResource("/c.js", "hash-c")], + ])); + await managerB.updateIndices(readerNowWithC, ["/c.js"]); + + const signaturesB = managerB.getIndexSignatures(); + t.is(signaturesB[1], referenceRun.signature, + "Once the probe resolves, the composite signature collapses to the natural tree hash"); + }); + +test("ResourceRequestManager: unresolved-keys marker survives toCacheObject / fromCache", + async (t) => { + const reader = createMockReader(new Map([ + ["/a.js", createMockResource("/a.js", "hash-a")], + ])); + + const manager1 = new ResourceRequestManager("test.project", "myTask", false); + await manager1.addRequests({paths: ["/a.js"], patterns: []}, reader); + const probe = await manager1.addRequests({ + paths: ["/a.js", "/c.js"], + patterns: [], + }, reader); + const signaturesBefore = manager1.getIndexSignatures(); + + const cacheObj = manager1.toCacheObject(); + const manager2 = ResourceRequestManager.fromCache("test.project", "myTask", false, cacheObj); + + const signaturesAfter = manager2.getIndexSignatures(); + t.deepEqual(signaturesAfter, signaturesBefore, + "Signatures survive the cache round-trip bit-for-bit"); + t.is(signaturesAfter[1], probe.signature, + "Probing signature is preserved"); + }); + +test("ResourceRequestManager: fully empty root recording gets a distinguished signature", + async (t) => { + // Two independent tasks that both recorded reads to files that don't exist + // must not collide on the same "dir::empty" tree constant. + const emptyReader = createMockReader(new Map()); + + const managerA = new ResourceRequestManager("test.project", "taskA", false); + const runA = await managerA.addRequests({paths: ["/only-in-A.js"], patterns: []}, emptyReader); + + const managerB = new ResourceRequestManager("test.project", "taskB", false); + const runB = await managerB.addRequests({paths: ["/only-in-B.js"], patterns: []}, emptyReader); + + t.not(runA.signature, runB.signature, + "Distinct root recordings with all-unresolved reads produce distinct signatures"); + }); + +test("ResourceRequestManager: BuildTaskCache-shape flow with unresolved probe (integration-ish)", + async (t) => { + // Mirrors what BuildTaskCache.recordRequests does with two consecutive + // addRequests recordings that share a parent but differ by one probed path. + const readerBefore = createMockReader(new Map([ + ["/a.js", createMockResource("/a.js", "hash-a")], + ["/b.js", createMockResource("/b.js", "hash-b")], + ])); + + const manager = new ResourceRequestManager("test.project", "myTask", true); + + // First recording produces a valid parent. + const first = await manager.addRequests({paths: ["/a.js", "/b.js"], patterns: []}, readerBefore); + + // Second recording adds an unresolvable byPath — the shape that used to throw. + const second = await manager.addRequests({ + paths: ["/a.js", "/b.js", "/optional.json"], + patterns: [], + }, readerBefore); + + t.truthy(second.signature, "Second recording completes without throwing"); + t.not(second.signature, first.signature, "Cache key reflects the extra probe"); + + // hasNewOrModifiedCacheEntries stays true for a fresh manager. + t.true(manager.hasNewOrModifiedCacheEntries()); + }); + test("ResourceRequestManager: Serialization round-trip with multiple request sets and following update", async (t) => { const resources = new Map([ ["/a.js", createMockResource("/a.js", "hash-a")], From a477c9094235eab9dbe4cd5cbcd2e4a1bdcb83f2 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:04 +0200 Subject: [PATCH 06/18] refactor(project): Extract request-vs-paths predicate in ResourceRequestManager #collectUnresolvedKeys and #drainUnresolvedKeys both branched on request.type === 'path' vs 'patterns', with the path branch scanning the path array via Array.includes. Extract the check into #requestResolvesAgainstPaths and back the path lookup with a Set; the patterns branch still gets the array form it needs for micromatch. --- .../lib/build/cache/ResourceRequestManager.js | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/packages/project/lib/build/cache/ResourceRequestManager.js b/packages/project/lib/build/cache/ResourceRequestManager.js index ac7ae76a571..d2e15044d60 100644 --- a/packages/project/lib/build/cache/ResourceRequestManager.js +++ b/packages/project/lib/build/cache/ResourceRequestManager.js @@ -679,17 +679,11 @@ class ResourceRequestManager { return new Set(); } const fetchedPaths = fetchedResources.map((res) => res.getOriginalPath()); + const fetchedPathSet = new Set(fetchedPaths); const unresolved = new Set(); for (const request of recordedRequests) { - if (request.type === "path") { - if (!fetchedPaths.includes(request.value)) { - unresolved.add(request.toKey()); - } - } else { - const matches = micromatch(fetchedPaths, request.value, {dot: true}); - if (!matches.length) { - unresolved.add(request.toKey()); - } + if (!this.#requestResolvesAgainstPaths(request, fetchedPathSet, fetchedPaths)) { + unresolved.add(request.toKey()); } } return unresolved; @@ -709,18 +703,13 @@ class ResourceRequestManager { * @param {string[]} resolvedPaths Paths that now resolve to a resource */ #drainUnresolvedKeys(metadata, addedRequests, resolvedPaths) { + const resolvedPathSet = new Set(resolvedPaths); for (const request of addedRequests) { const key = request.toKey(); if (!metadata.unresolvedKeys.has(key)) { continue; } - let resolvesNow; - if (request.type === "path") { - resolvesNow = resolvedPaths.includes(request.value); - } else { - resolvesNow = micromatch(resolvedPaths, request.value, {dot: true}).length > 0; - } - if (resolvesNow) { + if (this.#requestResolvesAgainstPaths(request, resolvedPathSet, resolvedPaths)) { metadata.unresolvedKeys.delete(key); } } @@ -729,6 +718,23 @@ class ResourceRequestManager { } } + /** + * Whether a recorded request would resolve against the given set of resource paths. + * 'path' requests hit the Set for O(1) lookup; 'patterns' requests fall back to + * micromatch, which needs the array form. + * + * @param {Request} request + * @param {Set} pathSet Set of resource paths + * @param {string[]} pathList Same paths in array form, for micromatch + * @returns {boolean} + */ + #requestResolvesAgainstPaths(request, pathSet, pathList) { + if (request.type === "path") { + return pathSet.has(request.value); + } + return micromatch(pathList, request.value, {dot: true}).length > 0; + } + /** * Associates a request set from this manager with one from another manager * From 29f8bef34a959df7d5907530395e875d427e39bb Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:05 +0200 Subject: [PATCH 07/18] refactor(project): Recover from dropped file watcher events MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @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.resetForFullRescan() 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 d5116f30866..91e3b4bd1f8 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` retains 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" is emitted with 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); @@ -440,7 +572,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 @@ -452,7 +587,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 @@ -743,7 +880,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..3c10a7de990 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.forceFullRescan(); + } + /** * 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 85ef299378f..bf9f79f0b88 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -1238,6 +1238,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) — is intentionally kept. + * + * No-op in {@link @ui5/project/build/cache/Cache}.Off mode, where no index or result + * cache exists to reset. + * + * @public + */ + resetForFullRescan() { + if (this.#cacheMode === Cache.Off) { + return; + } + this.#combinedIndexState = INDEX_STATES.RESTORING_PROJECT_INDICES; + this.#sourceIndex = null; + this.#taskCache.clear(); + // Result cache state must also be reset: a prior validateCache may have transitioned + // it to NO_CACHE or FRESH_AND_IN_USE. The next build'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.#changedProjectSourcePaths = []; + this.#changedDependencyResourcePaths = []; + } + /** * Signals that all tasks have completed and switches to the result stage * @@ -1275,14 +1310,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.resetForFullRescan(); 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..e9c7c7e8045 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. + */ + forceFullRescan() { + for (const ctx of this._projectBuildContexts.values()) { + ctx.resetForFullRescan(); + } + } + 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..9d03042931d 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#resetForFullRescan}. + */ + resetForFullRescan() { + return this._buildCache.resetForFullRescan(); + } + 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 e41cac036f8..01ae6993dbf 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("resetForFullRescan 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 resetForFullRescan 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.resetForFullRescan(); + 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, + "resetForFullRescan re-armed initSourceIndex to re-glob the source tree"); +}); + +test("resetForFullRescan 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.resetForFullRescan()); +}); 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 abbcddfbdee21a3f87799d87e5dbe2afa96d36b0 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:06 +0200 Subject: [PATCH 08/18] 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 resetForFullRescan() on it; abort and SourceChangedDuringBuildError are excluded via isAbortError. Harden resetForFullRescan() 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 | 14 ++++++ .../lib/build/cache/ProjectBuildCache.js | 24 +++++++++- .../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, 207 insertions(+), 2 deletions(-) diff --git a/packages/project/lib/build/ProjectBuilder.js b/packages/project/lib/build/ProjectBuilder.js index 3c10a7de990..b558ffb3357 100644 --- a/packages/project/lib/build/ProjectBuilder.js +++ b/packages/project/lib/build/ProjectBuilder.js @@ -434,9 +434,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 +480,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 +492,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 user fixes the source) + // 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?.resetForFullRescan(); } throw err; } diff --git a/packages/project/lib/build/cache/ProjectBuildCache.js b/packages/project/lib/build/cache/ProjectBuildCache.js index bf9f79f0b88..ade52e207e5 100644 --- a/packages/project/lib/build/cache/ProjectBuildCache.js +++ b/packages/project/lib/build/cache/ProjectBuildCache.js @@ -1244,10 +1244,19 @@ export default class ProjectBuildCache { * 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. + * changing during a build — as well as 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 — + * full re-scan. The derived per-build signatures (#currentResultSignature, + * #cachedResultSignature, #currentStageSignatures) and the + * written-path accumulator are cleared too, and the project's stage pipeline is reset + * 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} would match the retained + * #currentResultSignature and skip re-importing the (uncorrupted) cached + * stages, serving the failed build's partial output instead. + * + * 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) — is intentionally kept. @@ -1271,6 +1280,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 = []; + // Return the stage pipeline to its initial state 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..cb68d7a5470 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, + resetForFullRescan: 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.resetForFullRescan.callCount, 1, + "resetForFullRescan 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, + resetForFullRescan: 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.resetForFullRescan.callCount, 0, + "resetForFullRescan 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 01ae6993dbf..64f2e59f0c8 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("resetForFullRescan re-arms a full source re-scan", async (t) => { "resetForFullRescan re-armed initSourceIndex to re-glob the source tree"); }); +test("resetForFullRescan 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.resetForFullRescan(); + + t.true(resetStub.callCount > resetCallsBefore, + "resetForFullRescan 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("resetForFullRescan 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 8fd80b9f838b41426a64d4a18592909b200a610b Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:06 +0200 Subject: [PATCH 09/18] 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 91e3b4bd1f8..a0897cb1581 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. @@ -526,14 +543,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() { @@ -1284,6 +1310,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 534e0ee6432b0be1581f85bfb7c76d8bfb6cde89 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:07 +0200 Subject: [PATCH 10/18] 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 a0897cb1581..d3b5fd2c141 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 is waiting 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(); @@ -543,6 +563,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 @@ -597,7 +624,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. @@ -609,6 +636,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. @@ -641,7 +671,7 @@ class BuildServer extends EventEmitter { // ServeLogger; keep the server alive. this.#setState(SERVER_STATES.ERROR, {error: err}); }); - }, 10); + }, delay); } /** @@ -759,11 +789,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 @@ -1310,6 +1345,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 4f41bcdb1ab9e4fd3af46b8bc57cde9f683551a3 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:08 +0200 Subject: [PATCH 11/18] 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 | 72 +++++++++---- .../test/lib/build/BuildServer.integration.js | 40 +++++++ .../project/test/lib/build/BuildServer.js | 102 ++++++++++++++++-- 10 files changed, 262 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 d3b5fd2c141..68b02db75cc 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 will move 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. @@ -563,9 +566,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); } @@ -718,6 +722,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, @@ -740,13 +745,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()) { @@ -787,17 +795,23 @@ 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 + // changing. 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. + // + // 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; } @@ -878,6 +892,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 @@ -897,7 +916,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) { @@ -1047,9 +1067,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
    • * @@ -1060,18 +1084,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]); } @@ -1082,6 +1109,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 36951bc664cb77c147fe4e81d339f0bb5939b11a Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:09 +0200 Subject: [PATCH 12/18] 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 68b02db75cc..21084102596 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 @@ -572,6 +585,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 @@ -607,24 +629,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 snappy 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 snappy 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(); } @@ -1377,6 +1399,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 d02986d39bf470e2dba4bb9a8b3652d2e0eb9b11 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:09 +0200 Subject: [PATCH 13/18] 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 7d4b1b2eed7b79461a3b4bffe65731e1aca1f5d7 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:10 +0200 Subject: [PATCH 14/18] 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 3656dd5ef2c7757e45872de18a5dc0127c4515c9 Mon Sep 17 00:00:00 2001 From: Merlin Beutlberger Date: Mon, 13 Jul 2026 10:27:11 +0200 Subject: [PATCH 15/18] 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("