fix(signals): a memo over an error boundary's output no longer halts on a held rejection (#4640)

.changeset/errored-memo-over-output-held-reject.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+A memo over an error boundary's output no longer halts when the boundary's source rejects during a held change: the boundary shows its fallback, as it does when a render effect reads the output directly (#4640).

packages/signals/src/core/verdict.ts

@@ -722,18 +722,26 @@ function holdWork(n: Computed<any>, source: Computed<any>): unknown {
 function stageMark(node: Computed<any>, el: Computed<any>): void {
   if (
     el._config & CONFIG_STAGED_ERROR &&
-    !(
-      node._config &
-      (CONFIG_REDERIVE | CONFIG_STAGED_ERROR | CONFIG_ASYNC | (EFFECT_TRACKED << 23))
-    )
+    !(node._config & (CONFIG_REDERIVE | CONFIG_STAGED_ERROR | (EFFECT_TRACKED << 23)))
   ) {
-    node._pendingValue = el._pendingValue;
-    node._config |= CONFIG_STAGED_ERROR;
-    queuePendingNode(node);
-    // A derived store's readers are its family's (store/projection.ts): they
-    // re-derive from the staging, as from the derive's status (#4189).
-    (node as any)._fam?.st(STATUS_ERROR);
-    settlePendingSource(node);
+    // Only a node that reads a staging carries it. The walk also reaches the
+    // readers of a node it does not mark (a boundary's output, whose pass
+    // decides content or fallback): what they get comes through that node,
+    // not as the staging. An async memo over a staging is marked, so its
+    // readers stay pending until the commit (F-9, D-22, D-23).
+    // A memo over an error boundary's output is a plain reader of it (F-3:
+    // where the memo sits does not matter); marked, it threw the error past
+    // the boundary that caught it, and the commit halted (#4640).
+    for (let d = node._deps; d; d = d._nextDep)
+      if (d._dep._config & CONFIG_STAGED_ERROR) {
+        node._pendingValue = el._pendingValue;
+        node._config |= CONFIG_STAGED_ERROR;
+        queuePendingNode(node);
+        // A derived store's readers are its family's (store/projection.ts): they
+        // re-derive from the staging, as from the derive's status (#4189).
+        (node as any)._fam?.st(STATUS_ERROR);
+        return settlePendingSource(node);
+      }
   }
 }
 
@@ -744,7 +752,10 @@ function stageMark(node: Computed<any>, el: Computed<any>): void {
 function keepReaders(el: Computed<any>): void {
   for (let s = el._subs; s !== null; s = s._nextSub) {
     const r = s._sub;
-    if ((r._config & (CONFIG_HELD | CONFIG_STAGED)) !== (CONFIG_HELD | CONFIG_STAGED) || (r as any)._type)
+    if (
+      (r._config & (CONFIG_HELD | CONFIG_STAGED)) !== (CONFIG_HELD | CONFIG_STAGED) ||
+      (r as any)._type
+    )
       continue;
     let d = r._deps;
     while (d !== null && (d._dep === el || !(d._dep._config & CONFIG_HELD))) d = d._nextDep;

packages/signals/tests/errored-memo-over-output-held-reject-4640.test.ts

@@ -0,0 +1,97 @@
+/**
+ * #4640: an error boundary built outside (`untrack`) over an async memo;
+ * the boundary's output is read by a render effect, directly or through a
+ * memo, in the open or under a `<Loading>` without `on`. The first ask
+ * answers "A"; a write starts a held change; the second ask rejects.
+ *
+ * The rejection is the boundary's: it shows its fallback, and nothing halts
+ * (F-12, D-39). A memo is not a reader in its own right — it counts only
+ * through the render effect that reads it, wherever it was created
+ * (Vocabulary "Reader"; F-3: "where the memo was created does not matter"),
+ * so every shape shows the same frames. The memo over the output read the
+ * output, not the staged error (D-41: only "a derivation that reads a staged
+ * error throws it"), yet the settle walk from the rejected source marked it
+ * with the staging through the output it does not mark; the commit then
+ * published the error on the memo, past the boundary, and halted.
+ */
+import { expect, it } from "vitest";
+import {
+  createErrorBoundary,
+  createLoadingBoundary,
+  createMemo,
+  createRenderEffect,
+  createRoot,
+  createSignal,
+  flush,
+  untrack
+} from "../src/index.js";
+
+const tick = async () => {
+  for (let k = 0; k < 4; k++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+};
+
+it.each([
+  ["direct", "open"],
+  ["memo", "open"],
+  ["direct", "loading"],
+  ["memo", "loading"]
+] as const)(
+  "a held rejection under an error boundary built outside shows its fallback (%s, %s)",
+  async (between, place) => {
+    const asks: { res: (v: string) => void; rej: (e: unknown) => void }[] = [];
+    const frames: unknown[] = [];
+    const [sel, setSel] = createSignal(3);
+    const dispose = createRoot(d => {
+      const detail = createMemo(() => {
+        sel();
+        return new Promise<string>((res, rej) => asks.push({ res, rej }));
+      });
+      const shown = untrack(() =>
+        createErrorBoundary(
+          () => detail(),
+          () => "ERR"
+        )
+      );
+      const t = between === "memo" ? createMemo(() => shown()) : shown;
+      const view =
+        place === "loading"
+          ? untrack(() =>
+              createLoadingBoundary(
+                () => t(),
+                () => "fb"
+              )
+            )
+          : t;
+      createRenderEffect(
+        () => view(),
+        v => void frames.push(v)
+      );
+      return d;
+    });
+    // The first load: the root attach waits for it in the open (D-6); a
+    // fresh <Loading> shows its fallback (F-4).
+    const first = place === "loading" ? ["fb", "A"] : ["A"];
+    try {
+      await tick();
+      asks.shift()!.res("A");
+      await tick();
+      expect(frames).toEqual(first);
+
+      setSel(1);
+      flush();
+      await tick();
+      // Held: the screen keeps what it showed.
+      expect(frames).toEqual(first);
+
+      asks.shift()!.rej(new Error("boom"));
+      await tick();
+      // The boundary's fallback, no halt (an uncaught error fails the run).
+      expect(frames).toEqual([...first, "ERR"]);
+    } finally {
+      dispose();
+    }
+  }
+);

packages/signals/tests/staged-error-through-async-memo-4640.test.ts

@@ -0,0 +1,130 @@
+/**
+ * #4640 review: a plain memo `m` over an async memo `am` over an async source
+ * `s`. A write (or an action's write) re-asks `s` and a sibling flight on the
+ * same write keeps the change held after `s` rejects.
+ *
+ * `m` reads the staging through `am` (D-41: "a derivation that reads a
+ * staged error throws it"), so its verdict stays pending until the commit
+ * (F-9, D-22, D-23) and an untracked `latest(m)` in that window throws the
+ * staged error rather than returning the old value that the commit then
+ * replaces (#4456). The commit shows the error once: an effect's error arm
+ * over `m` fires a single time.
+ *
+ * Narrowing the staging to readers of a staging (#4640) must not drop the
+ * async memo from the marking: unmarked, `m` reported final and `latest(m)`
+ * returned "A!" in the window.
+ */
+import { expect, it } from "vitest";
+import {
+  action,
+  createEffect,
+  createErrorBoundary,
+  createMemo,
+  createRenderEffect,
+  createRoot,
+  createSignal,
+  flush,
+  isPending,
+  latest
+} from "../src/index.js";
+
+const tick = async () => {
+  for (let k = 0; k < 4; k++) {
+    await new Promise(r => setTimeout(r, 0));
+    flush();
+  }
+};
+
+it.each(["write", "action"] as const)(
+  "a memo over an async memo over a rejected source stays pending until the commit (%s)",
+  async how => {
+    const asks: { res: (v: string) => void; rej: (e: unknown) => void }[] = [];
+    const keeps: ((v: string) => void)[] = [];
+    const frames: string[] = [];
+    const errors: string[] = [];
+    const scr = { m: "-", p: "-" };
+    const [sel, setSel] = createSignal(1);
+    let m!: () => string;
+    createRoot(() => {
+      const s = createMemo(() => {
+        sel();
+        return new Promise<string>((res, rej) => asks.push({ res, rej }));
+      });
+      const am = createMemo(async () => {
+        const v = s();
+        await 0;
+        return v;
+      });
+      m = createMemo(() => am() + "!");
+      const keep = createMemo(() => {
+        sel();
+        return new Promise<string>(res => keeps.push(res));
+      });
+      const eb = <T>(f: () => T) => createErrorBoundary(f, () => "ERR");
+      const vm = eb(() => m());
+      const vp = eb(() => String(isPending(m)));
+      const vk = eb(() => keep());
+      createRenderEffect(
+        () => vm(),
+        v => void (scr.m = v)
+      );
+      createRenderEffect(
+        () => vp(),
+        v => void (scr.p = v)
+      );
+      createRenderEffect(
+        () => vk(),
+        () => {}
+      );
+      createEffect(m, {
+        effect: () => {},
+        error: e => void errors.push((e as Error).message)
+      });
+    });
+    const frame = (l: string) => frames.push(`${l}[${scr.m} pending=${scr.p}]`);
+
+    await tick();
+    asks.shift()!.res("A");
+    keeps.shift()!("k");
+    await tick();
+    frame("shown");
+
+    let release: (() => void) | undefined;
+    if (how === "action")
+      action(function* () {
+        setSel(2);
+        yield new Promise<void>(r => (release = r));
+      })();
+    else setSel(2);
+    flush();
+    await tick();
+    frame("held");
+
+    asks.shift()!.rej(new Error("boom"));
+    await tick();
+    frame("rejected");
+    let mid: string;
+    try {
+      mid = latest(m);
+    } catch (e) {
+      mid = "throws " + ((e as any)?.cause?.message ?? (e as Error).message);
+    }
+
+    keeps.shift()!("k2");
+    await tick();
+    if (release) {
+      release();
+      await tick();
+    }
+    frame("committed");
+
+    expect(frames).toEqual([
+      "shown[A! pending=false]",
+      "held[A! pending=true]",
+      "rejected[A! pending=true]",
+      "committed[ERR pending=false]"
+    ]);
+    expect(mid).toBe("throws boom");
+    expect(errors).toEqual(["boom"]);
+  }
+);