Skip to content

fix(signals): settle async iterators on empty completion and sync rej…#2870

Merged
ryansolid merged 1 commit into
solidjs:nextfrom
yumemi-thomas:fix/async-iterator-settlement
Jul 10, 2026
Merged

fix(signals): settle async iterators on empty completion and sync rej…#2870
ryansolid merged 1 commit into
solidjs:nextfrom
yumemi-thomas:fix/async-iterator-settlement

Conversation

@yumemi-thomas

Copy link
Copy Markdown

Closes #2868
Closes #2869

Summary

Two settlement gaps in the async-iterator branch of the client async runtime, both of which leave the node STATUS_PENDING forever — the <Loading> fallback never leaves and <Errored> never renders:

  1. Empty completion (2.0.0-beta.16: async iterable that completes without yielding leaves <Loading> pending forever #2868): an async iterable that completes ({done: true}) asynchronously without ever yielding never settles. The sync completion path already commits undefined, so a stream that legitimately produces zero items is indistinguishable from a hung request.
  2. Synchronous rejection (2.0.0-beta.16: async-iterator rejection delivered synchronously is silently dropped (pends forever) #2869): a next() thenable that calls onRejected before .then() returns is dropped with no error handling. The promise branch handles the exact same thenable correctly — only the iterator branch loses it.

Repros: https://stackblitz.com/edit/axbsw9wp-5do9ntby?file=src%2FApp.tsx (#2868), https://stackblitz.com/edit/3etmyz9r-pn3hs9cj?file=src%2FApp.tsx (#2869)

Root cause

Both are missing halves of the sync/async duality the promise branch handles a few lines above. The fulfillment handler settles yielded values but not an empty completion (completed = true; schedule(); flush() — no write, no status change), and the rejection handler is guarded by if (!isSync && …), so a rejection delivered during the synchronous window is ignored entirely with nothing else recording it.

Fix

Mirror the promise branch:

  • Empty completion: when {done: true} arrives asynchronously and no value was ever yielded (hadSyncValue broadened to hadValue, which now also tracks async yields), settle through the standard path with asyncWrite(undefined) — matching the immediately-done sync iterator. Completion after a yield keeps the existing schedule/flush (the last value is already committed).
  • Synchronous rejection: record rejected / syncError during the synchronous window, then route through handleError(syncError) once the window closes, exactly like the promise branch — including the rethrow that unwinds the in-progress synchronous read. The rethrow is gated to the initial read: later iterate() calls run from asyncWrite continuations, where rethrowing would produce an unhandled rejection; there handleError settles the node and iteration stops.

No change to the per-yield hot path beyond one boolean assignment; stale-iterator writes remain guarded by the existing el._inFlight checks.

Solid 1.x note

Not applicable — async-iterable memo bodies are a new 2.0 API with no 1.x counterpart. Same hardening class as the server-side sync-thenable fixes in #2858, client iterator branch.

Tests

Seven new tests in tests/syncThenable.test.ts, five red-first (confirmed failing on the unfixed source) and two passing behavior-pins:

  • empty async iterator settles to undefined (red)
  • sync rejection on the initial read reaches the boundary with the original error (red)
  • sync rejection after synchronously yielded values (red)
  • nullish sync rejection preserves undefined through the boundary (red; composes with 2.0.0-beta.16: Errored fallback receives the internal StatusError wrapper for bare rejections #2866)
  • sync rejection after an asynchronous yield settles without an unhandled rejection (red)
  • completion after async yields preserves the last value (pin)
  • async completion after a synchronous yield preserves it (pin)

Both issue repros confirmed reproducing against the published 2.0.0-beta.16 npm package. Full solid-signals / solid-js / @solidjs/web suites pass.

Full disclosure: I wrote this with the help of an AI assistant (Claude Fable 5) and reviewed every line before pushing. All red-first tests were confirmed failing on the unfixed code, and the bugs were verified against the published beta.16 npm package, not just this repo.

@changeset-bot

changeset-bot Bot commented Jul 10, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: c46b34e

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 9 packages
Name Type
@solidjs/signals Patch
test-integration Patch
solid-js Patch
babel-preset-solid Patch
@solidjs/web Patch
@solidjs/html Patch
@solidjs/h Patch
@solidjs/universal Patch
@solidjs/element Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@codspeed-hq

codspeed-hq Bot commented Jul 10, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 25.66%

⚡ 1 improved benchmark
❌ 1 regressed benchmark
✅ 118 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
merge 266.2 µs 372.6 µs -28.56%
merge 166.9 µs 75.5 µs ×2.2

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing yumemi-thomas:fix/async-iterator-settlement (c46b34e) with next (a99d6c8)

Open in CodSpeed

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.

2 participants