Revert "Fix the black screen of death" (#7292)

Revert "Fix the black screen of death (#7238)"

This reverts commit 46b6707e3a.

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

@@ -61,7 +61,7 @@ test.describe('Test network related behaviors', () => {
       })
 
       // Expect the network to be down
-      await expect(networkToggle).toContainText('Network health (Offline)')
+      await expect(networkToggle).toContainText('Problem')
 
       // Click the network widget
       await networkWidget.click()
@@ -156,8 +156,7 @@ 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(

e2e/playwright/test-utils.ts

@@ -364,6 +364,11 @@ 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),
@@ -484,9 +489,15 @@ export async function getUtils(page: Page, test_?: typeof test) {
     emulateNetworkConditions: async (
       networkOptions: Protocol.Network.emulateNetworkConditionsParameters
     ) => {
-      return networkOptions.offline
+      if (cdpSession === null) {
-        ? page.evaluate('window.engineCommandManager.offline()')
+        // Use a fail safe if we can't simulate disconnect (on Safari)
-        : page.evaluate('window.engineCommandManager.online()')
+        return page.evaluate('window.engineCommandManager.tearDown()')
+      }
+
+      return cdpSession?.send(
+        'Network.emulateNetworkConditions',
+        networkOptions
+      )
     },
 
     toNormalizedCode(text: string) {

src/App.tsx

@@ -114,9 +114,6 @@ 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.

src/clientSideScene/sceneEntities.ts

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

src/components/EngineStream.tsx

@@ -1,3 +1,4 @@
+import { isPlaywright } from '@src/lib/isPlaywright'
 import { useAppState } from '@src/AppState'
 import { ClientSideScene } from '@src/clientSideScene/ClientSideSceneComp'
 import { ViewControlContextMenu } from '@src/components/ViewControlMenu'
@@ -51,10 +52,8 @@ export const EngineStream = (props: {
   const last = useRef<number>(Date.now())
 
   const [firstPlay, setFirstPlay] = useState(true)
-  const [goRestart, setGoRestart] = useState(false)
+  const [isRestartRequestStarting, setIsRestartRequestStarting] =
-  const [timeoutId, setTimeoutId] = useState<
+    useState(false)
-    ReturnType<typeof setTimeout> | undefined
-  >(undefined)
   const [attemptTimes, setAttemptTimes] = useState<[number, number]>([
     0,
     TIME_1_SECOND,
@@ -86,21 +85,18 @@ 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, engineStreamState])
+  }, [videoRef.current])
 
   useEffect(() => {
-    if (engineStreamState.context.canvasRef.current !== null) return
     engineStreamActor.send({
       type: EngineStreamTransition.SetCanvasRef,
       canvasRef: { current: canvasRef.current },
     })
-  }, [canvasRef.current, engineStreamState])
+  }, [canvasRef.current])
 
   useEffect(() => {
     engineStreamActor.send({
@@ -135,6 +131,24 @@ 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({
@@ -160,13 +174,12 @@ export const EngineStream = (props: {
     console.log('scene is ready, execute kcl')
     const kmp = kclManager.executeCode().catch(trap)
 
-    // Reset the restart timeouts
-    setAttemptTimes([0, TIME_1_SECOND])
-
-    console.log(firstPlay)
     if (!firstPlay) return
 
     setFirstPlay(false)
+    // Reset the restart timeouts
+    setAttemptTimes([0, TIME_1_SECOND])
+
     console.log('firstPlay true, zoom to fit')
     kmp
       .then(async () => {
@@ -198,76 +211,51 @@ 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 = () => {
-      // Timeout already set.
+      if (isRestartRequestStarting) return
-      if (timeoutId) return
+      setIsRestartRequestStarting(true)
-
+      setTimeout(() => {
-      setTimeoutId(
+        engineStreamState.context.videoRef.current?.pause()
-        setTimeout(() => {
+        engineCommandManager.tearDown()
-          engineStreamState.context.videoRef.current?.pause()
+        startOrReconfigureEngine()
-          engineCommandManager.tearDown()
+        setFirstPlay(false)
-          startOrReconfigureEngine()
+        setIsRestartRequestStarting(false)
-          setFirstPlay(true)
+      }, attemptTimes[0] + attemptTimes[1])
-
-          setTimeoutId(undefined)
-          setGoRestart(false)
-        }, attemptTimes[0] + attemptTimes[1])
-      )
       setAttemptTimes([attemptTimes[1], attemptTimes[0] + attemptTimes[1]])
     }
 
-    const onOffline = () => {
+    // Poll that we're connected. If not, send a reset signal.
-      if (
+    // Do not restart if we're in idle mode.
-        !(
+    const connectionCheckIntervalId = setInterval(() => {
-          EngineConnectionStateType.Disconnected ===
+      // SKIP DURING TESTS BECAUSE IT WILL MESS WITH REUSING THE
-            engineCommandManager.engineConnection?.state.type ||
+      // ELECTRON INSTANCE.
-          EngineConnectionStateType.Disconnecting ===
+      if (isPlaywright()) {
-            engineCommandManager.engineConnection?.state.type
-        )
-      ) {
         return
       }
-      engineStreamActor.send({ type: EngineStreamTransition.Stop })
-      attemptRestartIfNecessary()
-    }
 
-    if (
+      // Don't try try to restart if we're already connected!
-      !goRestart &&
+      const hasEngineConnectionInst = engineCommandManager.engineConnection
-      engineStreamState.value === EngineStreamState.WaitingForDependencies
+      const isDisconnected =
-    ) {
+        engineCommandManager.engineConnection?.state.type ===
-      setGoRestart(true)
+        EngineConnectionStateType.Disconnected
-    }
+      const inIdleMode = engineStreamState.value === EngineStreamState.Paused
+      if ((hasEngineConnectionInst && !isDisconnected) || inIdleMode) return
 
-    if (goRestart && !timeoutId) {
       attemptRestartIfNecessary()
-    }
+    }, TIME_1_SECOND)
 
     engineCommandManager.addEventListener(
       EngineCommandManagerEvents.EngineRestartRequest,
       attemptRestartIfNecessary
     )
-
-    engineCommandManager.addEventListener(
-      EngineCommandManagerEvents.Offline,
-      onOffline
-    )
-
     return () => {
+      clearInterval(connectionCheckIntervalId)
+
       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.
@@ -281,7 +269,7 @@ export const EngineStream = (props: {
     const canvas = engineStreamState.context.canvasRef?.current
     if (!canvas) return
 
-    const observer = new ResizeObserver(() => {
+    new ResizeObserver(() => {
       // Prevents:
       // `Uncaught ResizeObserver loop completed with undelivered notifications`
       window.requestAnimationFrame(() => {
@@ -292,19 +280,13 @@ export const EngineStream = (props: {
         if (
           (Math.abs(video.width - window.innerWidth) > 4 ||
             Math.abs(video.height - window.innerHeight) > 4) &&
-          engineStreamState.matches(EngineStreamState.Playing)
+          !engineStreamState.matches(EngineStreamState.WaitingToPlay)
         ) {
           timeoutStart.current = Date.now()
           startOrReconfigureEngine()
         }
       })
-    })
+    }).observe(document.body)
-
-    observer.observe(document.body)
-
-    return () => {
-      observer.disconnect()
-    }
   }, [engineStreamState.value])
 
   /**
@@ -363,21 +345,8 @@ export const EngineStream = (props: {
         timeoutStart.current = null
       } else if (timeoutStart.current) {
         const elapsed = Date.now() - timeoutStart.current
-        // Don't pause if we're already disconnected.
+        if (elapsed >= IDLE_TIME_MS) {
-        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 })
         }
       }
@@ -388,7 +357,7 @@ export const EngineStream = (props: {
     return () => {
       window.cancelAnimationFrame(frameId)
     }
-  }, [modelingMachineState, engineStreamState.value])
+  }, [modelingMachineState])
 
   useEffect(() => {
     if (!streamIdleMode) return
@@ -401,18 +370,9 @@ export const EngineStream = (props: {
         return
       }
 
-      engineStreamActor.send({
+      if (engineStreamState.value === EngineStreamState.Paused) {
-        type: EngineStreamTransition.Resume,
+        startOrReconfigureEngine()
-        modelingMachineActorSend,
+      }
-        settings: settingsEngine,
-        setAppState,
-        onMediaStream(mediaStream: MediaStream) {
-          engineStreamActor.send({
-            type: EngineStreamTransition.SetMediaStream,
-            mediaStream,
-          })
-        },
-      })
 
       timeoutStart.current = Date.now()
     }
@@ -511,11 +471,7 @@ export const EngineStream = (props: {
     }
 
     sendSelectEventToEngine(e)
-      .then((result) => {
+      .then(({ entity_id }) => {
-        if (!result) {
-          return
-        }
-        const { entity_id } = result
         if (!entity_id) {
           // No entity selected. This is benign
           return

src/hooks/useNetworkStatus.tsx

@@ -78,19 +78,18 @@ export function useNetworkStatus() {
   }, [hasIssues, internetConnected, ping])
 
   useEffect(() => {
+    const onlineCallback = () => {
+      setInternetConnected(true)
+    }
     const offlineCallback = () => {
       setInternetConnected(false)
       setSteps(structuredClone(initialConnectingTypeGroupState))
     }
-    engineCommandManager.addEventListener(
+    window.addEventListener('online', onlineCallback)
-      EngineCommandManagerEvents.Offline,
+    window.addEventListener('offline', offlineCallback)
-      offlineCallback
-    )
     return () => {
-      engineCommandManager.removeEventListener(
+      window.removeEventListener('online', onlineCallback)
-        EngineCommandManagerEvents.Offline,
+      window.removeEventListener('offline', offlineCallback)
-        offlineCallback
-      )
     }
   }, [])
 
@@ -140,8 +139,6 @@ 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) {
@@ -171,10 +168,6 @@ export function useNetworkStatus() {
 
             if (engineConnectionState.value.type === DisconnectingType.Error) {
               setError(engineConnectionState.value.value)
-            } else if (
-              engineConnectionState.value.type === DisconnectingType.Quit
-            ) {
-              return structuredClone(initialConnectingTypeGroupState)
             }
           }
         }

src/lang/std/engineConnection.ts

@@ -1,3 +1,4 @@
+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'
@@ -82,9 +83,6 @@ 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,
@@ -109,8 +107,6 @@ 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.',
 }
@@ -230,9 +226,6 @@ 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(
@@ -681,20 +674,9 @@ 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.Offline, {})
+                new CustomEvent(EngineConnectionEvents.RestartRequest, {})
               )
-              this.disconnectAll()
               break
             case 'closed':
               this.pc?.removeEventListener('icecandidate', this.onIceCandidate)
@@ -865,6 +847,7 @@ class EngineConnection extends EventTarget {
               'message',
               this.onDataChannelMessage
             )
+            this.disconnectAll()
           }
 
           this.unreliableDataChannel?.addEventListener(
@@ -883,6 +866,7 @@ class EngineConnection extends EventTarget {
                 },
               },
             }
+            this.disconnectAll()
           }
           this.unreliableDataChannel?.addEventListener(
             'error',
@@ -972,9 +956,6 @@ class EngineConnection extends EventTarget {
             this.onNetworkStatusReady
           )
 
-          this.dispatchEvent(
-            new CustomEvent(EngineConnectionEvents.Offline, {})
-          )
           this.disconnectAll()
         }
         this.websocket.addEventListener('close', this.onWebSocketClose)
@@ -993,6 +974,8 @@ class EngineConnection extends EventTarget {
               },
             }
           }
+
+          this.disconnectAll()
         }
         this.websocket.addEventListener('error', this.onWebSocketError)
 
@@ -1348,9 +1331,6 @@ export enum EngineCommandManagerEvents {
 
   // the whole scene is ready (settings loaded)
   SceneReady = 'scene-ready',
-
-  // we're offline
-  Offline = 'offline',
 }
 
 /**
@@ -1400,7 +1380,6 @@ 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[] = []
@@ -1474,8 +1453,13 @@ export class EngineCommandManager extends EventTarget {
     )
   }
 
-  private onEngineOffline = () => {
+  private onOffline = () => {
-    this.dispatchEvent(new CustomEvent(EngineCommandManagerEvents.Offline, {}))
+    console.log('Browser reported network is offline')
+    if (TEST) {
+      console.warn('DURING TESTS ENGINECONNECTION.ONOFFLINE WILL DO NOTHING.')
+      return
+    }
+    this.onEngineConnectionRestartRequest()
   }
 
   idleMode: boolean = false
@@ -1510,11 +1494,6 @@ export class EngineCommandManager extends EventTarget {
     if (settings) {
       this.settings = settings
     }
-
-    if (this.keepForcefulOffline) {
-      return
-    }
-
     if (width === 0 || height === 0) {
       return
     }
@@ -1530,6 +1509,8 @@ 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')
@@ -1556,11 +1537,6 @@ 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')
@@ -1576,9 +1552,9 @@ export class EngineCommandManager extends EventTarget {
         // Let's restart.
         console.warn("shit's gone south")
         console.warn(e)
-        // this.engineConnection?.dispatchEvent(
+        this.engineConnection?.dispatchEvent(
-        //   new CustomEvent(EngineConnectionEvents.RestartRequest, {})
+          new CustomEvent(EngineConnectionEvents.RestartRequest, {})
-        // )
+        )
         return
       }
 
@@ -1621,7 +1597,23 @@ export class EngineCommandManager extends EventTarget {
       console.log('camControlsCameraChange')
       this._camControlsCameraChange()
 
-      await this.sceneInfra?.camControls.restoreRemoteCameraStateAndTriggerSync()
+      // 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',
+          },
+        })
+      }
 
       setIsStreamReady(true)
 
@@ -1885,6 +1877,8 @@ 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([
@@ -1934,26 +1928,7 @@ 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 = {}
   }

src/lib/selections.ts

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

src/machines/engineStreamMachine.ts

@@ -70,6 +70,7 @@ export async function holdOntoVideoFrameInCanvas(
   video: HTMLVideoElement,
   canvas: HTMLCanvasElement
 ) {
+  video.pause()
   canvas.width = video.videoWidth
   canvas.height = video.videoHeight
   canvas.style.width = video.videoWidth + 'px'
@@ -219,14 +220,11 @@ export const engineStreamMachine = setup({
         if (context.videoRef.current && context.canvasRef.current) {
           await context.videoRef.current.pause()
 
-          // It's possible we've already frozen the frame due to a disconnect.
+          await holdOntoVideoFrameInCanvas(
-          if (context.videoRef.current.style.display !== 'none') {
+            context.videoRef.current,
-            await holdOntoVideoFrameInCanvas(
+            context.canvasRef.current
-              context.videoRef.current,
+          )
-              context.canvasRef.current
+          context.videoRef.current.style.display = 'none'
-            )
-            context.videoRef.current.style.display = 'none'
-          }
         }
 
         await rootContext.sceneInfra.camControls.saveRemoteCameraState()
@@ -367,12 +365,9 @@ export const engineStreamMachine = setup({
         }),
       },
       on: {
-        [EngineStreamTransition.Resume]: {
+        [EngineStreamTransition.StartOrReconfigureEngine]: {
           target: EngineStreamState.Resuming,
         },
-        [EngineStreamTransition.Stop]: {
-          target: EngineStreamState.Stopped,
-        },
       },
     },
     [EngineStreamState.Stopped]: {
@@ -403,23 +398,12 @@ 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: [