diff --git a/packages/project/lib/graph/ProjectDefinitionWatcher.js b/packages/project/lib/graph/ProjectDefinitionWatcher.js index d9a13549128..ad710ecda01 100644 --- a/packages/project/lib/graph/ProjectDefinitionWatcher.js +++ b/packages/project/lib/graph/ProjectDefinitionWatcher.js @@ -230,13 +230,13 @@ class ProjectDefinitionWatcher extends EventEmitter { } try { + this.#cancelSettleTimer(); + // Tear down the current subscriptions and re-subscribe the same watch set. The include // set (#watchedFiles / #watchDirs) is unchanged; only the OS-level handles are renewed. // Teardown failures are ignored here: the handles are discarded either way, and the // re-subscribe below is what decides whether recovery succeeded. - const subscriptions = this.#subscriptions; - this.#subscriptions = []; - await drainSubscriptions(subscriptions); + await this.#drainSubscriptions(); if (this.#destroyed) { return; } @@ -259,19 +259,29 @@ class ProjectDefinitionWatcher extends EventEmitter { */ async destroy() { this.#destroyed = true; + this.#cancelSettleTimer(); + const failures = await this.#drainSubscriptions(); + if (failures.length) { + const err = new AggregateError(failures, "Failed to unsubscribe one or more definition watchers"); + this.emit("error", err); + } + } + + // Cancels a pending settle timer, if any. Safe to call when no timer is armed. + #cancelSettleTimer() { if (this.#settleTimer) { clearTimeout(this.#settleTimer); this.#settleTimer = null; } - // Drain the subscriptions list first so a second destroy() is a no-op and a partial failure - // cannot leave stale handles to be unsubscribed twice. + } + + // Snapshots and clears the subscriptions list before draining it, so a second drain (a second + // destroy(), or a destroy() racing recovery) is a no-op and a partial failure cannot leave stale + // handles to be unsubscribed twice. Returns the unsubscribe failures for callers that report them. + async #drainSubscriptions() { const subscriptions = this.#subscriptions; this.#subscriptions = []; - const failures = await drainSubscriptions(subscriptions); - if (failures.length) { - const err = new AggregateError(failures, "Failed to unsubscribe one or more definition watchers"); - this.emit("error", err); - } + return drainSubscriptions(subscriptions); } } diff --git a/packages/project/test/lib/graph/ProjectDefinitionWatcher.js b/packages/project/test/lib/graph/ProjectDefinitionWatcher.js index ebf385a03df..15469eb36e8 100644 --- a/packages/project/test/lib/graph/ProjectDefinitionWatcher.js +++ b/packages/project/test/lib/graph/ProjectDefinitionWatcher.js @@ -454,6 +454,44 @@ test.serial("recovery: a watcher error tears down and re-subscribes", async (t) await watcher.destroy(); }); +test.serial("recovery: pending settle timer is cancelled before teardown", async (t) => { + const sub1 = createMockSubscription(); + const sub2 = createMockSubscription(); + let cb; + subscribeStub.onFirstCall().callsFake(async (_dir, callback) => { + cb = callback; + return sub1; + }); + subscribeStub.onSecondCall().resolves(sub2); + + const graph = createGraph({name: "root", rootPath: fixturePath("/app")}); + const watcher = await ProjectDefinitionWatcher.create({graph}); + const ui5YamlPath = fixtureFile("/app", "ui5.yaml"); + + const emitted = []; + watcher.on("definitionChanged", (e) => emitted.push(e)); + + const clock = sinon.useFakeTimers(); + // Open a burst so the settle timer is armed. + cb(null, [{type: "update", path: ui5YamlPath}]); + clock.tick(100); + + // Watcher error fires while the timer is still pending. + cb(new Error("Failed to read changes")); + + // Restore real timers before awaiting recovery so async callbacks can proceed. + clock.restore(); + await new Promise((resolve) => setImmediate(resolve)); + + // The old subscription is torn down and a new one created. + t.true(sub1.unsubscribe.calledOnce, "old subscription torn down"); + t.is(subscribeStub.callCount, 2, "re-subscribed after recovery"); + + t.is(emitted.length, 0, "cancelled settle timer does not fire into the closed watcher handle"); + + await watcher.destroy(); +}); + test.serial("recovery: loop protection escalates to error after the max attempts", async (t) => { const subs = []; subscribeStub.callsFake(async () => {