Skip to content

fix: bound emote detection so a stalled model cannot empty the import dialog - #1489

Open
cyaiox wants to merge 1 commit into
mainfrom
fix/bound-emote-detection
Open

fix: bound emote detection so a stalled model cannot empty the import dialog#1489
cyaiox wants to merge 1 commit into
mainfrom
fix/bound-emote-detection

Conversation

@cyaiox

@cyaiox cyaiox commented Aug 14, 2026

Copy link
Copy Markdown
Member

Independent of #1487 and #1488 — different root cause, no shared files, mergeable in any order.

Context and Problem Statement

Found while auditing for other instances of the #1485 silent-hang shape. This one is a genuinely different cause on a path that #1485's fix cannot reach.

useSliderAssets.ts:15-24 awaits isEmote for every selected file through Promise.all before the slider renders anything, and isEmote awaits Babylon's LoadAssetContainerAsync with no timeout. Its try/catch covers a rejection, not a load that simply never settles — a corrupt model or a lost WebGL context. When that happens assets stays [], Slider.tsx:107 returns null, and the import modal opens completely blank: nothing rendered, nothing logged, no way forward but cancel.

This runs before any preview mounts, so the preview-level deadline added in #1487 cannot cover it. For contrast, the two other places that wait on an external signal — engine-iframe.ts:155 and iframe-transport.ts:88 — both bound their waits already; this path was the outlier.

Solution

Key changes:

  • Race the loader against a timeout that rejects into the existing catch, so the failure path is the one already there rather than a new branch. It logs and answers false, and false only ever means "treat this as an ordinary model".
  • Because isEmote now always settles, Promise.all upstream is guaranteed to settle too — so useSliderAssets needs no change at all. Fixing it at the root kept this to one source file.
  • Clear the timer in the existing finally, alongside the URL revoke and the scene/engine dispose, so a fast load leaves nothing pending.

Testing

  • make test — 1413 passing across all six workspaces; make typecheck, make lint, make format clean
  • The new test reproduces the hang before the fix: with the loader stubbed to a promise that never settles, the case fails with Test timed out in 10000ms — the test hanging exactly as the dialog does. The two sibling cases (loads-and-is-an-emote, loader-rejects) pass throughout, which is what proves the mocks are faithful rather than the assertion being trivially true.
  • New utils.spec.ts covers all three outcomes: rigged model → emote, rejection → not an emote, never settles → gives up, answers false, and logs.

Impact

A model Babylon cannot finish loading no longer empties the import dialog: detection gives up after 30s, logs why, and the file imports as an ordinary model. Worst case shifts from "blank modal, cancel is the only option" to "a model that might have been an emote is imported as a static one" — and it is logged, so it is diagnosable rather than invisible.

Note the 30s only elapses in the pathological case. A normal load resolves in milliseconds and clears the timer.

Screenshots

Not applicable — no UI change. The difference is a modal that opens rather than one that stays blank.

… dialog

`useSliderAssets` awaits `isEmote` for every selected file through `Promise.all`
before the slider renders, and `isEmote` awaits Babylon's `LoadAssetContainerAsync`
with no timeout. Its `try/catch` covers a rejection, not a load that simply never
settles — a corrupt model or a lost WebGL context leaves `assets` empty, so the
slider returns null and the modal opens blank with nothing logged and no way
forward but cancel.

The loader now races a timeout that rejects into the existing catch, so the failure
path is the one already there rather than a new branch: it logs and answers false,
and false only means the file is treated as an ordinary model. `Promise.all`
upstream is therefore guaranteed to settle, which is why `useSliderAssets` needs no
change of its own.

This is the same silent-hang shape as #1485 on a different path — it runs before any
preview mounts, so the preview's own deadline cannot cover it — but a distinct root
cause, hence a separate change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant