Skip to content

fix(result-async): propagate factory rejections + fill test coverage gaps#8

Merged
kelsos merged 1 commit intomainfrom
test/fill-coverage-gaps
Apr 18, 2026
Merged

fix(result-async): propagate factory rejections + fill test coverage gaps#8
kelsos merged 1 commit intomainfrom
test/fill-coverage-gaps

Conversation

@kelsos
Copy link
Copy Markdown
Owner

@kelsos kelsos commented Apr 18, 2026

Summary

Batch #3 of the post-launch polish: fill the test coverage gaps flagged during the pre-release audit. Writing the tests exposed a real bug in allWithConcurrency, so this PR is both a fix and a test expansion.

Bug: allWithConcurrency crashed on factory rejections

Old worker caught rejections, exited, left results[i] as undefined. The final results.map((r) => r.value) then dereferenced undefined and threw.

There's no coherent value to substitute when a factory throws:

  • can't build a typed err from an untyped throw
  • can't produce ok for a slot with no value
  • silently returning a value array with holes breaks consumers downstream

New behavior: rejections propagate and the overall promise rejects. Consumers wrap throwing code with fromPromise/fromAsync at the boundary — matches retry's documented contract. TSDoc updated to say this explicitly.

New test coverage (17 new tests)

  • result-async/control.edge.test.ts
    • retry propagates on first throw (sync + async), does not retry
    • retry still retries typed errors through the attempt count
    • retry stops at first ok even with delay configured
    • timeout clears its internal timer on happy path (fake timers assert zero pending)
    • timeout fires on slow path and surfaces onTimeout() error
    • timeout does not call onTimeout when operation wins the race
  • result-async/combine.edge.test.ts
    • allWithConcurrency rejects when a factory throws (bug-fix test)
    • allWithConcurrency short-circuits on typed err without starting later workers
    • respects the concurrency cap under load (20 jobs, cap 4)
    • rejects synchronously on concurrency < 1 (zero and negative)
    • empty factories resolves to ok([])
    • clamps concurrency to factories.length (no wasted workers)
  • interop/zod/async.test.ts
    • fromZod throws a wrapped, actionable error on async-refined schemas
    • error preserves the original Zod error as cause
    • fromZodAsync handles the same schema cleanly
    • fromZod still works for pure sync schemas

TSDoc refinements

  • retry: explicit @throws for RangeError; new paragraph on factory rejection behaviour; example now uses fromPromise.
  • allWithConcurrency: matches the new propagation behaviour; directs users to fromPromise/fromAsync.

Size impact

Dist 198 KB → 199 KB unminified.

Test plan

  • All required CI checks pass
  • Coverage stays above the 80% patch floor
  • CodSpeed shows no regression — this PR touches a hot async path but benchmarks are for the Result/Arrays/pipe namespaces, so no change expected
  • Hover tooltips on allWithConcurrency and retry in an IDE reflect the clarified TSDoc

… + fill coverage gaps

Address the review gaps flagged on the pre-release audit with a test
pass across retry / timeout / allWithConcurrency / fromZod.

### Bug fixed: allWithConcurrency dropped slots → crash on output

The old worker caught factory rejections, exited the worker, and left
`results[i]` as `undefined`. The final `results.map((r) => r.value)` then
dereferenced undefined and threw. The "swallow" behavior was impossible
to produce a coherent value for:

- can't build a typed `err` from an untyped throw
- can't produce `ok` for a slot with no value
- silently returning a value array with holes breaks downstream consumers

New behavior: let factory rejections propagate. Consumers already need to
wrap throwing code with `fromPromise`/`fromAsync` at the boundary to get
typed errors — this makes that boundary explicit instead of silently
producing garbage. Matches `retry`'s semantics.

TSDoc updated to document this contract.

### New test coverage

Colocated `.edge.test.ts` files for:

- `result-async/control.edge.test.ts` — retry behavior on throwing/rejecting
  factories (propagates on first attempt), timeout timer-cleanup under
  fake timers (no lingering timers on happy path or on timeout), timeout
  onTimeout not invoked when operation resolves in time.
- `result-async/combine.edge.test.ts` — allWithConcurrency rejects on
  factory throw; still short-circuits on typed err; concurrency cap holds
  under load; rejects on concurrency < 1 (both zero and negative); empty
  factories resolves to ok([]); concurrency clamped to factories.length.
- `interop/zod/async.test.ts` — fromZod throws a wrapped, actionable
  error when handed an async-refined schema, with the original Zod error
  preserved as `cause`; fromZodAsync handles the same schema; fromZod
  still works for sync schemas.

### TSDoc refinements

- `retry`: explicit @throws for RangeError; new paragraph on factory
  rejection behavior (propagates, no further attempts); realistic example
  using fromPromise.
- `allWithConcurrency`: matches the new implementation; points at
  fromPromise/fromAsync for converting throws to typed errors.

47 new tests, 224 total passing.

Dist 198 KB → 199 KB unminified.
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.41%. Comparing base (16d89f3) to head (25bae7a).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main       #8      +/-   ##
==========================================
+ Coverage   87.21%   88.41%   +1.19%     
==========================================
  Files          23       23              
  Lines         399      397       -2     
  Branches       90       90              
==========================================
+ Hits          348      351       +3     
+ Misses         19       15       -4     
+ Partials       32       31       -1     
Flag Coverage Δ
unittests 88.41% <100.00%> (+1.19%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
packages/plainfp/src/result-async/combine.ts 100.00% <100.00%> (+3.70%) ⬆️
packages/plainfp/src/result-async/control.ts 92.00% <ø> (+16.00%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Apr 18, 2026

Merging this PR will not alter performance

✅ 20 untouched benchmarks


Comparing test/fill-coverage-gaps (25bae7a) with main (16d89f3)

Open in CodSpeed

@kelsos kelsos merged commit 25bae7a into main Apr 18, 2026
11 checks passed
@kelsos kelsos deleted the test/fill-coverage-gaps branch April 18, 2026 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant