fix: aggregate captured errors instead of dropping after the first#49
Open
voxpelli wants to merge 10 commits into
Open
fix: aggregate captured errors instead of dropping after the first#49voxpelli wants to merge 10 commits into
voxpelli wants to merge 10 commits into
Conversation
Previously, once a callback or source threw, the iterator would record only the first error and silently drop any subsequent ones still in the buffer. The captured error was then thrown when the buffer drained. Capture all errors into an array. On drain, throw the original error when only one was captured (identity-preserving), or throw an AggregateError containing all captured errors when there were two or more. The existing generator-map rejection test relied on the dropped-errors behaviour and is updated to unwrap AggregateError when present.
Aliases the new dispose method to the existing return() cleanup path so `await using it = bufferedAsyncMap(...)` runs source.return(), clears buffers, and is idempotent on repeat dispose/return calls. Bumps the supported Node range to >=22.0.0 so the well-known Symbol.asyncDispose is always available natively (Node 18 and 20 are both EOL as of May 2026), and updates the tsconfig preset and @types/node devDep to match.
Each call to bufferedAsyncMap now mints an internal AbortController
whose signal is passed as the second argument to the user callback —
`callback(item, { signal })`. The internal controller is aborted from
inside markAsEnded() so iterator.return(), iterator.throw(), and
Symbol.asyncDispose all surface as `signal.aborted === true` to any
in-flight callback within one microtask, giving callbacks a
fast-path to bail out of long-running fetches/loops on shutdown.
Existing one-arg callbacks keep working — JavaScript ignores the extra
argument — so this widening is non-breaking.
Mirrors the pattern from mcollina/hwp; the consumer-supplied signal
option layered on top arrives in the next commit.
Adds opts.signal: AbortSignal so consumers can cancel iteration without
hand-wiring signal.addEventListener('abort', () => it.return()).
Contract:
- Pre-aborted signal: source.next() is never called and the first
iterator.next() rejects synchronously with signal.reason.
- Mid-iteration abort: the next pending or freshly-called iterator.next()
rejects exactly once with signal.reason; subsequent iterator.next()
calls return { done: true, value: undefined }.
- After abort: the source iterator's .return() runs once via the
existing markAsEnded() path, and in-flight callbacks observe
signal.aborted === true on the second-arg signal within one microtask.
- signal.reason is preserved by identity, including non-Error reasons.
- Abort wins over a buffered value resolving in the same tick.
- Holds in both ordered and unordered modes and across sub-iterator
callbacks.
Implementation:
- Validates options.signal is undefined or AbortSignal at construction
time.
- Links external → internal AbortController by hand (simple addEventListener,
no AbortSignal.any) and short-circuits if the iterator was already
closed via return()/throw()/dispose so a late abort is a no-op.
- nextValue() races the buffered await against an abort sentinel and
threads abort-state through a dedicated handleAbortIfPending() helper
so the "reject once, then done forever" contract is centralised.
- The currentStep .next() chain is now then(nextValue, nextValue) so a
rejection on one .next() does not poison every subsequent call —
required for the post-abort done semantics.
Adds opts.errors: 'fail-eventually' | 'fail-fast' (default
'fail-eventually', preserving existing semantics).
In 'fail-fast' mode the first error from the callback or the source
short-circuits iteration: the next iterator.next() rejects with the
original error (no AggregateError wrapping), subsequent iterator.next()
calls return { done: true }, source.next() is never called again,
source.return() is called once, and in-flight callbacks observe
signal.aborted === true on the second-arg signal within one microtask.
Implementation reuses commit 4's abort state machine: the captured
error is routed through abortReason and internalAC.abort(err), so the
"reject once, then done forever" contract is identical to external
abort.
Precedence rules (also tested):
- fail-fast + external abort fired before any error → external reason wins.
- fail-fast + callback error before any external abort → fail-fast wins.
- fail-eventually + external abort fired with errors queued → external
reason wins; AggregateError discarded.
The default flip to 'fail-fast' and the proposed 'isolate' envelope
mode are deferred to a future major release.
Removes the long-standing describe.skip and rewrites the spec on top of
sinon useFakeTimers (matching return.spec.js), asserting that:
- iterator.throw(err) rejects with err and the next iterator.next()
returns { done: true, value: undefined }.
- The source iterator's .return() is called exactly once via the
shared markAsEnded() cleanup path.
- In-flight callbacks observe signal.aborted === true on the second-arg
signal within one microtask of throw() — confirming the throw path
reuses the same abort propagation as return()/dispose/external abort.
No production-code change.
Documents the three new public surfaces:
- options.signal: AbortSignal — with a runnable AbortController + setTimeout
example, an explicit "cancels consumption, not in-flight work" caveat,
and guidance to forward the per-callback signal into fetch/undici.
- options.errors: 'fail-eventually' | 'fail-fast' — explains the
AggregateError shape of the default mode, the Promise.all-style
semantics of fail-fast, and the precedence rule that external abort
wins over queued/captured errors.
- Symbol.asyncDispose — covers `await using` usage, idempotency, and the
Node 22+ requirement.
Updates the bufferedAsyncMap signature/options sections to surface the
new fields and the widened (item, { signal }) callback shape.
Both TODO comments (one calling out hwp's AbortController pattern as inspiration, one wondering if an AbortController could improve markAsEnded cleanup) are now resolved by the per-callback signal and external-signal commits.
chai-quantifiers ships its own type declarations (its package.json sets "types": "src/index.d.ts"), so the separate @types/chai-quantifiers package is unused. knip has been flagging this; removing it lets the pre-push check chain pass cleanly.
Captures the JSDoc-as-source convention, the npm test pre-push gate, the bufferedAsyncMap state machine (internalAC, abortReason, capturedErrors, fillQueue/nextValue split, markAsEnded as single cleanup path), the public-API contracts worth preserving, and the IIFE + clock.runAllAsync test pattern future contributors need to avoid fake-timer deadlocks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, once a callback or source threw, the iterator would record
only the first error and silently drop any subsequent ones still in the
buffer. The captured error was then thrown when the buffer drained.
Capture all errors into an array. On drain, throw the original error
when only one was captured (identity-preserving), or throw an
AggregateError containing all captured errors when there were two or
more.
The existing generator-map rejection test relied on the dropped-errors
behaviour and is updated to unwrap AggregateError when present.