fix(signals): route async protocol throws and error-formatting failures to the memo (#4112)

.changeset/async-protocol-throw-4112.md

@@ -0,0 +1,5 @@
+---
+"@solidjs/signals": patch
+---
+
+Route synchronous throws from the async iterator / thenable protocol (a later `next()`, a step's `then`, or deferred iterator setup) to the owning memo's error handling instead of escaping as unhandled rejections, and make `StatusError` message formatting best-effort so it never replaces the original error (#4112).

packages/signals/src/core/async.ts

@@ -41,7 +41,7 @@ import {
   freshHook,
   laneStageHook
 } from "./scheduler.js";
-import type { Computed, Link } from "./types.js";
+import type { Computed, Link, Owner } from "./types.js";
 
 // The lazily-created Set is the ONE container for pending sources. Its
 // predecessor — a singular slot promoted to a Set on the second source —
@@ -578,18 +578,14 @@ export function handleAsync<T>(
   // close registration uses the ambient owner; and DEFERRED (the flattening
   // path — a thenable resolved to an iterable in a later microtask), where
   // there is no caller to serve and no ambient owner: sync-settled steps
-  // write through asyncWrite, and close registration goes through the slot
-  // the thenable branch pre-registered while it still owned the context.
-  // Returns whether a sync answer landed (first yield or empty completion) —
-  // meaningful only in the live posture.
-  const consumeIterator = (
-    source: AsyncIterable<T>,
-    registerClose?: (fn: () => void) => void
-  ): boolean => {
+  // write through asyncWrite, and close registration targets `owner` (el
+  // itself) directly. Returns whether a sync answer landed (first yield or
+  // empty completion) — meaningful only in the live posture.
+  const consumeIterator = (source: AsyncIterable<T>, owner?: Owner): boolean => {
     const it = source[Symbol.asyncIterator]();
     let hadValue = false;
     let completed = false;
-    let initialRead = !registerClose;
+    let initialRead = !owner;
 
     const close = () => {
       if (completed) return;
@@ -599,7 +595,7 @@ export function handleAsync<T>(
         if (isThenable(returned)) returned.then(undefined, () => {});
       } catch {}
     };
-    registerClose ? registerClose(close) : cleanup(close);
+    cleanup(close, owner);
     // Flight-identity cancellation (#3122): the registration above is the
     // owner-death backstop; the teardown slot fires at the _inFlight release
     // sites so supersede stops this stream immediately.
@@ -608,8 +604,15 @@ export function handleAsync<T>(
     // Release check before each next pull: an unobserved lazy node must tear
     // down (its close above runs via disposal, closing the iterator) instead
     // of pumping the stream forever with zero subscribers (#2935).
+    // Later pulls run from an async continuation with no compute catch above
+    // them: a throw invoking the protocol (next(), the step's then) settles
+    // as this flight's error instead of escaping as a rejection (#4112).
     const iterateOrRelease = () => {
-      if (!settleAutodispose()) iterate();
+      try {
+        if (!settleAutodispose()) iterate();
+      } catch (e) {
+        handleError(e);
+      }
     };
 
     const iterate = (): boolean => {
@@ -700,7 +703,7 @@ export function handleAsync<T>(
   // stream itself rather than settling on the iterable object. One level
   // only: A+ `then` already collapses nested thenables, so the resolved
   // value is never itself a thenable.
-  const flattenIfIterable = (value: any, registerClose?: (fn: () => void) => void): boolean => {
+  const flattenIfIterable = (value: any, owner?: Owner): boolean => {
     let innerIterator: any = false;
     if (typeof value === "object" && value !== null) {
       untrack(() => {
@@ -709,8 +712,8 @@ export function handleAsync<T>(
     }
     if (!innerIterator) return false;
     syncValue = undefined as T;
-    const landed = consumeIterator(value as AsyncIterable<T>, registerClose);
-    if (!registerClose) liveLanded = landed;
+    const landed = consumeIterator(value as AsyncIterable<T>, owner);
+    if (!owner) liveLanded = landed;
     return true;
   };
 
@@ -719,34 +722,32 @@ export function handleAsync<T>(
       rejected = false,
       syncError: any,
       isSync = true;
-    // Close registration for the flattening path. Consumption starts in a
-    // microtask where the ambient owner is gone (or worse, someone else's),
-    // so `cleanup()` can't be used — the close targets el's disposal list
-    // directly, exactly where a live cleanup() during this recompute would
-    // have put it. Only a flight that actually flattens becomes
-    // disposal-bearing.
-    const registerDeferredClose = (fn: () => void) => {
-      if (!el._disposal) el._disposal = fn;
-      else if (Array.isArray(el._disposal)) el._disposal.push(fn);
-      else el._disposal = [el._disposal, fn];
-    };
     (__DEV__ ? watchAsyncTail(el, result as PromiseLike<T>) : (result as PromiseLike<T>)).then(
       v => {
         if (isSync) {
           syncValue = v;
           resolved = true;
-        } else if (
-          el._x?._inFlight === result &&
-          !(el._flags & REACTIVE_DISPOSED) &&
-          flattenIfIterable(v, registerDeferredClose)
-        ) {
-          // Flattened: the stream is the value. Each yield lands through
-          // asyncWrite under this flight's identity; the first one clears
-          // pending exactly like a plain promise resolution would have.
-          // (Disposed nodes never start a pump — their disposal list has
-          // already run, so nothing could ever close the iterator.)
         } else {
-          asyncWrite(v);
+          // Flattening consumes the stream as the value: each yield lands
+          // through asyncWrite under this flight's identity; the first one
+          // clears pending exactly like a plain promise resolution would.
+          // Disposed nodes never start a pump (their disposal list already
+          // ran, so nothing could close the iterator). Consumption starts in
+          // a microtask where the ambient owner is gone, so the close
+          // registers on el's own disposal list. Iterator setup here has no
+          // compute catch above it: a protocol throw settles as this
+          // flight's error (#4112).
+          try {
+            if (
+              el._x?._inFlight === result &&
+              !(el._flags & REACTIVE_DISPOSED) &&
+              flattenIfIterable(v, el)
+            )
+              return;
+            asyncWrite(v);
+          } catch (e) {
+            handleError(e);
+          }
           settleAutodispose();
         }
       },

packages/signals/src/core/error.ts

@@ -51,9 +51,13 @@ export class StatusError extends Error {
     public source: any,
     original: any
   ) {
-    super(original instanceof Error ? original.message : String(original), {
-      cause: original
-    });
+    // Best-effort message: formatting must never replace the original (#4112).
+    try {
+      var message: string | undefined = String(
+        original instanceof Error ? original.message : original
+      );
+    } catch {}
+    super(message, { cause: original });
   }
 }
 

packages/signals/src/core/owner.ts

@@ -301,13 +301,15 @@ export function getOwner(): Owner | null {
 /**
  * Low-level: registers `fn` as a disposal callback on the current owner.
  * Most code should use `onCleanup()` from `solid-js`, which adds dev-mode
- * checks. `cleanup()` is the unchecked primitive used by internals.
+ * checks. `cleanup()` is the unchecked primitive used by internals; `owner`
+ * (internal) targets a specific owner's disposal list instead of the ambient
+ * one.
  */
-export function cleanup(fn: Disposable): Disposable {
-  if (!context) return fn;
-  if (!context._disposal) context._disposal = fn;
-  else if (Array.isArray(context._disposal)) context._disposal.push(fn);
-  else context._disposal = [context._disposal, fn];
+export function cleanup(fn: Disposable, owner: Owner | null = context): Disposable {
+  if (!owner) return fn;
+  if (!owner._disposal) owner._disposal = fn;
+  else if (Array.isArray(owner._disposal)) owner._disposal.push(fn);
+  else owner._disposal = [owner._disposal, fn];
   return fn;
 }
 

packages/signals/tests/async-protocol-throw-4112.test.ts

@@ -0,0 +1,165 @@
+import {
+  createErrorBoundary,
+  createMemo,
+  createRenderEffect,
+  createRoot,
+  flush
+} from "../src/index.js";
+
+// #4112: throws raised while Solid invokes the async-iterator / thenable
+// protocol (not rejections delivered through it) must reach the owning memo's
+// error handling, and error formatting must never replace the original error.
+
+afterEach(() => flush());
+
+const tick = () => new Promise<void>(r => setTimeout(r, 0));
+const original = (e: any) => e?.cause ?? e;
+
+describe("#4112 async protocol throws reach the memo", () => {
+  let unhandled: unknown[];
+  const onUnhandled = (e: unknown) => void unhandled.push(e);
+  beforeEach(() => {
+    unhandled = [];
+    process.on("unhandledRejection", onUnhandled);
+  });
+  afterEach(() => void process.off("unhandledRejection", onUnhandled));
+
+  // Yields 1 after a timer, then defers to `second` for the next pull.
+  function iterableWith(second: () => any): AsyncIterable<number> {
+    return {
+      [Symbol.asyncIterator]() {
+        let n = 0;
+        return {
+          next() {
+            if (n++ === 0)
+              return new Promise<IteratorResult<number>>(r =>
+                setTimeout(() => r({ done: false, value: 1 }))
+              );
+            return second();
+          }
+        } as AsyncIterator<number>;
+      }
+    };
+  }
+
+  // Mirrors <Errored>{memo()}</Errored>: the boundary's fallback is where a
+  // protocol failure must surface.
+  async function run(source: () => any) {
+    const shown: unknown[] = [];
+    createRoot(() => {
+      const memo = createMemo(source as () => number);
+      const b = createErrorBoundary(
+        () => memo(),
+        err => original(err())
+      );
+      createRenderEffect(
+        () => {
+          try {
+            return b();
+          } catch {
+            return "loading";
+          }
+        },
+        v => void shown.push(v)
+      );
+    });
+    flush();
+    for (let i = 0; i < 4; i++) {
+      await tick();
+      flush();
+    }
+    return shown;
+  }
+
+  const last = (shown: unknown[]) => (shown[shown.length - 1] as any)?.message;
+
+  it("control: later next() rejecting reaches the boundary", async () => {
+    const shown = await run(() => iterableWith(() => Promise.reject(new Error("rejected"))));
+    expect(last(shown)).toBe("rejected");
+    expect(unhandled).toEqual([]);
+  });
+
+  it("later next() throwing synchronously reaches the boundary", async () => {
+    const shown = await run(() =>
+      iterableWith(() => {
+        throw new Error("next failed");
+      })
+    );
+    expect(last(shown)).toBe("next failed");
+    expect(unhandled).toEqual([]);
+  });
+
+  it("next step's then() throwing reaches the boundary", async () => {
+    const shown = await run(() =>
+      iterableWith(() => ({
+        then() {
+          throw new Error("then failed");
+        }
+      }))
+    );
+    expect(last(shown)).toBe("then failed");
+    expect(unhandled).toEqual([]);
+  });
+
+  it("deferred iterator factory throwing reaches the boundary", async () => {
+    const shown = await run(() =>
+      new Promise(r => setTimeout(r)).then(() => ({
+        [Symbol.asyncIterator]() {
+          throw new Error("iterator failed");
+        }
+      }))
+    );
+    expect(last(shown)).toBe("iterator failed");
+    expect(unhandled).toEqual([]);
+  });
+
+  it("deferred first next() throwing reaches the boundary", async () => {
+    const shown = await run(() =>
+      new Promise(r => setTimeout(r)).then(() => ({
+        [Symbol.asyncIterator]() {
+          return {
+            next() {
+              throw new Error("first next failed");
+            }
+          };
+        }
+      }))
+    );
+    expect(last(shown)).toBe("first next failed");
+    expect(unhandled).toEqual([]);
+  });
+});
+
+describe("#4112 error formatting keeps the original error", () => {
+  function create(thrown: unknown) {
+    let memo!: () => unknown;
+    expect(() =>
+      createRoot(() => {
+        memo = createMemo(() => {
+          throw thrown;
+        });
+      })
+    ).not.toThrow();
+    let caught: unknown;
+    try {
+      memo();
+    } catch (e) {
+      caught = e;
+    }
+    expect(original(caught)).toBe(thrown);
+  }
+
+  it("unstringifiable thrown value does not escape createMemo", () => {
+    create(Object.create(null));
+  });
+
+  it("throwing message getter does not escape createMemo", () => {
+    const err = new Error("x");
+    Object.defineProperty(err, "message", {
+      get() {
+        throw new Error("getter");
+      }
+    });
+    create(err);
+  });
+});