fix: bubble errors from recovering class boundaries

.changeset/error-boundary-recovery.md

@@ -0,0 +1,5 @@
+---
+"@tanstack/redact": patch
+---
+
+Bubble errors thrown during boundary recovery to an enclosing boundary and preserve hydrated Suspense marker placement.

packages/redact/src/dom/features/class/full.ts

@@ -1,6 +1,6 @@
 import { devToolsMetadata } from '../../../core/devtools';
 import { resolveClassProps } from '../../../core/class-props';
-import { invokeCommit, stageCommit } from '../../reconcile';
+import { invokeCommit, stageCommit, recoveringBoundaries } from '../../reconcile';
 import { installRenderer, reconcileRaw, childrenToArray, scheduleUpdate, scheduleLifecycle, isThenable, handleSuspended, handleErrorInRender, readContext, attachRef, syncRefIfChanged, isActivityHidden, layoutIsDisconnected, RenderErrorCapture, handleCommitError, } from '../../reconcile';
 import { checkpointCommit, rewindCommit, queueBeforeMutation, queueMutation } from '../../commit';
 import { FiberTag, type Fiber, type ReactNode } from '../../../core';
@@ -98,38 +98,47 @@ function renderClass(fiber: Fiber): void {
         }
         instance.props = props;
     }
-    if (captured && Ctor.getDerivedStateFromError) {
-        const update = Ctor.getDerivedStateFromError(captured.e);
-        if (update != null)
-            instance.state = { ...instance.state, ...update };
-    }
-    let rendered: ReactNode = captured && !Ctor.getDerivedStateFromError ? null : didRender ? instance.render() : fiber.b?.r;
-    fiber.b = { ...(fiber.b ?? {}), r: rendered };
-    const canCatch = Ctor.getDerivedStateFromError || instance.componentDidCatch;
-    if (canCatch)
-        fiber.b.b = Ctor;
-    if (canCatch && fiber.r)
-        fiber.r.eb = true;
-    const checkpoint = canCatch ? checkpointCommit(fiber) : -1;
+    // Errors from a recovery tree belong to an enclosing boundary. A nested
+    // boundary introduced by that recovery remains eligible to catch them.
+    if (captured) recoveringBoundaries.add(fiber);
     try {
-        reconcileRaw(fiber, rendered);
-    }
-    catch (error) {
-        if (!(error instanceof RenderErrorCapture) || error.b !== fiber)
-            throw error;
-        rewindCommit(checkpoint);
-        captured = error;
-        didRender = true;
-        if (Ctor.getDerivedStateFromError) {
-            const update = Ctor.getDerivedStateFromError(error.e);
+        if (captured && Ctor.getDerivedStateFromError) {
+            const update = Ctor.getDerivedStateFromError(captured.e);
             if (update != null)
                 instance.state = { ...instance.state, ...update };
-            rendered = instance.render();
         }
-        else
-            rendered = null;
+        let rendered: ReactNode = captured && !Ctor.getDerivedStateFromError ? null : didRender ? instance.render() : fiber.b?.r;
         fiber.b = { ...(fiber.b ?? {}), r: rendered };
-        reconcileRaw(fiber, rendered);
+        const canCatch = Ctor.getDerivedStateFromError || instance.componentDidCatch;
+        if (canCatch)
+            fiber.b.b = Ctor;
+        if (canCatch && fiber.r)
+            fiber.r.eb = true;
+        const checkpoint = canCatch ? checkpointCommit(fiber) : -1;
+        try {
+            reconcileRaw(fiber, rendered);
+        }
+        catch (error) {
+            if (!(error instanceof RenderErrorCapture) || error.b !== fiber)
+                throw error;
+            rewindCommit(checkpoint);
+            captured = error;
+            recoveringBoundaries.add(fiber);
+            didRender = true;
+            if (Ctor.getDerivedStateFromError) {
+                const update = Ctor.getDerivedStateFromError(error.e);
+                if (update != null)
+                    instance.state = { ...instance.state, ...update };
+                rendered = instance.render();
+            }
+            else
+                rendered = null;
+            fiber.b = { ...(fiber.b ?? {}), r: rendered };
+            reconcileRaw(fiber, rendered);
+        }
+    }
+    finally {
+        recoveringBoundaries.delete(fiber);
     }
     fiber.m = props;
     const state = instance.state;

packages/redact/src/dom/features/suspense/full.ts

@@ -96,7 +96,7 @@ function recoverBoundaryHydration(fiber: Fiber, children: any, parent: Node, sta
     root.h = false;
     try {
         if (retryBoundary) {
-            fiber.b = {};
+            fiber.b = { b: startMark, e: endMark };
             renderSuspense(fiber);
         }
         else

packages/redact/src/dom/reconcile.ts

@@ -79,13 +79,17 @@ function collectHostDoms(fiber: Fiber, out: Node[]): void {
     }
     if (fiber.t === FiberTag.Portal)
         return;
+    if (fiber.t === FiberTag.Suspense && fiber.b?.b)
+        out.push(fiber.b.b);
     let c = fiber.c;
     while (c) {
         collectHostDoms(c, out);
         c = c.s;
     }
     if (fiber.t === FiberTag.Suspense && fiber.b?.f)
         collectHostDoms(fiber.b.f, out);
+    if (fiber.t === FiberTag.Suspense && fiber.b?.e)
+        out.push(fiber.b.e);
 }
 export function stageCommit(fiber: Fiber, work: Effect | (() => void), ref = false): void {
     let entries = stagedCommits.get(fiber);
@@ -760,9 +764,10 @@ export function reconcileChildren(parent: Fiber, nodes: Parameters<typeof reconc
 export function installRenderer(tag: FiberTag, fn: RenderFn): void {
     RENDERERS[tag] = fn;
 }
+export const recoveringBoundaries = new Set<Fiber>();
 function findErrorBoundary(fiber: Fiber): Fiber | null {
     for (let parent = fiber.p; parent; parent = parent.p) {
-        if (parent.u || parent.pd)
+        if (parent.u || parent.pd || recoveringBoundaries.has(parent))
             continue;
         const instance = parent.i;
         if (instance?._f === parent && (parent.b?.b?.getDerivedStateFromError || instance.componentDidCatch))
@@ -1220,6 +1225,10 @@ export function removeFiber(fiber: Fiber, remove = true): void {
     if (fiber.t === FiberTag.Suspense && fiber.b?.b) {
         const state = fiber.b;
         state.b._reactRetry = undefined;
+        if (remove && !state.w) {
+            state.b.remove();
+            state.e?.remove();
+        }
         if (remove && state.w) {
             let node: Node | null = state.b;
             while (node) {

scripts/size-check.mjs

@@ -63,11 +63,16 @@ const STORE_HYDRATION_SIZE_ALLOWANCE = {
   'redact/dom-client (hydration=stub)': [3, 0],
 }
 
+// Measured error recovery and hydrated marker ownership cost, [source, built].
+const ERROR_RECOVERY_SIZE_ALLOWANCE = {
+  'redact/dom-client (hydration=stub)': [72, 81],
+}
+
 let failed = false
 for (const tree of ['src', 'dist']) {
   for (const row of await measureSizes(tree)) {
     const baseBudget = BUDGETS[row.name]
-    const budget = baseBudget === undefined ? undefined : baseBudget + (STORE_HYDRATION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (ROOT_TRANSITION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (QUEUE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0)
+    const budget = baseBudget === undefined ? undefined : baseBudget + (ERROR_RECOVERY_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (STORE_HYDRATION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (ROOT_TRANSITION_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0) + (QUEUE_SIZE_ALLOWANCE[row.name]?.[tree === 'src' ? 0 : 1] ?? 0)
     const ok = budget !== undefined && row.gz <= budget
     failed ||= !ok
     console.log(`${ok ? 'PASS' : 'FAIL'} ${tree} ${row.name}: ${row.gz} B gzip, budget ${budget ?? 'missing'}`)

tests/error-boundary-recovery.test.tsx

@@ -0,0 +1,83 @@
+import { afterEach, expect, it, vi } from 'vitest'
+import { Component, Suspense, useState, type ReactNode } from 'react'
+import { createRoot, hydrateRoot } from 'react-dom/client'
+import { flushSync } from 'react-dom'
+
+const cleanup: Array<() => void> = []
+afterEach(() => { for (const dispose of cleanup.splice(0)) dispose() })
+const original = new Error('loader failure'), fallbackError = new Error('fallback failure')
+class Boundary extends Component<{ children?: ReactNode, fallback: (error: Error) => ReactNode, caught?: (error: Error) => void }, { error: Error | null }> {
+  state = { error: null as Error | null }
+  static getDerivedStateFromError(error: Error) { return { error } }
+  componentDidCatch(error: Error) { this.props.caught?.(error) }
+  render() { return this.state.error ? this.props.fallback(this.state.error) : this.props.children }
+}
+function ErrorPage() { const [count, setCount] = useState(0); return <button onClick={() => setCount(value => value + 1)}>error {count}</button> }
+function Thrower(): never { throw original }
+function Rethrow({ error }: { error: Error }): never { throw error }
+
+it.each(['csr', 'hydrate', 'scheduled', 'commit'] as const)('%s: bubbles a fallback rethrow to the enclosing boundary and preserves the Suspense range', async mode => {
+  const container = document.createElement('div')
+  document.body.append(container)
+  cleanup.push(() => container.remove())
+  const caught = vi.fn(), innerCaught = vi.fn(), errors = vi.fn()
+  let trigger!: () => void
+  function Child() {
+    const [failed, setFailed] = useState(false)
+    trigger = () => setFailed(true)
+    if (mode === 'csr' || mode === 'hydrate' || failed) return <Thrower />
+    return <span ref={mode === 'commit' ? () => { throw original } : undefined}>ready</span>
+  }
+  const tree = <main><Suspense fallback={null}><Boundary fallback={() => <ErrorPage />} caught={caught}><Boundary fallback={error => <Rethrow error={error} />} caught={innerCaught}><Child /></Boundary></Boundary></Suspense><em>after</em></main>
+  if (mode === 'hydrate') container.innerHTML = '<main><!--$0--><p>server generic error</p><!--/$--><em>after</em></main>'
+  const sibling = container.querySelector('em')
+  const root = mode === 'hydrate' ? hydrateRoot(container, tree, { onRecoverableError() {}, onUncaughtError: errors }) : createRoot(container, { onUncaughtError: errors })
+  cleanup.push(() => root.unmount())
+  if (mode !== 'hydrate') flushSync(() => root.render(tree))
+  if (mode === 'scheduled') flushSync(trigger)
+  await vi.waitFor(() => expect(container.querySelector('button')?.textContent).toBe('error 0'))
+  expect(caught).toHaveBeenCalledTimes(1)
+  expect(caught).toHaveBeenCalledWith(original)
+  expect(innerCaught).not.toHaveBeenCalled()
+  expect(errors).not.toHaveBeenCalled()
+  if (mode === 'hydrate') {
+    expect(container.querySelector('em')).toBe(sibling)
+    const main = container.querySelector('main')!
+    const start = Array.from(main.childNodes).find(node => node.nodeType === 8 && node.textContent?.startsWith('$'))!
+    const end = Array.from(main.childNodes).find(node => node.nodeType === 8 && node.textContent === '/$')!
+    expect(start.compareDocumentPosition(container.querySelector('button')!) & Node.DOCUMENT_POSITION_FOLLOWING).not.toBe(0)
+    expect(container.querySelector('button')!.compareDocumentPosition(end) & Node.DOCUMENT_POSITION_FOLLOWING).not.toBe(0)
+  }
+  flushSync(() => container.querySelector('button')!.click())
+  expect(container.querySelector('button')?.textContent).toBe('error 1')
+})
+
+it('lets a newly nested boundary catch during fallback recovery, then restores the original boundary after reset', () => {
+  const container = document.createElement('div'), nestedCaught = vi.fn(), outerCaught = vi.fn(), innerCaught = vi.fn()
+  let inner!: Boundary
+  const tree = (fail: boolean) => <Boundary fallback={() => <p>outer</p>} caught={outerCaught}><Boundary ref={value => { if (value) inner = value }} fallback={() => <Boundary fallback={() => <p>nested</p>} caught={nestedCaught}><Rethrow error={fallbackError} /></Boundary>} caught={innerCaught}>{fail ? <Thrower /> : <span>ready</span>}</Boundary></Boundary>
+  const root = createRoot(container)
+  cleanup.push(() => root.unmount())
+  flushSync(() => root.render(tree(true)))
+  expect(container.textContent).toBe('nested')
+  expect(nestedCaught).toHaveBeenCalledWith(fallbackError)
+  expect(innerCaught).toHaveBeenCalledWith(original)
+  expect(outerCaught).not.toHaveBeenCalled()
+  flushSync(() => { root.render(tree(false)); inner.setState({ error: null }) })
+  expect(container.textContent).toBe('ready')
+  flushSync(() => root.render(tree(true)))
+  expect(container.textContent).toBe('nested')
+  expect(innerCaught).toHaveBeenCalledTimes(2)
+})
+
+it('bubbles a different fallback error and clears recovery scope after an uncaught fallback', () => {
+  const container = document.createElement('div'), caught = vi.fn(), uncaught = vi.fn()
+  const root = createRoot(container, { onUncaughtError: uncaught })
+  cleanup.push(() => root.unmount())
+  const inner = <Boundary fallback={() => <Rethrow error={fallbackError} />}><Thrower /></Boundary>
+  flushSync(() => root.render(inner))
+  expect(uncaught).toHaveBeenCalledWith(fallbackError, expect.anything())
+  flushSync(() => root.render(<Boundary fallback={() => <ErrorPage />} caught={caught}>{inner}</Boundary>))
+  expect(caught).toHaveBeenCalledWith(fallbackError)
+  expect(container.textContent).toBe('error 0')
+})