ByoungSooPark/modeling-app
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix the black screen of death (#7238)

Browse Source 
* 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
This commit is contained in:
Zookeeper Lee
2025-05-29 15:02:12 -04:00
committed by GitHub
parent 0eebb76bfd
commit 46b6707e3a
9 changed files with 219 additions and 132 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
e2e/playwright/test-network-and-connection-issues.spec.ts
View File

@ -61,7 +61,7 @@ test.describe('Test network related behaviors', () => {
})
// Expect the network to be down
await expect(networkToggle).toContainText('Problem')
await expect(networkToggle).toContainText('Network health (Offline)')
// Click the network widget
await networkWidget.click()
@ -156,7 +156,8 @@ test.describe('Test network related behaviors', () => {
// Expect the network to be down
await networkToggle.hover()
await expect(networkToggle).toContainText('Problem')
await expect(networkToggle).toContainText('Network health (Offline)')
// Ensure we are not in sketch mode
await expect(

17
e2e/playwright/test-utils.ts
View File

@ -364,11 +364,6 @@ export async function getUtils(page: Page, test_?: typeof test) {
)
}
// Chrome devtools protocol session only works in Chromium
const browserType = page.context().browser()?.browserType().name()
const cdpSession =
browserType !== 'chromium' ? null : await page.context().newCDPSession(page)
const util = {
waitForAuthSkipAppStart: () => waitForAuthAndLsp(page),
waitForPageLoad: () => waitForPageLoad(page),
@ -489,15 +484,9 @@ export async function getUtils(page: Page, test_?: typeof test) {
emulateNetworkConditions: async (
networkOptions: Protocol.Network.emulateNetworkConditionsParameters
) => {
if (cdpSession === null) {
// Use a fail safe if we can't simulate disconnect (on Safari)
return page.evaluate('window.engineCommandManager.tearDown()')
}
return cdpSession?.send(
'Network.emulateNetworkConditions',
networkOptions
)
return networkOptions.offline
? page.evaluate('window.engineCommandManager.offline()')
: page.evaluate('window.engineCommandManager.online()')
},
toNormalizedCode(text: string) {

3
src/App.tsx
View File

@ -114,6 +114,9 @@ export function App() {
// by the Projects view.
billingActor.send({ type: BillingTransition.Update, apiToken: authToken })
// Tell engineStream to wait for dependencies to start streaming.
engineStreamActor.send({ type: EngineStreamTransition.WaitForDependencies })
// When leaving the modeling scene, cut the engine stream.
return () => {
// When leaving the modeling scene, cut the engine stream.

3
src/clientSideScene/sceneEntities.ts
View File

@ -3584,7 +3584,8 @@ export class SceneEntities {
})
if (!resp) {
return Promise.reject('no response')
console.warn('No response')
return {} as Models['GetSketchModePlane_type']
}
if (isArray(resp)) {

170
src/components/EngineStream.tsx
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

21
src/hooks/useNetworkStatus.tsx
View File

@ -78,18 +78,19 @@ export function useNetworkStatus() {
}, [hasIssues, internetConnected, ping])
useEffect(() => {
const onlineCallback = () => {
setInternetConnected(true)
}
const offlineCallback = () => {
setInternetConnected(false)
setSteps(structuredClone(initialConnectingTypeGroupState))
}
window.addEventListener('online', onlineCallback)
window.addEventListener('offline', offlineCallback)
engineCommandManager.addEventListener(
EngineCommandManagerEvents.Offline,
offlineCallback
)
return () => {
window.removeEventListener('online', onlineCallback)
window.removeEventListener('offline', offlineCallback)
engineCommandManager.removeEventListener(
EngineCommandManagerEvents.Offline,
offlineCallback
)
}
}, [])
@ -139,6 +140,8 @@ export function useNetworkStatus() {
if (
engineConnectionState.type === EngineConnectionStateType.Connecting
) {
setInternetConnected(true)
const groups = Object.values(nextSteps)
for (let group of groups) {
for (let step of group) {
@ -168,6 +171,10 @@ export function useNetworkStatus() {
if (engineConnectionState.value.type === DisconnectingType.Error) {
setError(engineConnectionState.value.value)
} else if (
engineConnectionState.value.type === DisconnectingType.Quit
) {
return structuredClone(initialConnectingTypeGroupState)
}
}
}

99
src/lang/std/engineConnection.ts
View File

@ -1,4 +1,3 @@
import { TEST } from '@src/env'
import type { Models } from '@kittycad/lib'
import { VITE_KC_API_WS_MODELING_URL, VITE_KC_DEV_TOKEN } from '@src/env'
import { jsAppSettings } from '@src/lib/settings/settingsUtils'
@ -83,6 +82,9 @@ export enum ConnectionError {
TooManyConnections,
Outage,
// Observed to happen on a local network outage.
PeerConnectionRemoteDisconnected,
// An unknown error is the most severe because it has not been classified
// or encountered before.
Unknown,
@ -107,6 +109,8 @@ export const CONNECTION_ERROR_TEXT: Record<ConnectionError, string> = {
'There are too many open engine connections associated with your account.',
[ConnectionError.Outage]:
'We seem to be experiencing an outage. Please visit [status.zoo.dev](https://status.zoo.dev) for updates.',
[ConnectionError.PeerConnectionRemoteDisconnected]:
'The remote end has disconnected.',
[ConnectionError.Unknown]:
'An unexpected error occurred. Please report this to us.',
}
@ -226,6 +230,9 @@ export enum EngineConnectionEvents {
Opened = 'opened', // (engineConnection: EngineConnection) => void
Closed = 'closed', // (engineConnection: EngineConnection) => void
NewTrack = 'new-track', // (track: NewTrackArgs) => void
// A general offline state.
Offline = 'offline',
}
function toRTCSessionDescriptionInit(
@ -674,9 +681,20 @@ class EngineConnection extends EventTarget {
// The remote end broke up with us! :(
case 'disconnected':
this.state = {
type: EngineConnectionStateType.Disconnecting,
value: {
type: DisconnectingType.Error,
value: {
error: ConnectionError.PeerConnectionRemoteDisconnected,
context: event,
},
},
}
this.dispatchEvent(
new CustomEvent(EngineConnectionEvents.RestartRequest, {})
new CustomEvent(EngineConnectionEvents.Offline, {})
)
this.disconnectAll()
break
case 'closed':
this.pc?.removeEventListener('icecandidate', this.onIceCandidate)
@ -847,7 +865,6 @@ class EngineConnection extends EventTarget {
'message',
this.onDataChannelMessage
)
this.disconnectAll()
}
this.unreliableDataChannel?.addEventListener(
@ -866,7 +883,6 @@ class EngineConnection extends EventTarget {
},
},
}
this.disconnectAll()
}
this.unreliableDataChannel?.addEventListener(
'error',
@ -956,6 +972,9 @@ class EngineConnection extends EventTarget {
this.onNetworkStatusReady
)
this.dispatchEvent(
new CustomEvent(EngineConnectionEvents.Offline, {})
)
this.disconnectAll()
}
this.websocket.addEventListener('close', this.onWebSocketClose)
@ -974,8 +993,6 @@ class EngineConnection extends EventTarget {
},
}
}
this.disconnectAll()
}
this.websocket.addEventListener('error', this.onWebSocketError)
@ -1331,6 +1348,9 @@ export enum EngineCommandManagerEvents {
// the whole scene is ready (settings loaded)
SceneReady = 'scene-ready',
// we're offline
Offline = 'offline',
}
/**
@ -1380,6 +1400,7 @@ export class EngineCommandManager extends EventTarget {
* This is compared to the {@link outSequence} number to determine if we should ignore
* any out-of-order late responses in the unreliable channel.
*/
keepForcefulOffline = false
inSequence = 1
engineConnection?: EngineConnection
commandLogs: CommandLog[] = []
@ -1453,13 +1474,8 @@ export class EngineCommandManager extends EventTarget {
)
}
private onOffline = () => {
console.log('Browser reported network is offline')
if (TEST) {
console.warn('DURING TESTS ENGINECONNECTION.ONOFFLINE WILL DO NOTHING.')
return
}
this.onEngineConnectionRestartRequest()
private onEngineOffline = () => {
this.dispatchEvent(new CustomEvent(EngineCommandManagerEvents.Offline, {}))
}
idleMode: boolean = false
@ -1494,6 +1510,11 @@ export class EngineCommandManager extends EventTarget {
if (settings) {
this.settings = settings
}
if (this.keepForcefulOffline) {
return
}
if (width === 0 || height === 0) {
return
}
@ -1509,8 +1530,6 @@ export class EngineCommandManager extends EventTarget {
return
}
window.addEventListener('offline', this.onOffline)
let additionalSettings = this.settings.enableSSAO ? '&post_effect=ssao' : ''
additionalSettings +=
'&show_grid=' + (this.settings.showScaleGrid ? 'true' : 'false')
@ -1537,6 +1556,11 @@ export class EngineCommandManager extends EventTarget {
this.onEngineConnectionRestartRequest as EventListener
)
this.engineConnection.addEventListener(
EngineConnectionEvents.Offline,
this.onEngineOffline as EventListener
)
// eslint-disable-next-line @typescript-eslint/no-misused-promises
this.onEngineConnectionOpened = async () => {
console.log('onEngineConnectionOpened')
@ -1552,9 +1576,9 @@ export class EngineCommandManager extends EventTarget {
// Let's restart.
console.warn("shit's gone south")
console.warn(e)
this.engineConnection?.dispatchEvent(
new CustomEvent(EngineConnectionEvents.RestartRequest, {})
)
// this.engineConnection?.dispatchEvent(
// new CustomEvent(EngineConnectionEvents.RestartRequest, {})
// )
return
}
@ -1597,23 +1621,7 @@ export class EngineCommandManager extends EventTarget {
console.log('camControlsCameraChange')
this._camControlsCameraChange()
// We should eventually only have 1 restoral call.
if (this.idleMode) {
await this.sceneInfra?.camControls.restoreRemoteCameraStateAndTriggerSync()
} else {
// NOTE: This code is old. It uses the old hack to restore camera.
console.log('call default_camera_get_settings')
// eslint-disable-next-line @typescript-eslint/no-floating-promises
await this.sendSceneCommand({
// CameraControls subscribes to default_camera_get_settings response events
// firing this at connection ensure the camera's are synced initially
type: 'modeling_cmd_req',
cmd_id: uuidv4(),
cmd: {
type: 'default_camera_get_settings',
},
})
}
await this.sceneInfra?.camControls.restoreRemoteCameraStateAndTriggerSync()
setIsStreamReady(true)
@ -1877,8 +1885,6 @@ export class EngineCommandManager extends EventTarget {
tearDown(opts?: { idleMode: boolean }) {
this.idleMode = opts?.idleMode ?? false
window.removeEventListener('offline', this.onOffline)
if (this.engineConnection) {
for (const [cmdId, pending] of Object.entries(this.pendingCommands)) {
pending.reject([
@ -1928,7 +1934,26 @@ export class EngineCommandManager extends EventTarget {
this.engineCommandManager.engineConnection = null
}
this.engineConnection = undefined
// It is possible all connections never even started, but we still want
// to signal to the whole application we are "offline".
this.dispatchEvent(new CustomEvent(EngineCommandManagerEvents.Offline, {}))
}
offline() {
this.keepForcefulOffline = true
this.tearDown()
console.log('offline')
}
online() {
this.keepForcefulOffline = false
this.dispatchEvent(
new CustomEvent(EngineCommandManagerEvents.EngineRestartRequest, {})
)
console.log('online')
}
async startNewSession() {
this.responseMap = {}
}

3
src/lib/selections.ts
View File

@ -703,7 +703,8 @@ export async function sendSelectEventToEngine(
cmd_id: uuidv4(),
})
if (!res) {
return Promise.reject('no response')
console.warn('No response')
return undefined
}
if (isArray(res)) {

30
src/machines/engineStreamMachine.ts
View File

@ -70,7 +70,6 @@ export async function holdOntoVideoFrameInCanvas(
video: HTMLVideoElement,
canvas: HTMLCanvasElement
) {
video.pause()
canvas.width = video.videoWidth
canvas.height = video.videoHeight
canvas.style.width = video.videoWidth + 'px'
@ -220,11 +219,14 @@ export const engineStreamMachine = setup({
if (context.videoRef.current && context.canvasRef.current) {
await context.videoRef.current.pause()
await holdOntoVideoFrameInCanvas(
context.videoRef.current,
context.canvasRef.current
)
context.videoRef.current.style.display = 'none'
// It's possible we've already frozen the frame due to a disconnect.
if (context.videoRef.current.style.display !== 'none') {
await holdOntoVideoFrameInCanvas(
context.videoRef.current,
context.canvasRef.current
)
context.videoRef.current.style.display = 'none'
}
}
await rootContext.sceneInfra.camControls.saveRemoteCameraState()
@ -365,9 +367,12 @@ export const engineStreamMachine = setup({
}),
},
on: {
[EngineStreamTransition.StartOrReconfigureEngine]: {
[EngineStreamTransition.Resume]: {
target: EngineStreamState.Resuming,
},
[EngineStreamTransition.Stop]: {
target: EngineStreamState.Stopped,
},
},
},
[EngineStreamState.Stopped]: {
@ -398,12 +403,23 @@ export const engineStreamMachine = setup({
rootContext: args.self.system.get('root').getSnapshot().context,
event: args.event,
}),
// Usually only fails if there was a disconnection mid-way.
onError: [
{
target: EngineStreamState.WaitingForDependencies,
reenter: true,
},
],
},
on: {
// The stream can be paused as it's resuming.
[EngineStreamTransition.Pause]: {
target: EngineStreamState.Paused,
},
// The stream can be stopped as it's resuming.
[EngineStreamTransition.Stop]: {
target: EngineStreamState.Stopped,
},
[EngineStreamTransition.SetMediaStream]: {
target: EngineStreamState.Playing,
actions: [