fix(items): retry failed model loads, settle skipped items, keep exports clean (#480)

* fix(items): retry failed model loads, settle skipped items, keep exports clean A transient storage failure (observed: Supabase 504s under the bake page's ~200-concurrent-request burst) permanently broke an item for the whole session: drei's useGLTF caches the rejected load by URL, the per-item ErrorBoundary swallowed it, and the red debug-box fallback was BAKED into the exported GLB (observed in a prod artifact). Meanwhile ItemSystem cleared the dirty mark at group registration — before the model resolved — so scene-ready could fire while GLBs were still loading, risking exported placeholder geometry. - ModelWithRetry: bounded retries (1s/3s) that clear the useGLTF cache entry and re-mount via the boundary's new resetKey. The timer is owned by an effect keyed on the failure count, so StrictMode's synthetic unmount/remount re-arms it instead of silently discarding it (the naive onError-scheduled timer died exactly that way in dev). - Exhausted retries settle the item as SKIPPED: the debug box renders nothing during exports, and the failure lands in useViewer.itemLoadFailures (nodeId -> url) so a bake host can persist which items are missing from the artifact. - ItemSystem holds the dirty mark until the item settles (model mounted, terminally failed, or never expected) — scene-ready now genuinely waits for item content; loading placeholders also hide during exports. - ErrorBoundary: onError + resetKey props. Verified against a 200+-item prod scene locally: permanent 504 -> exactly 3 fetch attempts, bake completes without the item and without debug boxes; 504-once -> retry heals, artifact byte-equivalent to the intact run; no-failure runs unchanged (demo_1 byte-identical). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(items): reset retry budget + settled flag when the asset URL changes Bugbot: after a terminal load failure, swapping the item's model kept the stale failures/epoch state and the settled flag — the new URL never even attempted to load. ModelWithRetry is now keyed by asset src (clean retry budget per URL) and un-settles the item on mount so the replacement load is awaited by ItemSystem/scene-ready too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 14:50:20 -04:00
parent 5831b7b234
commit feeabf4bd3
4 changed files with 139 additions and 7 deletions
@@ -36,7 +36,7 @@ import { useAnimations } from '@react-three/drei'
 import { Clone } from '@react-three/drei/core/Clone'
 import { useGLTF } from '@react-three/drei/core/Gltf'
 import { useFrame } from '@react-three/fiber'
-import { Suspense, useEffect, useLayoutEffect, useMemo, useRef } from 'react'
+import { Suspense, useCallback, useEffect, useLayoutEffect, useMemo, useRef, useState } from 'react'
 import type { AnimationAction, Group, Material, Mesh } from 'three'
 import { MathUtils } from 'three'
 import { positionLocal, smoothstep, time } from 'three/tsl'
@@ -181,6 +181,7 @@ const resolveItemMaterial = (
 const BrokenItemFallback = ({ node }: { node: ItemNode }) => {
  const handlers = useNodeEvents(node, 'item')
  const shading = useViewer((s) => s.shading)
+  const isExporting = useViewer((s) => s.isExporting)
  const [w, h, d] = node.asset.dimensions
  const material = useMemo(() => {
    const next = createDefaultMaterial('#ef4444', 1, shading) as MutableMaterial
@@ -191,6 +192,10 @@ const BrokenItemFallback = ({ node }: { node: ItemNode }) => {
    return next
  }, [shading])

+  // Debug affordance only — a bake must never ship the red placeholder box
+  // (observed baked into a prod artifact when an item GLB 504'd mid-capture).
+  if (isExporting) return null
+
  return (
    <mesh position-y={h / 2} {...handlers}>
      <boxGeometry args={[w, h, d]} />
@@ -199,11 +204,88 @@ const BrokenItemFallback = ({ node }: { node: ItemNode }) => {
  )
 }

+const MODEL_RETRY_DELAYS_MS = [1_000, 3_000]
+
+/**
+ * Load the item model with bounded retries. drei's `useGLTF` caches a rejected
+ * load by URL, so a transient fetch failure (e.g. a storage 504 under the bake
+ * page's asset-request burst) would otherwise stay broken for the whole
+ * session — clear the cache entry and re-mount. After the retries are
+ * exhausted the item settles as SKIPPED: it renders the debug box (nothing
+ * during exports) and lands in `useViewer.itemLoadFailures` so a bake host can
+ * record which items are missing from the artifact.
+ */
+const ModelWithRetry = ({
+  node,
+  setSettled,
+}: {
+  node: ItemNode
+  setSettled: (value: boolean) => void
+}) => {
+  // `failures` counts boundary catches; `epoch` bumps after each cache clear
+  // to reset the boundary and re-mount the loader. The retry timer is owned by
+  // an effect (not the error handler) so StrictMode's synthetic
+  // unmount/remount re-arms it instead of silently discarding it. The host
+  // keys this component by asset URL, so a model swap starts from a clean
+  // retry budget — and the mount effect below un-settles the item so the new
+  // load is awaited too.
+  const [failures, setFailures] = useState(0)
+  const [epoch, setEpoch] = useState(0)
+  const url = resolveCdnUrl(node.asset.src) || ''
+  const gaveUp = !url || failures > MODEL_RETRY_DELAYS_MS.length
+
+  const handleError = useCallback(() => setFailures((current) => current + 1), [])
+
+  useEffect(() => {
+    setSettled(false)
+  }, [setSettled])
+
+  useEffect(() => {
+    if (failures === 0 || gaveUp) return
+    const delay = MODEL_RETRY_DELAYS_MS[failures - 1] ?? 0
+    const timer = setTimeout(() => {
+      console.log(`[item] retrying model load (${failures}/${MODEL_RETRY_DELAYS_MS.length}) ${url}`)
+      useGLTF.clear(url)
+      setEpoch((current) => current + 1)
+    }, delay)
+    return () => clearTimeout(timer)
+  }, [failures, gaveUp, url])
+
+  const markSettled = useCallback(() => setSettled(true), [setSettled])
+
+  useEffect(() => {
+    if (!gaveUp) return
+    markSettled()
+    useViewer.getState().reportItemLoadFailure(node.id, url)
+    return () => useViewer.getState().clearItemLoadFailure(node.id)
+  }, [gaveUp, markSettled, node.id, url])
+
+  if (gaveUp) return <BrokenItemFallback node={node} />
+
+  return (
+    <ErrorBoundary fallback={<PreviewModel node={node} />} onError={handleError} resetKey={epoch}>
+      <Suspense fallback={<PreviewModel node={node} />}>
+        <ModelRenderer markSettled={markSettled} node={node} />
+      </Suspense>
+    </ErrorBoundary>
+  )
+}
+
 export const ItemRenderer = ({ node: storeNode }: { node: ItemNode }) => {
  const ref = useRef<Group>(null!)

  useRegistry(storeNode.id, storeNode.type, ref)

+  // "Settled" = the model resolved, terminally failed (skipped), or was never
+  // expected. `ItemSystem` holds the dirty mark until then, so scene-ready
+  // (and headless bakes) wait for real item content instead of exporting the
+  // loading placeholder. A model swap un-settles (ModelWithRetry's mount
+  // effect via its URL key) so the replacement load is awaited too.
+  const setSettled = useCallback((value: boolean) => {
+    const group = ref.current as (Group & { userData: Record<string, unknown> }) | null
+    if (group) group.userData.itemModelSettled = value
+  }, [])
+
  // Merge live drag overrides so the mesh transforms in real time during a
  // drag (e.g. the in-world rotate gizmo). The handle writes the in-flight
  // rotation to `useLiveNodeOverrides` on every pointer move and commits to
@@ -217,17 +299,17 @@ export const ItemRenderer = ({ node: storeNode }: { node: ItemNode }) => {
  const roomClearPreview =
    (node as ItemNode & { roomClearPreview?: unknown }).roomClearPreview === true

+  useEffect(() => {
+    if (roomClearPreview) setSettled(true)
+  }, [roomClearPreview, setSettled])
+
  const content = (
    <group position={node.position} ref={ref} rotation={node.rotation} visible={node.visible}>
      {roomClearPreview ? (
        <ClearPreviewModel node={node} />
      ) : (
        <>
-          <ErrorBoundary fallback={<BrokenItemFallback node={node} />}>
-            <Suspense fallback={<PreviewModel node={node} />}>
-              <ModelRenderer node={node} />
-            </Suspense>
-          </ErrorBoundary>
+          <ModelWithRetry key={node.asset.src ?? 'no-src'} node={node} setSettled={setSettled} />
          {node.children?.map((childId) => (
            <NodeRenderer key={childId} nodeId={childId} />
          ))}
@@ -262,6 +344,9 @@ function getPreviewMaterial(shading: RenderShading): Material {

 const PreviewModel = ({ node }: { node: ItemNode }) => {
  const shading = useViewer((s) => s.shading)
+  const isExporting = useViewer((s) => s.isExporting)
+  // Loading placeholder — must never land in an exported GLB.
+  if (isExporting) return null
  return (
    <mesh material={getPreviewMaterial(shading)} position-y={node.asset.dimensions[1] / 2}>
      <boxGeometry
@@ -296,10 +381,16 @@ const multiplyScales = (
  b: [number, number, number],
 ): [number, number, number] => [a[0] * b[0], a[1] * b[1], a[2] * b[2]]

-const ModelRenderer = ({ node }: { node: ItemNode }) => {
+const ModelRenderer = ({ node, markSettled }: { node: ItemNode; markSettled?: () => void }) => {
  const { scene, nodes, animations } = useGLTF(resolveCdnUrl(node.asset.src) || '')
  const ref = useRef<Group>(null!)
  const { actions } = useAnimations(animations, ref)
+
+  // Mounting past the suspense gate means the GLB resolved — the item's build
+  // work is done (`ItemSystem` may clear its dirty mark, scene-ready may fire).
+  useEffect(() => {
+    markSettled?.()
+  }, [markSettled])
  const shading = useViewer((s) => s.shading)
  const textures = useViewer((s) => s.textures)
  const colorPreset = useViewer((s) => s.colorPreset)