Handle serial console close event properly (#2246)

* the event is 'close', not 'closed'

* plumb instance state into serial console skeleton

* make a dent in a more robust can't connect message

* handle error case in mediocre fashion, aria-label on scroll buttons

Author: david-crespo

app/api/util.ts

@@ -100,6 +100,8 @@ const instanceActions: Record<string, InstanceState[]> = {
   attachDisk: ['creating', 'stopped'],
   // https://github.com/oxidecomputer/omicron/blob/8f0cbf0/nexus/db-queries/src/db/datastore/network_interface.rs#L482
   updateNic: ['stopped'],
+  // https://github.com/oxidecomputer/omicron/blob/ebcc2acd/nexus/src/app/instance.rs#L1648-L1676
+  serialConsole: ['running', 'rebooting', 'migrating', 'repairing'],
 }
 
 // setting .states is a cute way to make it ergonomic to call the test function

app/components/Terminal.tsx

@@ -106,11 +106,11 @@ export default function Terminal({ ws }: TerminalProps) {
     <>
       <div className="h-full w-[calc(100%-3rem)] text-mono-code" ref={terminalRef} />
       <div className="absolute right-0 top-0 space-y-2 text-secondary">
-        <ScrollButton onClick={() => term?.scrollToTop()}>
-          <DirectionUpIcon />
+        <ScrollButton onClick={() => term?.scrollToTop()} aria-label="Scroll to top">
+          <DirectionUpIcon aria-hidden />
         </ScrollButton>
-        <ScrollButton onClick={() => term?.scrollToBottom()}>
-          <DirectionDownIcon />
+        <ScrollButton onClick={() => term?.scrollToBottom()} aria-label="Scroll to bottom">
+          <DirectionDownIcon aria-hidden />
         </ScrollButton>
       </div>
     </>

app/pages/project/instances/instance/SerialConsolePage.tsx

@@ -5,14 +5,22 @@
  *
  * Copyright Oxide Computer Company
  */
+import cn from 'classnames'
 import { lazy, Suspense, useEffect, useRef, useState } from 'react'
-import { Link } from 'react-router-dom'
-
-import { api } from '@oxide/api'
+import { Link, type LoaderFunctionArgs } from 'react-router-dom'
+
+import {
+  api,
+  apiQueryClient,
+  instanceCan,
+  usePrefetchedApiQuery,
+  type InstanceState,
+} from '@oxide/api'
 import { PrevArrow12Icon } from '@oxide/design-system/icons/react'
 
 import { EquivalentCliCommand } from '~/components/EquivalentCliCommand'
-import { useInstanceSelector } from '~/hooks'
+import { InstanceStatusBadge } from '~/components/StatusBadge'
+import { getInstanceSelector, useInstanceSelector } from '~/hooks/use-params'
 import { Badge, type BadgeColor } from '~/ui/lib/Badge'
 import { Spinner } from '~/ui/lib/Spinner'
 import { cliCmd } from '~/util/cli-cmd'
@@ -36,13 +44,29 @@ const statusMessage: Record<WsState, string> = {
   error: 'error',
 }
 
+SerialConsolePage.loader = async ({ params }: LoaderFunctionArgs) => {
+  const { project, instance } = getInstanceSelector(params)
+  await apiQueryClient.prefetchQuery('instanceView', {
+    path: { instance },
+    query: { project },
+  })
+  return null
+}
+
 export function SerialConsolePage() {
   const instanceSelector = useInstanceSelector()
   const { project, instance } = instanceSelector
 
+  const { data: instanceData } = usePrefetchedApiQuery('instanceView', {
+    query: { project },
+    path: { instance },
+  })
+
   const ws = useRef<WebSocket | null>(null)
 
-  const [connectionStatus, setConnectionStatus] = useState<WsState>('connecting')
+  const canConnect = instanceCan.serialConsole(instanceData)
+  const initialState = canConnect ? 'connecting' : 'closed'
+  const [connectionStatus, setConnectionStatus] = useState<WsState>(initialState)
 
   // In dev, React 18 strict mode fires all effects twice for lulz, even ones
   // with no dependencies. In order to prevent the websocket from being killed
@@ -54,6 +78,8 @@ export function SerialConsolePage() {
   // 1a. cleanup runs, nothing happens because socket was not open yet
   // 2.  effect runs, but `ws.current` is truthy, so nothing happens
   useEffect(() => {
+    if (!canConnect) return
+
     // TODO: error handling if this connection fails
     if (!ws.current) {
       const { project, instance } = instanceSelector
@@ -70,7 +96,7 @@ export function SerialConsolePage() {
         ws.current.close()
       }
     }
-  }, [instanceSelector])
+  }, [instanceSelector, canConnect])
 
   // Because this one does not look at ready state, just whether the thing is
   // defined, it will remove the event listeners before the spurious second
@@ -81,20 +107,22 @@ export function SerialConsolePage() {
   // 1a. cleanup runs, event listeners removed
   // 2.  effect runs again, event listeners attached again
   useEffect(() => {
+    if (!canConnect) return // don't bother if instance is not running
+
     const setOpen = () => setConnectionStatus('open')
     const setClosed = () => setConnectionStatus('closed')
     const setError = () => setConnectionStatus('error')
 
     ws.current?.addEventListener('open', setOpen)
-    ws.current?.addEventListener('closed', setClosed)
+    ws.current?.addEventListener('close', setClosed)
     ws.current?.addEventListener('error', setError)
 
     return () => {
       ws.current?.removeEventListener('open', setOpen)
-      ws.current?.removeEventListener('closed', setClosed)
+      ws.current?.removeEventListener('close', setClosed)
       ws.current?.removeEventListener('error', setError)
     }
-  }, [])
+  }, [canConnect])
 
   return (
     <div className="!mx-0 flex h-full max-h-[calc(100vh-60px)] !w-full flex-col">
@@ -109,7 +137,13 @@ export function SerialConsolePage() {
       </Link>
 
       <div className="gutter relative w-full shrink grow overflow-hidden">
-        {connectionStatus !== 'open' && <SerialSkeleton />}
+        {connectionStatus === 'connecting' && <ConnectingSkeleton />}
+        {connectionStatus === 'error' && <ErrorSkeleton />}
+        {connectionStatus === 'closed' && !canConnect && (
+          <CannotConnect instanceState={instanceData.runState} />
+        )}
+        {/* closed && canConnect shouldn't be possible because there's no way to
+         * close an open connection other than leaving the page */}
         <Suspense fallback={null}>{ws.current && <Terminal ws={ws.current} />}</Suspense>
       </div>
       <div className="shrink-0 justify-between overflow-hidden border-t bg-default border-secondary empty:border-t-0">
@@ -127,16 +161,22 @@ export function SerialConsolePage() {
   )
 }
 
-function SerialSkeleton() {
-  const instanceSelector = useInstanceSelector()
-
+function SerialSkeleton({
+  children,
+  connecting,
+}: {
+  children: React.ReactNode
+  connecting?: boolean
+}) {
   return (
     <div className="relative h-full shrink grow overflow-hidden">
       <div className="h-full space-y-2 overflow-hidden">
         {[...Array(200)].map((_e, i) => (
           <div
             key={i}
-            className="h-4 rounded bg-tertiary motion-safe:animate-pulse"
+            className={cn('h-4 rounded bg-tertiary', {
+              'motion-safe:animate-pulse': connecting,
+            })}
             style={{
               width: `${Math.sin(Math.sin(i)) * 20 + 40}%`,
             }} /* this is silly deterministic way to get random looking lengths */
@@ -150,18 +190,56 @@ function SerialSkeleton() {
           background: 'linear-gradient(180deg, rgba(8, 15, 17, 0) 0%, #080F11 100%)',
         }}
       />
-      <div className="absolute left-1/2 top-1/2 flex w-96 -translate-x-1/2 -translate-y-1/2 flex-col items-center justify-center space-y-4 rounded-lg border p-12 !bg-raise border-secondary elevation-3">
-        <Spinner size="lg" />
-
-        <div className="space-y-2">
-          <p className="text-center text-sans-xl text-default">
-            Connecting to{' '}
-            <Link to={pb.instance(instanceSelector)} className="text-accent-secondary">
-              {instanceSelector.instance}
-            </Link>
-          </p>
-        </div>
+      <div className="absolute left-1/2 top-1/2 flex w-96 -translate-x-1/2 -translate-y-1/2 flex-col items-center justify-center rounded-lg border p-12 !bg-raise border-secondary elevation-3">
+        {children}
       </div>
     </div>
   )
 }
+
+function InstanceLink() {
+  const { instance, project } = useInstanceSelector()
+  return (
+    <Link
+      className="text-sans-xl text-accent-secondary hover:text-accent"
+      to={pb.instance({ project, instance })}
+    >
+      {instance}
+    </Link>
+  )
+}
+
+const ConnectingSkeleton = () => (
+  <SerialSkeleton connecting>
+    <Spinner size="lg" />
+    <div className="mt-4 text-center">
+      <p className="text-sans-xl">Connecting to</p>
+      <InstanceLink />
+    </div>
+  </SerialSkeleton>
+)
+
+const CannotConnect = ({ instanceState }: { instanceState: InstanceState }) => (
+  <SerialSkeleton>
+    <p className="flex items-center justify-center text-sans-xl">
+      <span>
+        Instance <InstanceLink /> is
+      </span>
+      <InstanceStatusBadge className="ml-1" status={instanceState} />
+    </p>
+    <p className="mt-2 text-center text-secondary">
+      You can only connect to the serial console on a running instance.
+    </p>
+  </SerialSkeleton>
+)
+
+// TODO: sure would be nice to say something useful about the error, but
+// we don't know what kind of thing we might pull off the error event
+const ErrorSkeleton = () => (
+  <SerialSkeleton>
+    <p className="flex items-center justify-center text-center text-sans-xl">
+      Serial console connection failed
+    </p>
+    <p className="mt-2 text-center text-secondary">Please try again.</p>
+  </SerialSkeleton>
+)

app/routes.tsx

@@ -276,6 +276,7 @@ export const routes = createRoutesFromElements(
           <Route path=":instance" handle={{ crumb: instanceCrumb }}>
             <Route
               path="serial-console"
+              loader={SerialConsolePage.loader}
               element={<SerialConsolePage />}
               handle={{ crumb: 'Serial Console' }}
             />

app/util/path-builder.ts

@@ -41,6 +41,7 @@ export const pb = {
 
   instances: (params: Project) => `${projectBase(params)}/instances`,
   instancesNew: (params: Project) => `${projectBase(params)}/instances-new`,
+  /** Don't link directly to this. Use instancePage instead. */
   instance: (params: Instance) => `${pb.instances(params)}/${params.instance}`,
 
   /**