feat!: add AbortSignal, Symbol.asyncDispose and fail-fast error handling (v2.0.0)#50
Open
voxpelli wants to merge 21 commits into
Open
feat!: add AbortSignal, Symbol.asyncDispose and fail-fast error handling (v2.0.0)#50voxpelli wants to merge 21 commits into
voxpelli wants to merge 21 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.
voxpelli
changed the base branch from
copilot/harden-implementation-type-safety
to
main
voxpelli
force-pushed
the
claude/merge-branches-to-master-nixCB
branch
from
ba54b28 to
1b45e12
Compare
Layers minimal refactors on top of the abort-signal feature commits:
- Add normalizeError helper (lib/misc.js) and use it in all 5 fillQueue /
fail-fast catch sites
- Extract isValueObject helper in lib/type-checks.js (DRY for isIterable /
isAsyncIterable)
- Add bounds check to ordered-insertion while loop in fillQueue so the
BufferPromise type cast is honest past array end
- Inline null-safety on the IteratorResult shape check
(`!result || typeof result !== 'object'`); typeof null === 'object' would
have let null through to the next-line property accesses
Drops the larger-scope tweaks from the harden / efficiency branches that
weren't earning their keep:
- @voxpelli/typed-utils runtime dep (kept the lib zero-dep; the only
material correctness win — null check on IteratorResult — is inlined)
- yieldArrayWithItem generator (replaces a 1-element array allocation in
the common case with a generator allocation; not a measurable win and
less readable than the spread)
- guardedArrayIncludes (the type cast in isPartOfArray is honest enough)
BREAKING CHANGE: engine requirement bumped from >=18.6.0 to >=22.0.0 (native
Symbol.asyncDispose is required). Callback signature widened from
`(item)` to `(item, { signal })`; existing one-arg callbacks keep working
since JS ignores extra args, but TypeScript consumers that pass a callback
type with a strict single-parameter signature may need to update the type.
voxpelli
force-pushed
the
claude/merge-branches-to-master-nixCB
branch
from
1b45e12 to
1d87f8e
Compare
- Add normalizeError(err, defaultMessage) to lib/misc.js entry, with a reuse hint to avoid open-coding the err-instanceof-Error pattern. - Add isObject to lib/type-checks.js entry; note it closes the typeof null === 'object' hole and is what isAsyncIterable / isIterable / the IteratorResult shape check are now built on.
…variants
- Hoist raceAbort() into a single abortPromise created at construction so
the {once:true} listener count does not grow per consumer pull. Flatten
the per-call Promise.race so buffered promises and abortPromise live in
one input array.
- Use IteratorYieldResult<R> as the cast type for the yielded result
(still {value} at runtime — the lib type's done is optional false, and
two existing specs assert on the loose shape).
- Rename local normalisedErr -> normalizedErr to match the helper name.
- Document the load-bearing invariants inline (why internalAC is
unconditional, why abortPromise is shared, why return() bypasses the
currentStep chain) and expand JSDoc on bufferedAsyncMap, markAsEnded,
fillQueue, nextValue, and handleAbortIfPending so future readers don't
have to re-derive them from tests.
Refresh CLAUDE.md to reflect the shared abortPromise and add an
"Implementation invariants worth preserving" block plus a "Style notes"
note (zero-runtime-dep / one-listener-per-call / unconditional-internalAC,
helper/local spelling convention).
Autofix from npx eslint --fix; pre-existing warning unrelated to the preceding logic change.
- Rename internalAC -> internalAbortController in index.js and CLAUDE.md (the variable was short for AbortController; the abbreviation overloaded with "AC X.Y" acceptance-criteria refs and was hard to read). - Strip "AC X.Y(.Y)?(+...)?: " prefix from 41 it() descriptions across test/abort.spec.js, test/dispose.spec.js, test/errors.spec.js, test/errors-fail-fast.spec.js, test/per-task-signal.spec.js, and test/throw.spec.js. Acceptance-criteria numbering isn't persisted outside the test labels themselves so the prefixes don't add value. - Replace AC-numbered prose in index.js comments and the CLAUDE.md invariants block with descriptive references to the spec files.
mergeIterables previously only accepted bufferSize. Widen its options bag to forward signal, errors, and ordered through to bufferedAsyncMap. Pure pass-through — no new code paths, no test breakage. Existing "should process iterables in parallel" still passes unchanged because it doesn't specify ordered. Also lands the sensible follow-ups from the /review pass: - More specific "Expected ... iterator next() result to be an object" TypeError messages so the stack distinguishes subiterator vs source iterator protocol violations. test/values.spec.js updated. - README: document the new mergeIterables options; clarify that in-flight callbacks see signal.aborted within one microtask of iterator close (but Promises can't be cancelled); add an explicit "Requirements: Node.js >=22.0.0" line. Fix "simultanoeus" / "ordinare" typos. - test/values.spec.js: three new specs covering signal abort, fail-fast error mode, and ordered:true merging.
…ofix sort - .github/workflows/nodejs.yml: node-versions 18,20,21 -> 22,24. The previous matrix was below the engines floor (>=22.0.0) and only passed because `[Symbol.asyncDispose]` becomes the string key "undefined" when the global symbol is absent — so the dispose specs never exercised the real path. Node 22 (current LTS) and Node 24 are now in matrix. - index.js: sort the mergeIterables type and inner option object so `errors` precedes `ordered`/`signal` alphabetically; lands the perfectionist/sort-objects auto-fix. - test/values.spec.js: rewrite the "ordered: true" mergeIterables spec with asymmetric timing (first source 10x slower than second). Under the default ordered:false this would interleave second-* values between first-1 and first-2; under ordered:true the first iterable drains completely first. The previous symmetric-timing version would have passed even if `ordered` had been silently dropped.
There was a problem hiding this comment.
Pull request overview
This PR extends bufferedAsyncMap (and the mergeIterables wrapper) with cancellation and improved error surfacing by adding AbortSignal support, a configurable fail-fast error mode, and Symbol.asyncDispose for deterministic cleanup.
Changes:
- Add
options.signalcancellation, a per-callback{ signal }argument, and iterator support forSymbol.asyncDispose. - Add
errors: 'fail-fast' | 'fail-eventually'mode and normalize/aggregate error reporting semantics. - Update docs, Node engine/CI targets, and expand test coverage for abort, disposal, per-task signals, and fail-fast behavior.
Reviewed changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Updates TS config baseline (extends Node 20 config). |
| package.json | Bumps Node engine to ≥22 and updates type deps. |
| index.js | Implements abort handling, per-task signal propagation, fail-fast mode, and Symbol.asyncDispose. |
| lib/type-checks.js | Adds isObject() and refactors iterable guards to use it. |
| lib/misc.js | Adds normalizeError() helper for non-Error rejections. |
| README.md | Documents new options (signal, errors), per-task cancellation, and resource management semantics. |
| .github/workflows/nodejs.yml | Updates CI Node matrix to 22/24. |
| CLAUDE.md | Adds repo guidance and documents the iterator state machine/contracts. |
| test/values.spec.js | Adjusts expectations for single-error vs AggregateError behavior and updated TypeError messages. |
| test/throw.spec.js | Enables/updates throw() tests and adds signal/cleanup assertions. |
| test/per-task-signal.spec.js | Adds coverage ensuring per-task signals exist and abort on close paths. |
| test/errors.spec.js | Adds coverage for fail-eventually error identity vs AggregateError. |
| test/errors-fail-fast.spec.js | Adds comprehensive fail-fast behavior, source cleanup, and abort interaction tests. |
| test/dispose.spec.js | Adds tests for Symbol.asyncDispose cleanup/idempotency. |
| test/abort.spec.js | Adds abort semantics tests including “reject once then done”, cleanup, and race coverage. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
333
to
+360
| const nextValue = async () => { | ||
| { | ||
| const earlyAbort = handleAbortIfPending(); | ||
| if (earlyAbort) { | ||
| await markAsEnded(); | ||
| return earlyAbort; | ||
| } | ||
| } | ||
|
|
||
| const nextBufferedPromise = bufferedPromises[0]; | ||
|
|
||
| if (!nextBufferedPromise) return markAsEnded(true); | ||
| if (isDone) return { done: true, value: undefined }; | ||
|
|
||
| // Single flat Promise.race: abortPromise is the last entry so a buffered | ||
| // value resolving in the same tick still gets re-checked against | ||
| // abortReason below. | ||
| const raced = await Promise.race( | ||
| ordered | ||
| ? [nextBufferedPromise, abortPromise] | ||
| : [...bufferedPromises, abortPromise] | ||
| ); | ||
|
|
||
| if (raced === ABORT_SENTINEL || abortReason) { | ||
| const handled = handleAbortIfPending(); | ||
| await markAsEnded(); | ||
| return handled ?? { done: true, value: undefined }; | ||
| } |
Owner
Author
There was a problem hiding this comment.
Addressed in dd775d7: handleAbortIfPending now returns a descriptor ({ kind: 'throw', reason } | { kind: 'done' } | undefined) instead of throwing, and both call sites in nextValue (the early-abort check and the post-race branch) run await markAsEnded() before propagating the abort reason — matching the ordering the fail-fast branch already used. test/abort.spec.js was strengthened to assert source.return() has been called once after the first rejecting .next() resolves, before any subsequent drain call (the previous assertion only checked after a second .next(), which masked the issue).
Generated by Claude Code
| } // source.return() runs here, regardless of how the block exited | ||
| ``` | ||
|
|
||
| `Symbol.asyncDispose` is aliased to `iterator.return()` and is idempotent. Native `await using` requires Node 22+ (or a transpiler). |
Previously handleAbortIfPending threw synchronously when an abort was
pending fresh delivery, propagating out of nextValue (async) as a
rejected promise before the await markAsEnded() lines ran. For direct
.next() consumers this meant source.return() wasn't called until either
a subsequent .next() or an explicit iterator.return() — the for await
loop's implicit cleanup-on-exception papered over the issue, but the
abort-delivery contract was not actually carrying its own cleanup.
Restructure handleAbortIfPending to return a discriminated descriptor
({kind:'throw', reason} | {kind:'done'} | undefined) instead of
throwing. The two call sites in nextValue (early-abort check and
post-race branch) now run markAsEnded() before propagating the throw,
matching the fail-fast branch's ordering.
Pin the new contract in test/abort.spec.js by asserting
returnSpy.calledOnce after the first rejecting .next() resolves, before
the drain call that previously masked the issue.
Also reword README §"Resource management": Symbol.asyncDispose is
equivalent to iterator.return() for cleanup, not literally aliased (the
dispose method has its own body returning Promise<void> per the
AsyncDisposable contract).
Resolves copilot-pull-request-reviewer comments on PR #50.
- .knip.jsonc: drop the stale `entry` array (no benchmark/ dir; index.js is auto-detected from package.json exports) — knip no longer prints configuration hints on every check run. - index.js: remove the stale `// TODO: Add "throw"` comment that sat directly above the implemented throw() method; document the invariant behind the currently-unreachable ordered-insertion loop body so the coverage gap reads as intentional. - test: cover normalizeError's non-Error branch via a callback that rejects with a non-Error value; lib/misc.js is now at 100%. PR description rewritten and the two copilot-pull-request-reviewer threads resolved separately on GitHub.
voxpelli
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
voxpelli
changed the title
voxpelli
changed the title
Guards against performance regressions in the buffered-async-iterable
state machine. One mitata group() per design decision in this PR:
- overhead: bufferedAsyncMap vs a raw for-await over the same source
- abort wiring: no options / options.signal / errors:'fail-fast' must
stay within noise of each other (proves the always-on
internalAbortController + per-callback {signal} + shared abortPromise
add no hot-path cost)
- dispatch loop: ordered vs unordered
- bufferSize scaling (1 / 4 / 16 / 64)
- nested sub-iterators (async-generator callback) vs a flat baseline
- mergeIterables wrapper overhead vs calling bufferedAsyncMap directly
Methodology (per mitata best practices): fixtures in benchmark/fixtures.js
never use timers — asyncRange yields on the microtask queue so the
numbers reflect the library's per-item bookkeeping, not simulated I/O.
do_not_optimize wraps the drained result so the JIT cannot eliminate the
loop; mitata handles warmup and flags dead-code-eliminated benches with
`!`.
- package.json: add mitata devDependency + `npm run bench` script
(node --expose-gc --allow-natives-syntax); exclude benchmark/**/*.js
from type-coverage (mirrors test/*.spec.js).
- .knip.jsonc: benchmark/index.js is the knip entry for the dir.
- CLAUDE.md: document the suite and the no-timers-in-fixtures rule.
- Split benchmark/index.js into theme files (throughput / abort /
nested) behind a thin index.js entry that parses an optional name
filter and a --json flag.
- Harden: .gc('inner') on the allocation-heavy groups (overhead,
bufferSize, input shape, nested) to remove cross-iteration GC noise;
run({ throw: true }) so a broken bench fails the process loudly.
- Deepen: new "abort & error delivery" group (pre-aborted signal,
mid-stream external abort, fail-fast trigger, fail-eventually
AggregateError accumulation) and a sync-iterable / array input
comparison exercising makeIterableAsync — all previously
unbenchmarked code paths.
- Add fixtures: syncRange, rejectingCallback.
- Add `npm run bench:json` for capture/diff against a local baseline.
- CLAUDE.md: refresh the Benchmarks section for the new layout.
- README.md: add a "## Performance" section with the stable
qualitative findings and relative ratios.
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
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.
Summary
Reconciles three feature branches into one release. Adds cancellation, deterministic cleanup, and a configurable error mode to
bufferedAsyncMap, bringsmergeIterablesto option parity, and applies the review-driven refactors and cleanup that followed.Features
Cancellation —
options.signalAbortSignal. When aborted, the next pending or freshly-callediterator.next()rejects withsignal.reasonexactly once; subsequent calls return{ done: true, value: undefined }..return()runs once as part of the abort-delivery path.Per-callback signal —
callback(item, { signal }){ signal }that is always present (even with nooptions.signal) and aborts on iterator close —return()/throw()/Symbol.asyncDispose/ source exhaustion / external abort / first fail-fast error. Lets callbacks forward the signal intofetch/undiciand fast-path on shutdown.Symbol.asyncDisposeSymbol.asyncDisposeforawait using, equivalent toiterator.return()for cleanup and idempotent.Error mode —
options.errors'fail-eventually'(default) keeps the historical "drain then throw" semantics: one captured error is thrown with identity preserved, two or more are wrapped in anAggregateError.'fail-fast'mirrorsPromise.all: the first error short-circuits iteration, the next.next()rejects with the original error, the source's.return()runs once.mergeIterablesparitymergeIterablesnow forwardsbufferSize,ordered,signalanderrorsthrough tobufferedAsyncMap(previouslybufferSizeonly).Internals & housekeeping
abortPromise(one{ once: true }listener, created at construction) instead of a per-.next()listener; flattened thePromise.race.handleAbortIfPendingreturns a descriptor instead of throwing, somarkAsEnded()always runs before the abort reason propagates.libhelpers reused across the catch sites:isObject,normalizeError.internalAbortController; tightenedTypeErrormessages; bounds-checked ordered-insertion loop.>=22.0.0,tsconfigto the Node 20 preset, CI matrix to Node 22 + 24.CLAUDE.mdadded documenting the state machine and the invariants worth preserving.Breaking changes
>=18.6.0to>=22.0.0(nativeSymbol.asyncDispose).callback(item)tocallback(item, { signal })— runtime-compatible; only relevant to consumers passing a strictly-typed single-parameter callback type.Tests & review
abort,dispose,errors,errors-fail-fast,per-task-signal,throw,values;lib/at 100% coverage,index.js≥98% strict.copilot-pull-request-reviewerfindings (abort-cleanup ordering,Symbol.asyncDisposedoc wording) addressed; review threads resolved.release-pleasewill cut2.0.0from thefeat!:history on merge.https://claude.ai/code/session_01FSyTJjbP823pjYfueuSsdu