From 73f0ef711a2b9a178c49e5f9185364d3ddd7b694 Mon Sep 17 00:00:00 2001 From: Christian Pillsbury Date: Thu, 18 Jun 2026 14:16:17 -0700 Subject: [PATCH] refactor(spf): memoize Task.run via a named #execute, fixing orphan rejections MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Memoize the run-machinery promise itself (`#promise ??= this.#execute()`) rather than reassigning `#promise` to fresh `Promise.resolve(value)` / `Promise.reject(error)` on settle. The reassigned promises were never awaited, so an errored or aborted task that's not re-run (the norm — the RecurringRunner moves on to a clone) left an unhandled rejection. The memoized promise is the one callers await, so it's always handled. Also closes a sync-throw gap: a `#runFn` that throws synchronously is now captured as a rejected memoized promise instead of leaving `#promise` unset (which would re-execute on the next run()). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/spf/src/core/tasks/task.ts | 34 ++++++++++++++++------------- 1 file changed, 19 insertions(+), 15 deletions(-) diff --git a/packages/spf/src/core/tasks/task.ts b/packages/spf/src/core/tasks/task.ts index 6fd3e8d5..653cb14d 100644 --- a/packages/spf/src/core/tasks/task.ts +++ b/packages/spf/src/core/tasks/task.ts @@ -105,24 +105,28 @@ export class Task implements TaskLike { - // Memoized: run the work once; repeated calls share the same promise (which - // resolves/rejects immediately once settled). Re-running needs a `clone()`. - this.#promise ??= (async () => { - this.#status = 'running'; - try { - const result = await this.#runFn(this.#signal); - this.#value = result; // value before status — ordering guarantee - this.#status = 'done'; - return result; - } catch (e) { - this.#error = e as TError; // error before status — ordering guarantee - this.#status = 'error'; - throw e; - } - })(); + // Memoized: run the work once, sharing the same promise across calls. The + // memoized promise IS what callers await, so it's always handled (no orphan + // `Promise.resolve/reject`); a sync-throwing `#runFn` is captured as a + // rejection rather than re-run. + this.#promise ??= this.#execute(); return this.#promise; } + async #execute(): Promise { + this.#status = 'running'; + try { + const result = await this.#runFn(this.#signal); + this.#value = result; // value before status — ordering guarantee + this.#status = 'done'; + return result; + } catch (e) { + this.#error = e as TError; // error before status — ordering guarantee + this.#status = 'error'; + throw e; + } + } + abort(): void { this.#abortController.abort(); }