Browse Source

fix(gui): contain selector failures and key the chain boundary by entry

Two hardenings on the chain outlet branch:

A throwing chain selector runs before its entry's SlotErrorBoundary
exists, so uncontained it blacked out the whole owner region and skipped
the remaining chain. It now degrades to a decline: reported via
console.error with the registrant identity, later entries still tried,
all-null/all-throw passes land on the owner fallback.

The elected entry's boundary is now keyed by entry identity: an unkeyed
boundary that failed on entry A survived a re-election and kept a
healthy entry B blacked out until the outlet unmounted. The key remounts
the boundary fresh whenever the election changes.
imccyu 2 months ago
parent
commit
a8c6dcb180

+ 36 - 3
packages/client/web-react/src/scoped-slots.tsx

@@ -135,6 +135,26 @@ function cachedSessionInject(entry: StoredEntry, cell: SessionCell, actions: obj
   return props
 }
 
+/**
+ * Entry-identity React keys for chain boundaries. A chain outlet renders ONE
+ * elected entry through an error boundary; without a key, a boundary that
+ * failed on entry A would survive a re-election and keep a healthy entry B
+ * blacked out. Keying by entry identity remounts the boundary fresh whenever
+ * the election changes (entries are identity-stable per registration, so the
+ * key is stable while the same entry stays elected).
+ */
+let nextEntryKey = 0
+const entryKeys = new WeakMap<StoredEntry, number>()
+
+function entryKeyOf(entry: StoredEntry): number {
+  let key = entryKeys.get(entry)
+  if (key === undefined) {
+    key = nextEntryKey++
+    entryKeys.set(entry, key)
+  }
+  return key
+}
+
 /**
  * Per-entry isolation: one registrant crashing (component render or inject
  * factory) must not take down siblings. Assembly errors (missing providers)
@@ -265,9 +285,22 @@ function SlotOutlet({ slotKey, ownerProps, opts }: {
     // pass runs per render with zero mount side effects: the first non-null
     // election renders, decliners never mount.
     for (const entry of entries) {
-      // Chain entries always carry select (SlotCore register validation).
-      const matched = (entry.select as (owner: object) => unknown)(ownerProps)
-      if (matched !== null) return guarded(entry, undefined, { ...ownerProps, matched })
+      let matched: unknown
+      try {
+        // Chain entries always carry select (SlotCore register validation).
+        matched = (entry.select as (owner: object) => unknown)(ownerProps)
+      } catch (error) {
+        // A throwing selector is a registrant contract breach (select MUST be
+        // pure and total), but it runs before the entry's SlotErrorBoundary
+        // exists — uncontained it would black out the whole owner region. So
+        // it degrades to a decline: the chain and the fallback stay intact,
+        // and the breach is reported like a crashed entry.
+        console.error(
+          `chain selector crashed in '${slotKey}' (${entry.registrant ?? 'unknown registrant'}), treating as declined:`,
+          error)
+        continue
+      }
+      if (matched !== null) return guarded(entry, entryKeyOf(entry), { ...ownerProps, matched })
     }
     return <>{opts?.fallback ?? null}</>
   }

+ 49 - 0
packages/client/web-react/tests/scoped-slots.spec.tsx

@@ -312,6 +312,55 @@ describe('chain outlets and the renderSlotChain binding', () => {
     expect(declinerBody).not.toHaveBeenCalled()
   })
 
+  it('contains a throwing selector to its entry: reported, treated as declined, chain and fallback intact', () => {
+    const h = makeHost()
+    h.declare('k.chain', CHAIN_ROOT)
+    h.add('k.chain', chainEntryOf({
+      component: () => <span>never</span>,
+      select: () => { throw new Error('selector boom') },
+    }))
+    h.add('k.chain', chainEntryOf({
+      component: ({ matched }: { matched?: string }) => <b>{matched}</b>,
+      select: (owner) => (owner as { pick?: string }).pick ?? null,
+    }))
+    const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
+    const { view } = mountChainRoot(h, { 'k.chain': CHAIN_ROOT }, (renderSlotChain) => <>
+      <main>{renderSlotChain('k.chain', { pick: 'OK' })}</main>
+      <aside>{renderSlotChain('k.chain', {}, { fallback: <i>fb</i> })}</aside>
+    </>)
+    // The breach never escapes to the owner region: later entries still get
+    // tried, and an all-throw/all-null pass still lands on the fallback.
+    expect(view.container.querySelector('main')!.textContent).toBe('OK')
+    expect(view.container.querySelector('aside')!.textContent).toBe('fb')
+    expect(spy.mock.calls.some(([msg]) => String(msg).includes('chain selector crashed'))).toBe(true)
+    spy.mockRestore()
+  })
+
+  it('remounts the boundary on re-election: a failed entry does not black out its replacement', () => {
+    const h = makeHost()
+    h.declare('k.chain', CHAIN_ROOT)
+    h.add('k.chain', chainEntryOf({
+      component: () => { throw new Error('entry A boom') },
+      select: (owner) => (owner as { pick?: string }).pick === 'A' ? {} : null,
+    }))
+    h.add('k.chain', chainEntryOf({
+      component: () => <b>B-ok</b>,
+      select: (owner) => (owner as { pick?: string }).pick === 'B' ? {} : null,
+    }))
+    let pick = 'A'
+    const spy = vi.spyOn(console, 'error').mockImplementation(() => {})
+    const { view } = mountChainRoot(h, { 'k.chain': CHAIN_ROOT },
+      (renderSlotChain) => renderSlotChain('k.chain', { pick }))
+    spy.mockRestore()
+    expect(view.container.querySelector('[data-slot-error]')).not.toBeNull()
+    // Re-elect entry B: the entry-keyed boundary remounts fresh instead of
+    // holding A's failed state over the healthy replacement.
+    pick = 'B'
+    act(() => { h.add('root', { component: () => null }) })   // root bump re-renders the dispatch site
+    expect(view.container.textContent).toBe('B-ok')
+    expect(view.container.querySelector('[data-slot-error]')).toBeNull()
+  })
+
   it('falls to the owner fallback when every selector declines, and re-routes live', () => {
     const h = makeHost()
     h.declare('k.chain', CHAIN_ROOT)