Try to avoid the black screen again & improve error messages (#7327)

* Fix the black screen of death

* fmt

* make check

* Clean up

* Fix up zoom to fit

* Change how emulateNetworkConditions work

* Do NOT use browser's offline/online mechanisms

* Fix test

* Improve network error messages

* Signal offline when failed event comes in

* Don't use logic on components that only want a loader

* Remove unnecessary pause state transition

---------

Co-authored-by: jacebrowning <jacebrowning@gmail.com>
This commit is contained in:
Zookeeper Lee
2025-06-04 13:59:22 -04:00
committed by GitHub
parent ff92c73ac4
commit 5ceb92d117
12 changed files with 330 additions and 183 deletions

View File

@ -1,4 +1,3 @@
import { isPlaywright } from '@src/lib/isPlaywright'
import { useAppState } from '@src/AppState'
import { ClientSideScene } from '@src/clientSideScene/ClientSideSceneComp'
import { ViewControlContextMenu } from '@src/components/ViewControlMenu'
@ -52,8 +51,10 @@ export const EngineStream = (props: {
const last = useRef<number>(Date.now())
const [firstPlay, setFirstPlay] = useState(true)
const [isRestartRequestStarting, setIsRestartRequestStarting] =
useState(false)
const [goRestart, setGoRestart] = useState(false)
const [timeoutId, setTimeoutId] = useState<
ReturnType<typeof setTimeout> | undefined
>(undefined)
const [attemptTimes, setAttemptTimes] = useState<[number, number]>([
0,
TIME_1_SECOND,
@ -85,18 +86,21 @@ export const EngineStream = (props: {
const streamIdleMode = settings.app.streamIdleMode.current
useEffect(() => {
// Will cause a useEffect loop if not checked for.
if (engineStreamState.context.videoRef.current !== null) return
engineStreamActor.send({
type: EngineStreamTransition.SetVideoRef,
videoRef: { current: videoRef.current },
})
}, [videoRef.current])
}, [videoRef.current, engineStreamState])
useEffect(() => {
if (engineStreamState.context.canvasRef.current !== null) return
engineStreamActor.send({
type: EngineStreamTransition.SetCanvasRef,
canvasRef: { current: canvasRef.current },
})
}, [canvasRef.current])
}, [canvasRef.current, engineStreamState])
useEffect(() => {
engineStreamActor.send({
@ -131,24 +135,6 @@ export const EngineStream = (props: {
})
}
useEffect(() => {
// Only try to start the stream if we're stopped or think we're done
// waiting for dependencies.
if (
!(
engineStreamState.value === EngineStreamState.WaitingForDependencies ||
engineStreamState.value === EngineStreamState.Stopped
)
)
return
// Don't bother trying to connect if the auth token is empty.
// We have the checks in the machine but this can cause a hot loop.
if (!engineStreamState.context.authToken) return
startOrReconfigureEngine()
}, [engineStreamState, setAppState])
// I would inline this but it needs to be a function for removeEventListener.
const play = () => {
engineStreamActor.send({
@ -174,12 +160,13 @@ export const EngineStream = (props: {
console.log('scene is ready, execute kcl')
const kmp = kclManager.executeCode().catch(trap)
if (!firstPlay) return
setFirstPlay(false)
// Reset the restart timeouts
setAttemptTimes([0, TIME_1_SECOND])
console.log(firstPlay)
if (!firstPlay) return
setFirstPlay(false)
console.log('firstPlay true, zoom to fit')
kmp
.then(async () => {
@ -211,51 +198,76 @@ export const EngineStream = (props: {
// We do a back-off restart, using a fibonacci sequence, since it
// has a nice retry time curve (somewhat quick then exponential)
const attemptRestartIfNecessary = () => {
if (isRestartRequestStarting) return
setIsRestartRequestStarting(true)
setTimeout(() => {
engineStreamState.context.videoRef.current?.pause()
engineCommandManager.tearDown()
startOrReconfigureEngine()
setFirstPlay(false)
setIsRestartRequestStarting(false)
}, attemptTimes[0] + attemptTimes[1])
// Timeout already set.
if (timeoutId) return
setTimeoutId(
setTimeout(() => {
engineStreamState.context.videoRef.current?.pause()
engineCommandManager.tearDown()
startOrReconfigureEngine()
setFirstPlay(true)
setTimeoutId(undefined)
setGoRestart(false)
}, attemptTimes[0] + attemptTimes[1])
)
setAttemptTimes([attemptTimes[1], attemptTimes[0] + attemptTimes[1]])
}
// Poll that we're connected. If not, send a reset signal.
// Do not restart if we're in idle mode.
const connectionCheckIntervalId = setInterval(() => {
// SKIP DURING TESTS BECAUSE IT WILL MESS WITH REUSING THE
// ELECTRON INSTANCE.
if (isPlaywright()) {
const onOffline = () => {
if (
!(
EngineConnectionStateType.Disconnected ===
engineCommandManager.engineConnection?.state.type ||
EngineConnectionStateType.Disconnecting ===
engineCommandManager.engineConnection?.state.type
)
) {
return
}
// Don't try try to restart if we're already connected!
const hasEngineConnectionInst = engineCommandManager.engineConnection
const isDisconnected =
engineCommandManager.engineConnection?.state.type ===
EngineConnectionStateType.Disconnected
const inIdleMode = engineStreamState.value === EngineStreamState.Paused
if ((hasEngineConnectionInst && !isDisconnected) || inIdleMode) return
engineStreamActor.send({ type: EngineStreamTransition.Stop })
attemptRestartIfNecessary()
}, TIME_1_SECOND)
}
if (
!goRestart &&
engineStreamState.value === EngineStreamState.WaitingForDependencies
) {
setGoRestart(true)
}
if (goRestart && !timeoutId) {
attemptRestartIfNecessary()
}
engineCommandManager.addEventListener(
EngineCommandManagerEvents.EngineRestartRequest,
attemptRestartIfNecessary
)
return () => {
clearInterval(connectionCheckIntervalId)
engineCommandManager.addEventListener(
EngineCommandManagerEvents.Offline,
onOffline
)
return () => {
engineCommandManager.removeEventListener(
EngineCommandManagerEvents.EngineRestartRequest,
attemptRestartIfNecessary
)
engineCommandManager.removeEventListener(
EngineCommandManagerEvents.Offline,
onOffline
)
}
}, [engineStreamState, attemptTimes, isRestartRequestStarting])
}, [
engineStreamState,
attemptTimes,
goRestart,
timeoutId,
engineCommandManager.engineConnection?.state.type,
])
useEffect(() => {
// If engineStreamMachine is already reconfiguring, bail.
@ -269,7 +281,7 @@ export const EngineStream = (props: {
const canvas = engineStreamState.context.canvasRef?.current
if (!canvas) return
new ResizeObserver(() => {
const observer = new ResizeObserver(() => {
// Prevents:
// `Uncaught ResizeObserver loop completed with undelivered notifications`
window.requestAnimationFrame(() => {
@ -280,13 +292,19 @@ export const EngineStream = (props: {
if (
(Math.abs(video.width - window.innerWidth) > 4 ||
Math.abs(video.height - window.innerHeight) > 4) &&
!engineStreamState.matches(EngineStreamState.WaitingToPlay)
engineStreamState.matches(EngineStreamState.Playing)
) {
timeoutStart.current = Date.now()
startOrReconfigureEngine()
}
})
}).observe(document.body)
})
observer.observe(document.body)
return () => {
observer.disconnect()
}
}, [engineStreamState.value])
/**
@ -345,8 +363,21 @@ export const EngineStream = (props: {
timeoutStart.current = null
} else if (timeoutStart.current) {
const elapsed = Date.now() - timeoutStart.current
if (elapsed >= IDLE_TIME_MS) {
// Don't pause if we're already disconnected.
if (
// It's unnecessary to once again setup an event listener for
// offline/online to capture this state, when this state already
// exists on the window.navigator object. In hindsight it makes
// me (lee) regret we set React state variables such as
// isInternetConnected in other files when we could check this
// object instead.
engineCommandManager.engineConnection?.state.type ===
EngineConnectionStateType.ConnectionEstablished &&
elapsed >= IDLE_TIME_MS &&
engineStreamState.value === EngineStreamState.Playing
) {
timeoutStart.current = null
console.log('PAUSING')
engineStreamActor.send({ type: EngineStreamTransition.Pause })
}
}
@ -357,7 +388,7 @@ export const EngineStream = (props: {
return () => {
window.cancelAnimationFrame(frameId)
}
}, [modelingMachineState])
}, [modelingMachineState, engineStreamState.value])
useEffect(() => {
if (!streamIdleMode) return
@ -370,9 +401,18 @@ export const EngineStream = (props: {
return
}
if (engineStreamState.value === EngineStreamState.Paused) {
startOrReconfigureEngine()
}
engineStreamActor.send({
type: EngineStreamTransition.Resume,
modelingMachineActorSend,
settings: settingsEngine,
setAppState,
onMediaStream(mediaStream: MediaStream) {
engineStreamActor.send({
type: EngineStreamTransition.SetMediaStream,
mediaStream,
})
},
})
timeoutStart.current = Date.now()
}
@ -471,7 +511,11 @@ export const EngineStream = (props: {
}
sendSelectEventToEngine(e)
.then(({ entity_id }) => {
.then((result) => {
if (!result) {
return
}
const { entity_id } = result
if (!entity_id) {
// No entity selected. This is benign
return
@ -535,7 +579,7 @@ export const EngineStream = (props: {
EngineStreamState.Resuming,
].some((s) => s === engineStreamState.value) && (
<Loading dataTestId="loading-engine" className="fixed inset-0 h-screen">
Connecting to engine
Connecting to engine...
</Loading>
)}
</div>