Summary
In sequentiallyHandleApplication (components/componentLoader.ts), a lock waiter that times out can leave an unhandled promise rejection behind, which crashes the process under Node's default --unhandled-rejections=throw.
Mechanism
When tryLock fails, the waiter parks on a promise with a setTimeout that rejects after timeout + 5_000 ms, and registers a wake callback with the lock store:
const callback = () => {
clearTimeout(timer);
whenResolved(sequentiallyHandleApplication(scope, plugin));
};
If the timeout fires first, the outer promise rejects and the caller gives up. But the wake callback is still registered. When the holder later unlocks, callback runs and evaluates sequentiallyHandleApplication(scope, plugin) before passing it to whenResolved. whenResolved is now resolve on an already-settled promise, so the settle is a no-op, but the argument was still evaluated: a fresh, completely unreferenced retry runs handleApplication again for a scope whose load already failed. If that retry rejects, nothing is attached to catch it.
This predates and is independent of #2884 (which only narrowed the lock key). #2884 reduces the contention that reaches this path but does not remove it: the same application loading on two threads still contends.
Options
- Minimal: capture the retry and attach a handler, e.g.
const retry = sequentiallyHandleApplication(scope, plugin); retry.catch((error) => harperLogger.warn?.(...)); whenResolved(retry); and skip it entirely once the outer promise has settled.
- Cleaner: adopt the wait-loop already used by
acquireRecordKey in resources/recordLock.ts, which parks on an internal wait promise woken by a single persistent slot rather than recursively spawning a fresh unawaited chain, so a late wake has nothing dangling to reject.
Raised by the automated reviewer on #2884.
Lavinia, via Claude
Summary
In
sequentiallyHandleApplication(components/componentLoader.ts), a lock waiter that times out can leave an unhandled promise rejection behind, which crashes the process under Node's default--unhandled-rejections=throw.Mechanism
When
tryLockfails, the waiter parks on a promise with asetTimeoutthat rejects aftertimeout + 5_000ms, and registers a wakecallbackwith the lock store:If the timeout fires first, the outer promise rejects and the caller gives up. But the wake callback is still registered. When the holder later unlocks,
callbackruns and evaluatessequentiallyHandleApplication(scope, plugin)before passing it towhenResolved.whenResolvedis nowresolveon an already-settled promise, so the settle is a no-op, but the argument was still evaluated: a fresh, completely unreferenced retry runshandleApplicationagain for a scope whose load already failed. If that retry rejects, nothing is attached to catch it.This predates and is independent of #2884 (which only narrowed the lock key). #2884 reduces the contention that reaches this path but does not remove it: the same application loading on two threads still contends.
Options
const retry = sequentiallyHandleApplication(scope, plugin); retry.catch((error) => harperLogger.warn?.(...)); whenResolved(retry);and skip it entirely once the outer promise has settled.acquireRecordKeyinresources/recordLock.ts, which parks on an internal wait promise woken by a single persistent slot rather than recursively spawning a fresh unawaited chain, so a late wake has nothing dangling to reject.Raised by the automated reviewer on #2884.
Lavinia, via Claude