Skip to content

fix(signals): settle async-generator actions on uncaught errors instead of freezing - #2847

Closed
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/async-generator-action-freeze
Closed

fix(signals): settle async-generator actions on uncaught errors instead of freezing#2847
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/async-generator-action-freeze

Conversation

@brenelz

@brenelz brenelz commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2841 — an uncaught error inside an async-generator action() froze the JS thread: the returned promise never settled, the event loop was starved by an infinite microtask loop, and the transition never completed because the iterator was never removed from _actions.

Root cause

When an async generator's body throws, it.next() returns a rejected promise with the generator already in the completed state. The action runner responded to that rejection by calling it.throw(e) — but throw() on a completed async generator just returns another rejected promise, whose rejection handler called it.throw(e) again, forever. Each iteration is a microtask, so the loop starves the event loop (frozen tab / hung process). Sync generators were unaffected because their it.next() throws synchronously, which was already handled.

Fix

A rejected iterator-result promise is terminal: async generators run user try/catch/finally internally before the rejection ever reaches the runner. So the rejection handler now settles the action via done(undefined, e) — mirroring how the fulfilled path handles done: true. The returned promise rejects, the iterator is removed from the transition's _actions, and the transition can complete.

try/catch around yield/await inside async generators still works (handled by the generator machinery itself), and the sync-generator it.throw path for yielded promise rejections is unchanged.

Tests

Four regression tests in action.test.ts:

  • immediate throw in an async generator rejects and does not starve the event loop (the issue's repro — this test hangs without the fix)
  • throw after an await/yield rejects
  • try/catch inside an async generator around an awaited rejection recovers and continues
  • a failing async-generator action is removed from the transition so a concurrent action still commits

Full solid-signals suite passes (818 tests, 49 files).

🤖 Generated with Claude Code

…ad of freezing

When an async generator's body throws, `it.next()` returns a rejected
promise with the generator already completed. The action runner responded
by calling `it.throw(e)`, which on a completed async generator just
returns another rejected promise — producing an infinite microtask loop
that starved the event loop and left the iterator in the transition's
`_actions` forever.

A rejected iterator result is terminal (async generators run user
try/catch/finally internally before rejecting), so settle the action via
`done(undefined, e)` instead: the returned promise rejects, the iterator
is removed from `_actions`, and the transition can complete.

Fixes solidjs#2841

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@changeset-bot

changeset-bot Bot commented Jul 7, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: fbe54ff

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@codspeed-hq

codspeed-hq Bot commented Jul 7, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 120 untouched benchmarks


Comparing brenelz:fix/async-generator-action-freeze (fbe54ff) with next (e2ebc11)

Open in CodSpeed

@ryansolid

Copy link
Copy Markdown
Member

Diagnosis and fix are both exactly right — a rejected iterator-result promise from an async generator is terminal (the body's error already escaped, user try/catch already ran inside the generator machinery), so throwing back in could only ever produce another rejected promise, and each round trip being a microtask is what starved the loop. Settling via done mirrors the synchronous catch path and correctly removes the iterator from the transition's _actions.

We verified the freeze on the current tree (the repro genuinely hangs a vitest worker), confirmed no conflict with the unhandled-error-halts philosophy (this is the action promise contract, which sync generators already honored — not an effect-queue error), and landed your diff as-is with a changeset, credited as co-author: 0e811993. All four regression tests included. Ships in the next beta — thanks!

@ryansolid ryansolid closed this Jul 8, 2026
ryansolid added a commit that referenced this pull request Jul 8, 2026
…ad of freezing (#2841, from #2847)

A rejected iterator-result promise (async generators) is terminal: the error
already escaped the generator body and it is completed, so it.throw() only
returns another rejected promise — the old handler re-threw forever, starving
the event loop in a microtask loop and never removing the iterator from the
transition's _actions. Settle via done() instead, mirroring the sync catch
path. User try/catch inside async generators still works (the generator
machinery handles it before the runner ever sees a rejection) and the
sync-generator throw path for yielded promise rejections is unchanged.

Co-authored-by: Brenley Dueck <brenelz@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
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