fix(signals): an effect's error boundary keeps no superseded staged error (#4527)

.changeset/error-boundary-superseded-staged-error-4527.md

@@ -0,0 +1,7 @@
+---
+"@solidjs/signals": patch
+---
+
+An error boundary no longer hands its reader an error that was superseded inside the same transition (#4527). Here is the case: an action keeps its transition open, the store's re-ask is rejected (that rejection is staged, D-41), and a second re-ask answers before the commit. An effect whose compute created `createErrorBoundary(() => store, …)` used to get the fallback at the commit and never ran again. Now it gets the answer, as the screen does.
+
+Two changes make that work. First, a pass that a derived store's pull throws a staged error into is now linked to the store's derive, including the untracked shape probe of a returned proxy. That way the re-ask and the answer that supersede the error reach the memo. Second, an error boundary no longer waits for first content: one that has not shown content reveals a recovery that lands in the hold it caught the error in together with that hold's commit, instead of keeping its fallback past it.

packages/signals/src/boundaries.ts

@@ -363,7 +363,11 @@ export function prune(b: Boundary, pass: boolean): number {
         // lane's reader reveals with the lane, this round.) A boundary that
         // has not shown content catches it instead: the fallback stays until
         // the reader is committed (#3540: what a fresh boundary catches holds
-        // nothing outside it).
+        // nothing outside it). Not an error boundary (born `_initialized`):
+        // what it caught in a hold was that hold's staged error (D-41), and an
+        // answer landing in the same hold replaces it there — the content
+        // reveals with the commit, and nothing ever shows the superseded
+        // error (D-39, #4527).
         if (!pass || !b._initialized) continue;
         joinFuture(txOf(s));
       }
@@ -612,7 +616,8 @@ function createBoundary<T>(
     _tree: null!,
     _output: null,
     _readers: new Map(),
-    _initialized: false,
+    // Loading only: an error boundary has no first-content wait (`prune`).
+    _initialized: type === STATUS_ERROR,
     _armed: false,
     _armLane: null,
     _lane: null,

packages/signals/src/store/store.ts

@@ -1885,6 +1885,11 @@ function pullFamily(target: StoreTarget): void {
     // flight the family has — not over a new question they do not answer
     // (optimistic.ts `stands`, #4336).
     if (verdict === null && fw._statusFlags & STATUS_PENDING && optHooks?.stands(target)) return;
+    // A pass the pull throws a staged error (D-41) is linked to the derive,
+    // untracked too — `handleAsync` asking a returned proxy whether it is a
+    // flight: the pass carries the error, and the re-ask or answer that
+    // supersedes it must reach it (#4527; a tracked read links anyway, #4519).
+    if (c && fw._config & CONFIG_STAGED_ERROR) link(fw, c);
     readNode(fw);
   } else {
     untrack(() => readNode(fw));

packages/signals/tests/error-boundary-superseded-staged-error-4527.test.ts

@@ -0,0 +1,130 @@
+/**
+ * #4527: an action re-asks a derived store and a memo, so its transition
+ * stays open. The store's re-ask rejects (staged, D-41), then a second
+ * re-ask answers before the transition commits. An effect whose compute
+ * creates an error boundary over the bare store proxy got the superseded
+ * error at the commit (the boundary's fallback, `null`) and never ran again.
+ *
+ * Two holes, both D-41 / D-39: the memo returning the proxy was thrown the
+ * staged error by `handleAsync`'s shape probe (an untracked read that pulls
+ * the derive) without a link to the derive, so neither the second re-ask nor
+ * its answer reached it; and an error boundary that never showed content
+ * kept its fallback over a reader whose replacement had landed in the same
+ * hold, so the commit showed an error nothing on screen carried.
+ */
+import { expect, it } from "vitest";
+import {
+  action,
+  createEffect,
+  createErrorBoundary,
+  createLoadingBoundary,
+  createMemo,
+  createOptimisticStore,
+  createRenderEffect,
+  createRoot,
+  createStore,
+  flush,
+  refresh,
+  untrack
+} from "../src/index.js";
+
+const tick = async () => {
+  for (let i = 0; i < 4; i++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+};
+
+for (const kind of ["optimistic", "store"] as const)
+  for (const effect of ["effect", "render"] as const)
+    it(`#4527 ${kind} ${effect}: the commit shows the answer, not the superseded error`, async () => {
+      const asks: { key: string; res: (v: any) => void; rej: (e: any) => void }[] = [];
+      const ask = (key: string) => new Promise<any>((res, rej) => asks.push({ key, res, rej }));
+      const settle = async (key: string, f: (a: (typeof asks)[number]) => void) => {
+        f(
+          asks.splice(
+            asks.findIndex(a => a.key === key),
+            1
+          )[0]
+        );
+        await tick();
+      };
+      const seen: string[] = [];
+      const frames: string[] = [];
+      let screen = "";
+      const snap = (label: string) => {
+        frames.push(`${label}: ${screen} | ${seen.join(",")}`);
+        seen.length = 0;
+      };
+      let again!: () => void;
+      let rename!: (t: string) => any;
+      const dispose = createRoot(d => {
+        const [issues] =
+          kind === "store"
+            ? createStore<any[]>(() => ask("issues"), [])
+            : createOptimisticStore<any[]>(() => ask("issues"), [], { key: "id" });
+        again = () => refresh(issues);
+        const detail = createMemo(() => ask("detail"));
+        rename = action(function* (t: string) {
+          yield ask(`rename:${t}`);
+          refresh(issues);
+          refresh(detail);
+        });
+        const compute = () =>
+          untrack(() =>
+            createErrorBoundary(
+              () => issues,
+              () => null
+            )
+          )();
+        const out = (v: any) => void seen.push(v === null ? "null" : `len${v.length}`);
+        if (effect === "render") createRenderEffect(compute, out);
+        else
+          createEffect(compute, {
+            effect: out,
+            error: (e: any) => void seen.push(`error ${e?.message}`)
+          } as any);
+        const list = createLoadingBoundary(
+          createErrorBoundary(
+            () => issues.map((r: any) => r.title).join(","),
+            (e: any) => `err ${e().message}`
+          ),
+          () => "loading"
+        );
+        const info = createLoadingBoundary(
+          () => String(detail()),
+          () => "…"
+        );
+        createRenderEffect(
+          () => `${list()} / ${info()}`,
+          v => void (screen = v)
+        );
+        return d;
+      });
+      await tick();
+      const rows = (t: string) => [
+        { id: 1, title: "Login" },
+        { id: 3, title: t },
+        { id: 4, title: "Typo" }
+      ];
+      await settle("issues", a => a.res(rows("Crash")));
+      await settle("detail", a => a.res("d1"));
+      seen.length = 0;
+      rename("Alpha");
+      await tick();
+      await settle("rename:Alpha", a => a.res({}));
+      await settle("issues", a => a.rej(new Error("declined")));
+      snap("re-ask failed");
+      again();
+      await tick();
+      await settle("issues", a => a.res(rows("Alpha").filter(r => r.id !== 4)));
+      await settle("detail", a => a.res("d2"));
+      snap("committed");
+      expect(frames).toEqual([
+        // D-41: the rejection is staged with the held frame — nothing shows it.
+        "re-ask failed: Login,Crash,Typo / d1 | ",
+        // The second answer superseded it before the commit: nothing sees it.
+        "committed: Login,Alpha / d2 | len2"
+      ]);
+      dispose();
+    });

packages/web/test/error-boundary-superseded-error-4527.spec.tsx

@@ -0,0 +1,124 @@
+/**
+ * @jsxImportSource @solidjs/web
+ * @vitest-environment jsdom
+ */
+// #4527: an action re-asks a store and a memo, so its transition stays open.
+// The store's re-ask fails (staged, D-41), then a second re-ask answers
+// before the transition commits. An effect reading the store through an
+// error boundary created in its compute got the superseded failure (the
+// fallback's `null`) at the commit and never ran again; the screen showed
+// the new rows. Nothing may see the superseded failure: the effect gets
+// `len2` with the screen.
+import { expect, test } from "vitest";
+import {
+  For,
+  Loading,
+  Errored,
+  action,
+  createOptimisticStore,
+  createMemo,
+  createEffect,
+  untrack,
+  flush,
+  refresh
+} from "solid-js";
+import { createErrorBoundary } from "solid-js/internal";
+import { render } from "../src/index.js";
+
+const tick = async () => {
+  for (let i = 0; i < 4; i++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+};
+
+test("#4527: an effect's error boundary gets the answer, not the superseded error", async () => {
+  const asks: { key: string; res: (v: any) => void; rej: (e: any) => void }[] = [];
+  const ask = (key: string) => new Promise<any>((res, rej) => asks.push({ key, res, rej }));
+  const settle = async (key: string, f: (a: (typeof asks)[number]) => void) => {
+    f(
+      asks.splice(
+        asks.findIndex(a => a.key === key),
+        1
+      )[0]
+    );
+    await tick();
+  };
+  const container = document.body.appendChild(document.createElement("div"));
+  const log: string[] = [];
+  const frames: string[] = [];
+  const snap = (label: string) => {
+    frames.push(`${label}: ${container.textContent} | ${log.join(",")}`);
+    log.length = 0;
+  };
+  let again!: () => void;
+  let rename!: (t: string) => any;
+  const dispose = render(() => {
+    const [issues] = createOptimisticStore<any[]>(() => ask("issues"), [], { key: "id" });
+    again = () => refresh(issues);
+    const detail = createMemo(() => ask("detail"));
+    rename = action(function* (t: string) {
+      yield ask(`rename:${t}`);
+      refresh(issues);
+      refresh(detail);
+    });
+    createEffect(
+      () =>
+        untrack(() =>
+          createErrorBoundary(
+            () => issues,
+            () => null
+          )
+        )(),
+      {
+        effect: (v: any) => void log.push(v === null ? "null" : `len${v.length}`),
+        error: (e: any) => void log.push(`error ${e?.message}`)
+      } as any
+    );
+    return (
+      <>
+        <Loading fallback="loading">
+          <Errored fallback={(e: any) => `list error ${e().message}`}>
+            <For each={issues}>
+              {(it: any) => (
+                <i>
+                  [#{it.id} {it.title}]
+                </i>
+              )}
+            </For>
+          </Errored>
+        </Loading>
+        {" | "}
+        <Loading fallback="…">{String(detail())}</Loading>
+      </>
+    );
+  }, container);
+  await tick();
+  const rows = (t: string) => [
+    { id: 1, title: "Login" },
+    { id: 3, title: t },
+    { id: 4, title: "Typo" }
+  ];
+  await settle("issues", a => a.res(rows("Crash")));
+  await settle("detail", a => a.res("d1"));
+  snap("loaded");
+  rename("Alpha");
+  await tick();
+  await settle("rename:Alpha", a => a.res({}));
+  await settle("issues", a => a.rej(new Error("declined")));
+  snap("re-ask failed");
+  again();
+  await tick();
+  await settle("issues", a => a.res(rows("Alpha").filter(r => r.id !== 4)));
+  snap("re-ask answered");
+  await settle("detail", a => a.res("d2"));
+  snap("detail answered");
+  expect(frames).toEqual([
+    "loaded: [#1 Login][#3 Crash][#4 Typo] | d1 | len3",
+    "re-ask failed: [#1 Login][#3 Crash][#4 Typo] | d1 | ",
+    "re-ask answered: [#1 Login][#3 Crash][#4 Typo] | d1 | ",
+    "detail answered: [#1 Login][#3 Alpha] | d2 | len2"
+  ]);
+  dispose();
+  container.remove();
+});