From 6a7d9e9ba6218a00dddf7591e5337b9ac468d6fb Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Thu, 9 Jul 2026 14:20:17 -0400 Subject: [PATCH 1/2] fix(items): retry failed model loads, settle skipped items, keep exports clean MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/nodes/src/item/renderer.tsx | 91 +++++++++++++++++-- .../viewer/src/components/error-boundary.tsx | 11 +++ packages/viewer/src/store/use-viewer.ts | 23 +++++ .../viewer/src/systems/item/item-system.tsx | 7 ++ 4 files changed, 125 insertions(+), 7 deletions(-) diff --git a/packages/nodes/src/item/renderer.tsx b/packages/nodes/src/item/renderer.tsx index d1d26929a..f9096d5c0 100644 --- a/packages/nodes/src/item/renderer.tsx +++ b/packages/nodes/src/item/renderer.tsx @@ -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 ( @@ -199,11 +204,74 @@ 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, markSettled }: { node: ItemNode; markSettled: () => 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. + 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(() => { + 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]) + + 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 + + return ( + } onError={handleError} resetKey={epoch}> + }> + + + + ) +} + export const ItemRenderer = ({ node: storeNode }: { node: ItemNode }) => { const ref = useRef(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. + const markSettled = useCallback(() => { + const group = ref.current as (Group & { userData: Record }) | null + if (group) group.userData.itemModelSettled = true + }, []) + // 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 +285,17 @@ export const ItemRenderer = ({ node: storeNode }: { node: ItemNode }) => { const roomClearPreview = (node as ItemNode & { roomClearPreview?: unknown }).roomClearPreview === true + useEffect(() => { + if (roomClearPreview) markSettled() + }, [roomClearPreview, markSettled]) + const content = ( {roomClearPreview ? ( ) : ( <> - }> - }> - - - + {node.children?.map((childId) => ( ))} @@ -262,6 +330,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 ( [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(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) diff --git a/packages/viewer/src/components/error-boundary.tsx b/packages/viewer/src/components/error-boundary.tsx index eada133ac..5f8ba896b 100644 --- a/packages/viewer/src/components/error-boundary.tsx +++ b/packages/viewer/src/components/error-boundary.tsx @@ -6,6 +6,11 @@ interface ErrorBoundaryProps { fallback: ReactNode /** Tag for log lines so we can tell which boundary swallowed an error. */ scope?: string + /** Notified once per caught error — lets the host schedule a retry. */ + onError?: (error: Error) => void + /** Changing this key clears a caught error and re-mounts `children` — the + * retry half of `onError` (bump it after clearing whatever failed). */ + resetKey?: unknown } export class ErrorBoundary extends Component { @@ -19,6 +24,12 @@ export class ErrorBoundary extends Component void + /** Item model loads that exhausted their retries — nodeId → asset URL. The + * scene renders without these items (they settle as skipped); a bake host + * can persist the map onto the artifact's metadata so a missing item is + * queryable instead of silently absent. Transient (never persisted). */ + itemLoadFailures: Record + reportItemLoadFailure: (nodeId: string, url: string) => void + clearItemLoadFailure: (nodeId: string) => void + /** Suspend the render loop while the canvas is fully covered (e.g. studio gallery). */ renderPaused: boolean setRenderPaused: (value: boolean) => void @@ -245,6 +253,21 @@ const useViewer = create()( isExporting: false, setExporting: (value) => set({ isExporting: value }), + itemLoadFailures: {}, + reportItemLoadFailure: (nodeId, url) => + set((state) => + state.itemLoadFailures[nodeId] === url + ? state + : { itemLoadFailures: { ...state.itemLoadFailures, [nodeId]: url } }, + ), + clearItemLoadFailure: (nodeId) => + set((state) => { + if (!(nodeId in state.itemLoadFailures)) return state + const next = { ...state.itemLoadFailures } + delete next[nodeId] + return { itemLoadFailures: next } + }), + renderPaused: false, setRenderPaused: (value) => set({ renderPaused: value }), diff --git a/packages/viewer/src/systems/item/item-system.tsx b/packages/viewer/src/systems/item/item-system.tsx index 0458437ad..7aa7fb679 100644 --- a/packages/viewer/src/systems/item/item-system.tsx +++ b/packages/viewer/src/systems/item/item-system.tsx @@ -53,6 +53,13 @@ export const ItemSystem = () => { } } + // Hold the mark until the model settles (loaded, terminally failed, or + // never expected — see ItemRenderer.markSettled). Registration alone + // isn't "built": clearing then would let scene-ready fire while GLBs are + // still downloading and a bake would export loading placeholders. + const settled = (mesh.userData as { itemModelSettled?: boolean }).itemModelSettled + if (!settled) return + clearDirty(id as AnyNodeId) }) }, 2) From 2b032cc2dafc73f8a88ee4b01f130242b2976a45 Mon Sep 17 00:00:00 2001 From: Wassim SAMAD Date: Thu, 9 Jul 2026 14:36:20 -0400 Subject: [PATCH 2/2] fix(items): reset retry budget + settled flag when the asset URL changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- packages/nodes/src/item/renderer.tsx | 36 +++++++++++++++++++--------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/packages/nodes/src/item/renderer.tsx b/packages/nodes/src/item/renderer.tsx index f9096d5c0..3445e536b 100644 --- a/packages/nodes/src/item/renderer.tsx +++ b/packages/nodes/src/item/renderer.tsx @@ -215,11 +215,20 @@ const MODEL_RETRY_DELAYS_MS = [1_000, 3_000] * during exports) and lands in `useViewer.itemLoadFailures` so a bake host can * record which items are missing from the artifact. */ -const ModelWithRetry = ({ node, markSettled }: { node: ItemNode; markSettled: () => void }) => { +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. + // 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) || '' @@ -227,19 +236,23 @@ const ModelWithRetry = ({ node, markSettled }: { node: ItemNode; markSettled: () 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}`, - ) + 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() @@ -266,10 +279,11 @@ export const ItemRenderer = ({ node: storeNode }: { node: ItemNode }) => { // "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. - const markSettled = useCallback(() => { + // 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 }) | null - if (group) group.userData.itemModelSettled = true + if (group) group.userData.itemModelSettled = value }, []) // Merge live drag overrides so the mesh transforms in real time during a @@ -286,8 +300,8 @@ export const ItemRenderer = ({ node: storeNode }: { node: ItemNode }) => { (node as ItemNode & { roomClearPreview?: unknown }).roomClearPreview === true useEffect(() => { - if (roomClearPreview) markSettled() - }, [roomClearPreview, markSettled]) + if (roomClearPreview) setSettled(true) + }, [roomClearPreview, setSettled]) const content = ( @@ -295,7 +309,7 @@ export const ItemRenderer = ({ node: storeNode }: { node: ItemNode }) => { ) : ( <> - + {node.children?.map((childId) => ( ))}