Skip to content

fix(web): surface a re-asked frame's error instead of re-asking forever - #3922

Open
ryansolid wants to merge 4 commits into
nextfrom
fix/frames-reask-loop
Open

ryansolid wants to merge 4 commits into
nextfrom
fix/frames-reask-loop

Conversation

@ryansolid

@ryansolid ryansolid commented Oct 8, 2026 •

Copy link
Copy Markdown
Member

Problem

A server component that throws during render ships its error as the sanitized message string (sink.error("", message) in src/server.ts and frames/src/frame-sink.ts). The frames client's landing() decided whether an error was one it had already surfaced by comparing the error value. Two flights that fail the same way carry equal strings, so a re-asked flight's error looked like the old one and the node re-asked again. That flight's error record bumped the mount's failed tick, the memo recomputed, and it re-asked again, with no end.

It shows up whenever a re-ask meets a repeatable server error:

  • <Errored> reset() on a component that keeps failing;
  • a mount over an address whose preload already failed. In the hackernews example, hovering a job post's /users/null link and then clicking it produced about 3000 requests per second, and the navigation never committed.

The existing reset spec missed it because its fixtures send { message } objects, and a freshly parsed object never compares equal.

Fix

landing() keys the surfaced error by the response version that carried it, read off the frame bound to the address, instead of by the payload. Every flight lands under a new version, so a re-asked flight's error is new however alike, while a re-read of the error already thrown (a reset, or a fresh consumer of an errored address) is still a re-ask. What <Errored> receives is unchanged.

Behaviour changes

  • A re-asked flight that fails with the same message now surfaces to the nearest <Errored> (or halts with REACTIVITY_HALTED when there is none) instead of looping.
  • Two stream-level error records within one response (e.g. the server's error followed by the body failing) surface once, not twice.

Public API changes

None. landing() is internal to @solidjs/web/frames.

Tests

New packages/web/test/frames-reask-loop.spec.tsx, using the server's real string payload. The fetch stub stops answering past 25 calls so a regression fails at a readable count instead of exhausting memory. The cases:

  • reset() re-asks once per reset, and the equal error surfaces again (via dynamic and dynamicComponent);
  • a mount over an errored preload surfaces the next flight's equal error, then stops asking (both entry points);
  • with no <Errored>, the same mount halts instead of re-asking.

All five fail at the cap without the fix. The full client web suite passes.

I also checked it end to end in the hackernews example (with #3910 applied, which that example needs). After the hover and click, the request count settles at the route's call plus one re-ask.

Follow-ups (not in this PR)

  • Server render errors reach <Errored> as a string, but transport failures (and keyed fragment errors) as { message } objects. Aligning them would be a public behaviour change.
  • The reset spec's fixture comment calls { message } "the server's own shape"; the server sends a string.

Size

The first version keyed the error through a helper that read frame.version and re-read the frame to throw, which cost +56 B minified per page. The trim folds the frame read, the version key and the surfaced check into one helper that returns false (no error), 1 (already surfaced, so re-ask) or 2 (new, so throw). reask() now chains the next landing itself. Behaviour is unchanged, and the change is now 5 B smaller minified than next. No cap or exception changes.

Bytes, measured locally with the CI harness after merging next (a963ec1). Minified is deterministic; brotli moves by tens of bytes with layout.

Scenario next min / br before trim min / br after trim min / br brotli cap gate
app: render + one signal 27,887 / 9,917 27,887 (+0) / 9,917 27,887 (0) / 9,917 9,930 under cap (13 B)
frames: eager client consumer 33,418 / 11,112 33,474 (+56) / 11,117 33,413 (-5) / 11,111 11,130 under cap (19 B)
page: base server components 105,413 / 33,910 105,469 (+56) / 33,904 105,408 (-5) / 33,899 33,920 under cap (21 B)
page: live server components 117,456 / 37,610 117,512 (+56) / 37,622 117,451 (-5) / 37,573 37,590 under cap (17 B)
page: compiled base server components 109,461 / 35,146 109,517 (+56) / 35,150 109,456 (-5) / 35,134 35,130 over by 4 B, within allowance (+10 B of 20)
page: compiled live server components 122,933 / 40,651 122,989 (+56) / 40,711 122,928 (-5) / 40,682 40,660 over by 22 B, within allowance (+10 B of 20)
page: base + router 144,238 / 45,996 144,294 (+56) / 46,080 144,233 (-5) / 46,009 46,020 under cap (11 B)
page: live + router 148,683 / 47,326 148,739 (+56) / 47,265 148,678 (-5) / 47,287 47,290 under cap (3 B)

The two compiled pages are over their brotli caps on brotli noise. They pass on the minified allowance (page: compiled base server components is already over on next, by 16 B). In CI (merged with a newer next) page: live + router measures 142,467 B minified and 11 B over its brotli cap, also within the allowance (5 B under its recorded minified). The size check passes with warnings only.

A server component that throws during render ships its error as the
sanitized message string. `landing()` told an error it had already
surfaced from the next flight's error by comparing payloads, so a
re-asked flight that failed the same way looked like the old error and
re-asked again: one request per flight with no end (about 3000/s in the
hackernews example after a hover preload of a failing route).

Key the surfaced error by the response version that carried it. Each
flight is a new version, so `reset()` and a fresh mount over an errored
address make one request and the error reaches the nearest `<Errored>`
again (or halts, with none).

Co-authored-by: Claude via Cursor <noreply@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
@changeset-bot

changeset-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 87c0068

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

This PR includes changesets to release 12 packages
Name Type
@solidjs/web Patch
@solidjs/babel-plugin Patch
@solidjs/diagnostics Patch
@solidjs/element Patch
@solidjs/h Patch
@solidjs/html Patch
test-integration Patch
todos-server-example Patch
@solidjs/compiler Patch
@solidjs/signals Patch
solid-js Patch
@solidjs/universal 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

@github-actions

github-actions Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Size (brotli, eager entry chunk)

scenario head vs base minified vs base minified vs recorded cap lazy chunks (not counted)
signals: core floor (createSignal/Memo/Effect/Root/flush) 7.43 KB 0 B 0 B +15 B 7.45 KB ✅
signals: + createStore 14.70 KB 0 B 0 B +10 B 14.70 KB ✅
signals: + isPending/latest 9.63 KB 0 B 0 B +15 B 9.65 KB ✅
app: render + one signal (the simple-app floor) 9.92 KB 0 B 0 B +15 B 9.93 KB ✅
app: hydrating (no stores) with Show/For/Loading/Errored/lazy 17.89 KB 0 B 0 B +15 B 17.91 KB ✅ lazy-page.js 0.04 KB
app: hydrating + every store primitive family 29.15 KB 0 B 0 B +65 B 29.19 KB ✅ lazy-page.js 0.04 KB
app: CSR with Show/For/Loading/Errored/lazy 12.92 KB 0 B 0 B +15 B 12.96 KB ✅ lazy-page.js 0.04 KB
app: CSR, observe tier (same app on the observe artifacts) 14.53 KB 0 B 0 B +15 B 14.53 KB ✅ lazy-page.js 0.04 KB
app: CSR, observe tier + attribution engine enabled 28.85 KB 0 B 0 B +15 B 28.89 KB ✅ lazy-page.js 0.04 KB
app: compiled floor (one template, one text hole, one delegated click) 10.12 KB 0 B 0 B +15 B 10.13 KB ✅
app: compiled CSR (JSX todo app: spread/merge/omit, events, class/style, keyed For, Show, Loading + lazy, store) 25.22 KB 0 B 0 B +65 B 25.24 KB ✅ stats.js 0.18 KB
app: compiled hydrating (the same JSX todo app through hydrate(), compiled hydratable) 31.21 KB 0 B 0 B +10 B 31.17 KB ⚠️ over by 43 B, 10 B minified headroom stats.js 0.20 KB
frames: eager client consumer (frames client + transport, lazy codec) 11.11 KB −1 B (−0.0%) −5 B −5 B 11.13 KB ✅
page: base server components (hydrating + dynamic + frames + sf reference) 33.90 KB −11 B (−0.0%) −5 B +156 B 33.92 KB ✅ assets.js 0.78 KB, bind.js 1.83 KB, decode.js 6.24 KB, lazy-page.js 0.04 KB, regions.js 0.80 KB, trace.js 8.21 KB, wire.js 0.93 KB
page: live server components (base + live/GET + action + isPending/latest) 37.57 KB −37 B (−0.1%) −5 B +10 B 37.59 KB ✅ assets.js 0.78 KB, bind.js 1.83 KB, decode.js 6.24 KB, lazy-page.js 0.04 KB, regions.js 0.80 KB, trace.js 8.20 KB, wire.js 0.93 KB
page: compiled base server components (the base page as JSX: templates with class/style/attributes/events, For/Show; no spread) 35.13 KB −12 B (−0.0%) −5 B +10 B 35.13 KB ⚠️ over by 4 B, 10 B minified headroom assets.js 0.78 KB, bind.js 1.83 KB, decode.js 6.24 KB, regions.js 0.80 KB, sc-comments.js 0.20 KB, trace.js 8.20 KB, wire.js 0.93 KB
page: compiled live server components (the compiled base page + live/GET + action + isPending/latest) 40.68 KB +31 B (+0.1%) −5 B +10 B 40.66 KB ⚠️ over by 22 B, 10 B minified headroom eager (counted): web.js 22.02 KB; assets.js 0.78 KB, bind.js 1.83 KB, decode.js 6.24 KB, regions.js 0.79 KB, sc-comments.js 0.19 KB, trace.js 8.21 KB, wire.js 0.93 KB
page: base + router (base page + @solidjs/router: createRouter, two routes, preload, useNavigate) 41.25 KB +7 B (+0.0%) −5 B −5 B 41.26 KB ✅ assets.js 0.78 KB, bind.js 1.85 KB, decode.js 6.24 KB, lazy-page.js 0.04 KB, regions.js 0.80 KB, server.js 1.02 KB, serverForms.js 3.58 KB, trace.js 8.23 KB, wire.js 0.94 KB
page: live + router (live page + @solidjs/router: createRouter, two routes, preload, useNavigate) 46.94 KB +22 B (+0.0%) −5 B −5 B 46.93 KB ⚠️ over by 11 B, 25 B minified headroom eager (counted): client.js 27.80 KB; assets.js 0.78 KB, bind.js 1.86 KB, decode.js 6.24 KB, lazy-page.js 0.04 KB, regions.js 0.81 KB, server.js 1.02 KB, serverForms.js 3.31 KB, trace.js 8.20 KB, wire.js 0.94 KB
server: floor (getRequestEvent + isServer) 1.33 KB 0 B 0 B 0 B 1.34 KB ✅
server: renderToString (the server-render floor) 20.40 KB 0 B 0 B +4 B 20.42 KB ✅

⚠️ Over the brotli cap within the minified allowance (passes)

  • app: compiled hydrating (the same JSX todo app through hydrate(), compiled hydratable): over brotli cap by 43 B; minified 99,682 B vs 99,672 B recorded with the cap (+10 B) — 10 B of the 20 B minified allowance left; +0 B minified over this PR's base
  • page: compiled base server components (the base page as JSX: templates with class/style/attributes/events, For/Show; no spread): over brotli cap by 4 B; minified 109,456 B vs 109,446 B recorded with the cap (+10 B) — 10 B of the 20 B minified allowance left; −5 B minified over this PR's base
  • page: compiled live server components (the compiled base page + live/GET + action + isPending/latest): over brotli cap by 22 B; minified 122,928 B vs 122,918 B recorded with the cap (+10 B) — 10 B of the 20 B minified allowance left; −5 B minified over this PR's base
  • page: live + router (live page + @solidjs/router: createRouter, two routes, preload, useNavigate): over brotli cap by 11 B; minified 142,467 B vs 142,472 B recorded with the cap (−5 B) — 25 B of the 20 B minified allowance left; −5 B minified over this PR's base

Bundled with Rolldown (what Vite ships), brotli q11, decimal KB. A scenario fails only when it is over its brotli cap and its minified size is more than 20 B over the minified recorded with the cap; over the cap within that allowance is brotli layout noise and passes with a warning. Caps and their recorded minified in scripts/size/scenarios.js; the floor and page caps in floor-caps.json are frozen (lower only, or Size-Exception: in the PR body). npm run ratchet lowers caps per RC; it never raises one (scripts/size/README.md).

@coveralls

coveralls commented Oct 8, 2026 •

Copy link
Copy Markdown

Coverage Report for CI Build 37806617613

Warning

Build has drifted: This PR's base is out of sync with its target branch, so coverage data may include unrelated changes.
Quick fix: rebase this PR. Learn more →

Coverage remained the same at 76.43%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 1227
Covered Lines: 996
Line Coverage: 81.17%
Relevant Branches: 958
Covered Branches: 674
Branch Coverage: 70.35%
Branches in Coverage %: Yes
Coverage Strength: 28.38 hits per line

💛 - Coveralls

@codspeed

codspeed Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 188 untouched benchmarks


Comparing fix/frames-reask-loop (be8c909) with next (893834c)

Open in CodSpeed

ryansolid and others added 2 commits October 8, 2026 08:48
Fold the frame read, the version key and the surfaced check into one
helper that returns false / 1 (already surfaced: re-ask) / 2 (new: throw),
and let reask() chain the next landing itself. Same behaviour as the
version-keyed fix; 5 B smaller minified than next instead of +56 B.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ypes

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