Skip to content

fix(signals): settle resolve() when the async source rejects - #2849

Closed
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/resolve-async-rejection
Closed

fix(signals): settle resolve() when the async source rejects#2849
brenelz wants to merge 1 commit into
solidjs:nextfrom
brenelz:fix/resolve-async-rejection

Conversation

@brenelz

@brenelz brenelz commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #2842resolve(fn) hung forever (never resolved, never rejected) and leaked its internal createRoot whenever the awaited async source rejected after having been pending. Resolution worked; rejection did not.

Root cause

resolve() was built on a bare computed inside a root, and computeds are pull-based. The two settlement paths are asymmetric:

  • When a pending source resolves, settlePendingSource() explicitly re-enqueues blocked dependents, so the computed re-ran, called res(fn()), and disposed the root.
  • When a pending source rejects, handleAsync's error path only propagates STATUS_ERROR through notifyStatus(), which marks subscribers errored for their next pull but never re-runs them. Nothing ever reads resolve()'s computed (it has no subscribers), so neither res nor rej fired and dispose() never ran.

Fix

Rebuild resolve() on a user effect — the primitive that owns an error-notification channel (_notifyStatus_errorFn) and is therefore actively told when a pending source rejects:

  • effect callback → res(value) + dispose (same observable timing as before)
  • error callback → rej(err) + dispose, unwrapping the internal StatusError wrapper so the promise rejects with the user's original error, matching what createErrorBoundary exposes

Also documents the rejection behavior in the JSDoc.

Tests

New tests/resolve.test.ts pins the settlement contract:

  • rejects with the original error when the async source rejects after pending (hung before this fix)
  • disposes the internal root on rejection (hung before this fix)
  • still resolves with the settled value on success
  • rejects on a synchronous throw

Full solid-signals suite (814 tests) and solid suite (418 tests) pass.

🤖 Generated with Claude Code

resolve() was built on a bare computed, which is pull-based: a pending
source that resolves re-enqueues blocked dependents via
settlePendingSource(), but a rejection only marks subscribers errored
for the next pull. Nothing ever pulls resolve()'s computed, so the
returned promise never settled and the internal root leaked.

Rebuild resolve() on a user effect — the primitive with an error
notification channel — so a rejection rejects the promise with the
original error (StatusError unwrapped, matching error boundaries) and
disposes the root.

Fixes solidjs#2842

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: 59d5291

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/resolve-async-rejection (59d5291) with next (e2ebc11)

Open in CodSpeed

@ryansolid

Copy link
Copy Markdown
Member

Root cause is exactly right, and the asymmetry framing is the valuable part: settlePendingSource re-enqueues blocked dependents on resolution, but a rejection only propagates STATUS_ERROR through notifyStatus — which marks a pull-based computed errored for a pull that, with no subscribers, never comes. resolve()'s bare computed was exactly that shape, so the promise hung and the root leaked.

Rebuilding on a user effect is the right fix — the effect's error-notification channel (_notifyStatus_errorFn) is the primitive that's actively told about rejections, and unwrapping the StatusError so the caller gets their original error matches what error boundaries expose. Verified the hang on the current tree, confirmed the full monorepo suite passes with your diff, and landed it as-is with a changeset, credited as co-author: c4ba5268. Ships in the next beta — thanks for another precise one.

@ryansolid ryansolid closed this Jul 8, 2026
ryansolid added a commit that referenced this pull request Jul 8, 2026
…from #2849)

resolve() sat on a bare computed with no subscribers: settlePendingSource
re-enqueues blocked dependents when a pending source resolves, but a
rejection only propagates STATUS_ERROR via notifyStatus — marking the
computed errored for a pull nothing would ever perform. The promise never
settled and the internal root leaked. Rebuild on a user effect, whose
_notifyStatus → _errorFn channel is actively invoked on rejection: resolve
now rejects with the user's original error (StatusError unwrapped, same as
error boundaries expose) and disposes its root on every terminal state.

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