From 14a03e86f495983b1a11ef0fe450a6640ab98105 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 15 Mar 2023 10:58:36 -0500 Subject: [PATCH 01/12] experiment with websocket serial console endpoint --- app/components/Terminal.tsx | 18 +++++----- .../instances/instance/SerialConsolePage.tsx | 36 +++++++------------ package-lock.json | 15 ++++++++ package.json | 1 + 4 files changed, 37 insertions(+), 33 deletions(-) diff --git a/app/components/Terminal.tsx b/app/components/Terminal.tsx index 132607c1c..ef50bce43 100644 --- a/app/components/Terminal.tsx +++ b/app/components/Terminal.tsx @@ -1,6 +1,7 @@ import { useEffect, useRef, useState } from 'react' import type { ITerminalOptions } from 'xterm' import { Terminal as XTerm } from 'xterm' +import { AttachAddon } from 'xterm-addon-attach' import { FitAddon } from 'xterm-addon-fit' import 'xterm/css/xterm.css' @@ -8,7 +9,7 @@ import { DirectionDownIcon, DirectionUpIcon } from '@oxide/ui' import { classed } from '@oxide/util' interface TerminalProps { - data?: number[] + wsUrl: string } const options: ITerminalOptions = { @@ -47,7 +48,7 @@ function getTheme() { } } -export const Terminal = ({ data }: TerminalProps) => { +export const Terminal = ({ wsUrl }: TerminalProps) => { const [term, setTerm] = useState(null) const terminalRef = useRef(null) @@ -64,6 +65,11 @@ export const Terminal = ({ data }: TerminalProps) => { const fitAddon = new FitAddon() newTerm.loadAddon(fitAddon) + // TODO: error handling if this connection fails + const ws = new WebSocket(wsUrl) + const attachAddon = new AttachAddon(ws) + newTerm.loadAddon(attachAddon) + // Handle window resizing const resize = () => fitAddon.fit() @@ -71,18 +77,12 @@ export const Terminal = ({ data }: TerminalProps) => { window.addEventListener('resize', resize) return () => { newTerm.dispose() + ws.close() // TODO: is this necessary window.removeEventListener('resize', resize) } // eslint-disable-next-line react-hooks/exhaustive-deps }, []) - useEffect(() => { - if (data) { - term?.clear() - term?.write(new Uint8Array(data)) - } - }, [term, data]) - return ( <>
diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index 8a91903b4..37dcf2962 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -1,10 +1,8 @@ import { Suspense, lazy } from 'react' import { Link } from 'react-router-dom' -import { useApiQuery } from '@oxide/api' import { Button } from '@oxide/ui' import { DirectionLeftIcon, Spinner } from '@oxide/ui' -import { MiB } from '@oxide/util' import { SerialConsoleStatusBadge } from 'app/components/StatusBadge' import { useInstanceSelector } from 'app/hooks' @@ -15,15 +13,14 @@ const Terminal = lazy(() => import('app/components/Terminal')) export function SerialConsolePage() { const { organization, project, instance } = useInstanceSelector() - const { isRefetching, data, refetch } = useApiQuery( - 'instanceSerialConsoleV1', - { - path: { instance }, - // holding off on using toPathQuery for now because it doesn't like numbers - query: { organization, project, maxBytes: 10 * MiB, fromStart: 0 }, - }, - { refetchOnWindowFocus: false } - ) + // TODO: might want to initialize websocket here and pass it into the terminal + // instead so we can do error handling and status management here + + // TODO: janky to do this url construction here. maybe we can have oxide.ts + // detect websocket endpoints and give us a helper that constructs a WebSocket + // instead of calling fetch + const wsUrl = `ws://${window.location.host}/v1/instances/${instance}/serial-console/stream?organization=${organization}&project=${project}` + const status = 'connected' // TODO return (
@@ -38,29 +35,20 @@ export function SerialConsolePage() {
- {!data && } + {status !== 'connected' && } - +
- - -
- +
diff --git a/package-lock.json b/package-lock.json index aa0613614..34272970f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -46,6 +46,7 @@ "tunnel-rat": "^0.0.4", "uuid": "^8.3.2", "xterm": "^5.1.0", + "xterm-addon-attach": "^0.8.0", "xterm-addon-fit": "^0.7.0", "zod": "^3.19.1", "zustand": "^4.0.0" @@ -20905,6 +20906,14 @@ "resolved": "https://registry.npmjs.org/xterm/-/xterm-5.1.0.tgz", "integrity": "sha512-LovENH4WDzpwynj+OTkLyZgJPeDom9Gra4DMlGAgz6pZhIDCQ+YuO7yfwanY+gVbn/mmZIStNOnVRU/ikQuAEQ==" }, + "node_modules/xterm-addon-attach": { + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/xterm-addon-attach/-/xterm-addon-attach-0.8.0.tgz", + "integrity": "sha512-k8N5boSYn6rMJTTNCgFpiSTZ26qnYJf3v/nJJYexNO2sdAHDN3m1ivVQWVZ8CHJKKnZQw1rc44YP2NtgalWHfQ==", + "peerDependencies": { + "xterm": "^5.0.0" + } + }, "node_modules/xterm-addon-fit": { "version": "0.7.0", "resolved": "https://registry.npmjs.org/xterm-addon-fit/-/xterm-addon-fit-0.7.0.tgz", @@ -36121,6 +36130,12 @@ "resolved": "https://registry.npmjs.org/xterm/-/xterm-5.1.0.tgz", "integrity": "sha512-LovENH4WDzpwynj+OTkLyZgJPeDom9Gra4DMlGAgz6pZhIDCQ+YuO7yfwanY+gVbn/mmZIStNOnVRU/ikQuAEQ==" }, + "xterm-addon-attach": { + "version": "0.8.0", + "resolved": "https://registry.npmjs.org/xterm-addon-attach/-/xterm-addon-attach-0.8.0.tgz", + "integrity": "sha512-k8N5boSYn6rMJTTNCgFpiSTZ26qnYJf3v/nJJYexNO2sdAHDN3m1ivVQWVZ8CHJKKnZQw1rc44YP2NtgalWHfQ==", + "requires": {} + }, "xterm-addon-fit": { "version": "0.7.0", "resolved": "https://registry.npmjs.org/xterm-addon-fit/-/xterm-addon-fit-0.7.0.tgz", diff --git a/package.json b/package.json index 764307b62..f32e5f4e7 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "tunnel-rat": "^0.0.4", "uuid": "^8.3.2", "xterm": "^5.1.0", + "xterm-addon-attach": "^0.8.0", "xterm-addon-fit": "^0.7.0", "zod": "^3.19.1", "zustand": "^4.0.0" From 962f7a17dfb3e17f00a2ad2db455eefefc7b9c84 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 15 Mar 2023 11:18:13 -0500 Subject: [PATCH 02/12] move websocket constructor up a level --- app/components/Terminal.tsx | 7 ++--- .../instances/instance/SerialConsolePage.tsx | 28 +++++++++++-------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/app/components/Terminal.tsx b/app/components/Terminal.tsx index ef50bce43..3def6eff3 100644 --- a/app/components/Terminal.tsx +++ b/app/components/Terminal.tsx @@ -9,7 +9,7 @@ import { DirectionDownIcon, DirectionUpIcon } from '@oxide/ui' import { classed } from '@oxide/util' interface TerminalProps { - wsUrl: string + ws: WebSocket } const options: ITerminalOptions = { @@ -48,7 +48,7 @@ function getTheme() { } } -export const Terminal = ({ wsUrl }: TerminalProps) => { +export const Terminal = ({ ws }: TerminalProps) => { const [term, setTerm] = useState(null) const terminalRef = useRef(null) @@ -65,8 +65,6 @@ export const Terminal = ({ wsUrl }: TerminalProps) => { const fitAddon = new FitAddon() newTerm.loadAddon(fitAddon) - // TODO: error handling if this connection fails - const ws = new WebSocket(wsUrl) const attachAddon = new AttachAddon(ws) newTerm.loadAddon(attachAddon) @@ -77,7 +75,6 @@ export const Terminal = ({ wsUrl }: TerminalProps) => { window.addEventListener('resize', resize) return () => { newTerm.dispose() - ws.close() // TODO: is this necessary window.removeEventListener('resize', resize) } // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index 37dcf2962..b599bfdd3 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -1,4 +1,4 @@ -import { Suspense, lazy } from 'react' +import { Suspense, lazy, useEffect, useRef } from 'react' import { Link } from 'react-router-dom' import { Button } from '@oxide/ui' @@ -13,14 +13,20 @@ const Terminal = lazy(() => import('app/components/Terminal')) export function SerialConsolePage() { const { organization, project, instance } = useInstanceSelector() - // TODO: might want to initialize websocket here and pass it into the terminal - // instead so we can do error handling and status management here + // unclear if this should be a ref, it could be normal state or even a useMemo + const wsRef = useRef(null) - // TODO: janky to do this url construction here. maybe we can have oxide.ts - // detect websocket endpoints and give us a helper that constructs a WebSocket - // instead of calling fetch - const wsUrl = `ws://${window.location.host}/v1/instances/${instance}/serial-console/stream?organization=${organization}&project=${project}` - const status = 'connected' // TODO + useEffect(() => { + // TODO: janky to do this url construction here. maybe we can have oxide.ts + // detect websocket endpoints and give us a helper that constructs a WebSocket + // instead of calling fetch + // TODO: error handling if this connection fails + const wsUrl = `ws://${window.location.host}/v1/instances/${instance}/serial-console/stream?organization=${organization}&project=${project}` + wsRef.current = new WebSocket(wsUrl) + return () => wsRef.current?.close() + }, [organization, project, instance]) + + const connected = wsRef.current?.readyState === WebSocket.OPEN return (
@@ -35,9 +41,9 @@ export function SerialConsolePage() {
- {status !== 'connected' && } + {!connected && } - + {wsRef.current && }
@@ -48,7 +54,7 @@ export function SerialConsolePage() {
- +
From a9d87c8f660c061777005401d4ecd57316c14f8c Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 15 Mar 2023 12:46:03 -0500 Subject: [PATCH 03/12] proxy ws request through vite dev server properly --- .../instances/instance/SerialConsolePage.tsx | 24 ++++++++++++------- vite.config.ts | 10 ++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index b599bfdd3..b77a9dd25 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -1,6 +1,7 @@ import { Suspense, lazy, useEffect, useRef } from 'react' import { Link } from 'react-router-dom' +import type { PathParams } from '@oxide/api' import { Button } from '@oxide/ui' import { DirectionLeftIcon, Spinner } from '@oxide/ui' @@ -10,28 +11,35 @@ import { pb } from 'app/util/path-builder' const Terminal = lazy(() => import('app/components/Terminal')) +// TODO: janky to do this url construction here. maybe we can have oxide.ts +// detect websocket endpoints and give us a helper that constructs a WebSocket +// instead of calling fetch +function serialConsoleUrl(sel: Required) { + // TODO: is there a better way to get the current host in there (or leave it implicit like with fetch) + const base = 'ws://' + window.location.host + const vitePrefix = process.env.NODE_ENV === 'development' ? '/ws-api' : '' + const route = `/v1/instances/${sel.instance}/serial-console/stream?organization=${sel.organization}&project=${sel.project}` + return base + vitePrefix + route +} + export function SerialConsolePage() { - const { organization, project, instance } = useInstanceSelector() + const instanceSelector = useInstanceSelector() // unclear if this should be a ref, it could be normal state or even a useMemo const wsRef = useRef(null) useEffect(() => { - // TODO: janky to do this url construction here. maybe we can have oxide.ts - // detect websocket endpoints and give us a helper that constructs a WebSocket - // instead of calling fetch // TODO: error handling if this connection fails - const wsUrl = `ws://${window.location.host}/v1/instances/${instance}/serial-console/stream?organization=${organization}&project=${project}` - wsRef.current = new WebSocket(wsUrl) + wsRef.current = new WebSocket(serialConsoleUrl(instanceSelector)) return () => wsRef.current?.close() - }, [organization, project, instance]) + }, [instanceSelector]) const connected = wsRef.current?.readyState === WebSocket.OPEN return (
diff --git a/vite.config.ts b/vite.config.ts index a206efc12..5ec7d5ca5 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -74,6 +74,16 @@ export default defineConfig(({ mode }) => ({ }, rewrite: (path) => path.replace(/^\/api/, ''), }, + '/ws-api': { + target: 'ws://localhost:12220', + ws: true, + configure(proxy) { + proxy.on('error', (_, req) => { + console.error(' to', '/api' + req.url) + }) + }, + rewrite: (path) => path.replace(/^\/ws-api/, ''), + }, // We want to actually hit Nexus for this because it gives us a login redirect '/login': { target: 'http://localhost:12220', From 4c7717911bd314f92c2818eaa3c4ba71de83384a Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 23 Mar 2023 17:09:05 -0500 Subject: [PATCH 04/12] update API to latest main --- OMICRON_VERSION | 2 +- libs/api/__generated__/Api.ts | 3 +++ libs/api/__generated__/OMICRON_VERSION | 2 +- libs/api/__generated__/validate.ts | 3 +++ 4 files changed, 8 insertions(+), 2 deletions(-) diff --git a/OMICRON_VERSION b/OMICRON_VERSION index 69cd6bbd9..6731e121c 100644 --- a/OMICRON_VERSION +++ b/OMICRON_VERSION @@ -1 +1 @@ -a796560cf630b9eef6197c7ab80f5743051714c2 +32dda1acdf0b21589b062010712871cb9126f35f diff --git a/libs/api/__generated__/Api.ts b/libs/api/__generated__/Api.ts index e62e5c558..b67d9a228 100644 --- a/libs/api/__generated__/Api.ts +++ b/libs/api/__generated__/Api.ts @@ -2073,6 +2073,9 @@ export interface InstanceSerialConsoleStreamPathParams { } export interface InstanceSerialConsoleStreamQueryParams { + fromStart?: number + maxBytes?: number + mostRecent?: number project?: NameOrId } diff --git a/libs/api/__generated__/OMICRON_VERSION b/libs/api/__generated__/OMICRON_VERSION index e80026b5f..4b37e6c2d 100644 --- a/libs/api/__generated__/OMICRON_VERSION +++ b/libs/api/__generated__/OMICRON_VERSION @@ -1,2 +1,2 @@ # generated file. do not update manually. see docs/update-pinned-api.md -a796560cf630b9eef6197c7ab80f5743051714c2 +32dda1acdf0b21589b062010712871cb9126f35f diff --git a/libs/api/__generated__/validate.ts b/libs/api/__generated__/validate.ts index 3bb6c5794..5b670cb0d 100644 --- a/libs/api/__generated__/validate.ts +++ b/libs/api/__generated__/validate.ts @@ -2291,6 +2291,9 @@ export const InstanceSerialConsoleStreamParams = z.preprocess( instance: NameOrId, }), query: z.object({ + fromStart: z.number().min(0).optional(), + maxBytes: z.number().min(0).optional(), + mostRecent: z.number().min(0).optional(), project: NameOrId.optional(), }), }) From 343a0fe65f89c9378aa2d0e3ba571b20b6a34cc5 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 23 Mar 2023 23:33:42 -0500 Subject: [PATCH 05/12] get sim sled agent running --- app/pages/project/instances/instance/SerialConsolePage.tsx | 4 +++- tools/start_api.sh | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index 50fe0e620..e39d194ac 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -11,6 +11,8 @@ import { pb } from 'app/util/path-builder' const Terminal = lazy(() => import('app/components/Terminal')) +const pathPrefix = process.env.NODE_ENV === 'development' ? '/ws-api' : '' + export function SerialConsolePage() { const instanceSelector = useInstanceSelector() @@ -20,7 +22,7 @@ export function SerialConsolePage() { useEffect(() => { // TODO: error handling if this connection fails wsRef.current = api.ws.instanceSerialConsoleStream( - window.location.host, + window.location.host + pathPrefix, toPathQuery('instance', instanceSelector) ) return () => wsRef.current?.close() diff --git a/tools/start_api.sh b/tools/start_api.sh index bfe213359..1c74ae03f 100755 --- a/tools/start_api.sh +++ b/tools/start_api.sh @@ -66,7 +66,7 @@ run_in_pane 2 "$UTILS" run_in_pane 2 "set_pane_title nexus" run_in_pane 2 "wait_for_up 32221" # cockroach run_in_pane 2 "cargo run --bin=omicron-dev -- db-populate --database-url postgresql://root@127.0.0.1:32221/omicron" -run_in_pane 2 "cargo run --bin=nexus -- nexus/examples/config.toml" +run_in_pane 2 "SKIP_ASIC_CONFIG=1 cargo run --bin=nexus -- nexus/examples/config.toml" run_in_pane 3 "$UTILS" run_in_pane 3 "set_pane_title sled-agent-sim" From 46a1ed228f257edcd7b9089b6f3ecc4ec06b6173 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 24 Mar 2023 16:05:55 -0500 Subject: [PATCH 06/12] made some progress --- app/components/Terminal.tsx | 9 +++-- .../instances/instance/SerialConsolePage.tsx | 38 +++++++++++++------ 2 files changed, 32 insertions(+), 15 deletions(-) diff --git a/app/components/Terminal.tsx b/app/components/Terminal.tsx index 3def6eff3..dd8f4320f 100644 --- a/app/components/Terminal.tsx +++ b/app/components/Terminal.tsx @@ -57,9 +57,6 @@ export const Terminal = ({ ws }: TerminalProps) => { // Persist terminal instance, initialize terminal setTerm(newTerm) - if (terminalRef.current) { - newTerm.open(terminalRef.current) - } // Setup terminal addons const fitAddon = new FitAddon() @@ -71,7 +68,11 @@ export const Terminal = ({ ws }: TerminalProps) => { // Handle window resizing const resize = () => fitAddon.fit() - resize() + if (terminalRef.current) { + newTerm.open(terminalRef.current) + resize() + } + window.addEventListener('resize', resize) return () => { newTerm.dispose() diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index e39d194ac..5a8e37c09 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -1,17 +1,17 @@ -import { Suspense, lazy, useEffect, useRef } from 'react' +import { Suspense, lazy, useEffect, useRef, useState } from 'react' import { Link } from 'react-router-dom' import { api } from '@oxide/api' import { Button, PrevArrow12Icon, Spinner } from '@oxide/ui' -import { toPathQuery } from '@oxide/util' +// import { toPathQuery } from '@oxide/util' import { SerialConsoleStatusBadge } from 'app/components/StatusBadge' import { useInstanceSelector } from 'app/hooks' import { pb } from 'app/util/path-builder' const Terminal = lazy(() => import('app/components/Terminal')) -const pathPrefix = process.env.NODE_ENV === 'development' ? '/ws-api' : '' +// const pathPrefix = process.env.NODE_ENV === 'development' ? '/ws-api' : '' export function SerialConsolePage() { const instanceSelector = useInstanceSelector() @@ -19,16 +19,30 @@ export function SerialConsolePage() { // unclear if this should be a ref, it could be normal state or even a useMemo const wsRef = useRef(null) + const [connectionStatus, setConnectionStatus] = useState< + 'connecting' | 'open' | 'closed' | 'error' + >('connecting') + useEffect(() => { // TODO: error handling if this connection fails + const { project, instance } = instanceSelector wsRef.current = api.ws.instanceSerialConsoleStream( - window.location.host + pathPrefix, - toPathQuery('instance', instanceSelector) + // window.location.host + pathPrefix, + 'localhost:12220', + { path: { instance }, query: { project, fromStart: 0 } } ) - return () => wsRef.current?.close() - }, [instanceSelector]) - - const connected = wsRef.current?.readyState === WebSocket.OPEN + // TODO: add listeners for other statuses? + wsRef.current.addEventListener('open', () => { + setConnectionStatus('open') + }) + // this is pretty important :| + wsRef.current.binaryType = 'arraybuffer' + // TODO: uh when do we close this + // return () => wsRef.current?.close() + // TODO: why is instanceSelector not stable?!?!?!?!? it's memoized! + // }, [instanceSelector]) + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []) return (
@@ -43,7 +57,7 @@ export function SerialConsolePage() {
- {!connected && } + {connectionStatus !== 'open' && } {wsRef.current && } @@ -56,7 +70,9 @@ export function SerialConsolePage() {
- +
From 776040fc25d6593187565b56dc67ce8f7c477012 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Mon, 27 Mar 2023 17:07:34 -0500 Subject: [PATCH 07/12] mock serial console server in deno --- .vscode/settings.json | 2 +- .../instances/instance/SerialConsolePage.tsx | 15 ++++---- tools/{ => deno}/bump-omicron.ts | 0 tools/deno/mock-serial-console.ts | 37 +++++++++++++++++++ tsconfig.json | 2 +- vite.config.ts | 9 +++-- 6 files changed, 51 insertions(+), 14 deletions(-) rename tools/{ => deno}/bump-omicron.ts (100%) create mode 100755 tools/deno/mock-serial-console.ts diff --git a/.vscode/settings.json b/.vscode/settings.json index 31101dd88..d9890a870 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -19,5 +19,5 @@ "vite.browserType": "system", "vite.buildCommand": "npm run build", "vite.devCommand": "npm run start:msw", - "deno.enablePaths": ["./tools/bump-omicron.ts"] + "deno.enablePaths": ["./tools/deno"] } diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index 5a8e37c09..954404bd7 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -11,7 +11,8 @@ import { pb } from 'app/util/path-builder' const Terminal = lazy(() => import('app/components/Terminal')) -// const pathPrefix = process.env.NODE_ENV === 'development' ? '/ws-api' : '' +// need prefix so Vite dev server can handle it specially +const pathPrefix = process.env.NODE_ENV === 'development' ? '/ws-serial-console' : '' export function SerialConsolePage() { const instanceSelector = useInstanceSelector() @@ -26,19 +27,17 @@ export function SerialConsolePage() { useEffect(() => { // TODO: error handling if this connection fails const { project, instance } = instanceSelector - wsRef.current = api.ws.instanceSerialConsoleStream( - // window.location.host + pathPrefix, - 'localhost:12220', - { path: { instance }, query: { project, fromStart: 0 } } - ) + wsRef.current = api.ws.instanceSerialConsoleStream(window.location.host + pathPrefix, { + path: { instance }, + query: { project, fromStart: 0 }, + }) // TODO: add listeners for other statuses? wsRef.current.addEventListener('open', () => { setConnectionStatus('open') }) // this is pretty important :| wsRef.current.binaryType = 'arraybuffer' - // TODO: uh when do we close this - // return () => wsRef.current?.close() + return () => wsRef.current?.close() // TODO: why is instanceSelector not stable?!?!?!?!? it's memoized! // }, [instanceSelector]) // eslint-disable-next-line react-hooks/exhaustive-deps diff --git a/tools/bump-omicron.ts b/tools/deno/bump-omicron.ts similarity index 100% rename from tools/bump-omicron.ts rename to tools/deno/bump-omicron.ts diff --git a/tools/deno/mock-serial-console.ts b/tools/deno/mock-serial-console.ts new file mode 100755 index 000000000..aadba9720 --- /dev/null +++ b/tools/deno/mock-serial-console.ts @@ -0,0 +1,37 @@ +#! /usr/bin/env -S deno run --allow-run --allow-net +import { delay } from 'https://deno.land/std@0.181.0/async/delay.ts' +import { serve } from 'https://deno.land/std@0.181.0/http/server.ts' + +/* + * This exists because MSW does not support websockets. So in MSW mode, we also + * run this little server and configure Vite to proxy WS requests to it. + */ + +async function streamString(socket: WebSocket, s: string, delayMs = 50) { + for (const c of s) { + socket.send(new TextEncoder().encode(c)) + await delay(delayMs) + } +} + +async function serialConsole(req: Request) { + await delay(500) + const { socket, response } = Deno.upgradeWebSocket(req) + console.log(`New client connected`) + + // send hello as a binary frame for xterm to display + socket.onopen = () => { + setTimeout(() => { + streamString(socket, 'Wake up Neo...') + }, 200) + } + + // echo back binary data + socket.onmessage = (m) => socket.send(m.data) + + socket.onclose = () => console.log('Connection closed') + + return response +} + +serve(serialConsole, { port: 6036 }) diff --git a/tsconfig.json b/tsconfig.json index 20899e359..48b8250d5 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -33,5 +33,5 @@ "target": "es2015", "types": ["node", "vite/client", "vitest/importMeta", "vitest/globals"] }, - "exclude": ["tools/bump-omicron.ts"] + "exclude": ["tools/deno"] } diff --git a/vite.config.ts b/vite.config.ts index 5ec7d5ca5..c69438102 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -74,15 +74,16 @@ export default defineConfig(({ mode }) => ({ }, rewrite: (path) => path.replace(/^\/api/, ''), }, - '/ws-api': { - target: 'ws://localhost:12220', + '/ws-serial-console': { + // local mock server vs Nexus + target: 'ws://localhost:' + (process.env.MSW ? 6036 : 12220), ws: true, configure(proxy) { proxy.on('error', (_, req) => { - console.error(' to', '/api' + req.url) + console.error(' to', '/ws-serial-console' + req.url) }) }, - rewrite: (path) => path.replace(/^\/ws-api/, ''), + rewrite: (path) => path.replace(/^\/ws-serial-console/, ''), }, // We want to actually hit Nexus for this because it gives us a login redirect '/login': { From 7f459c5fe7761a7f55146080f853975dbe2aaaaf Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 28 Mar 2023 17:53:15 -0500 Subject: [PATCH 08/12] handle React 18's spurious double render made for exactly this reason --- .../instances/instance/SerialConsolePage.tsx | 67 ++++++++++++------- 1 file changed, 42 insertions(+), 25 deletions(-) diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index e28765681..88976596b 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -1,4 +1,4 @@ -import { Suspense, lazy, useEffect, useRef, useState } from 'react' +import { Suspense, lazy, useCallback, useEffect, useRef, useState } from 'react' import { Link } from 'react-router-dom' import { api } from '@oxide/api' @@ -14,35 +14,54 @@ const Terminal = lazy(() => import('app/components/Terminal')) // need prefix so Vite dev server can handle it specially const pathPrefix = process.env.NODE_ENV === 'development' ? '/ws-serial-console' : '' +type WsState = 'connecting' | 'open' | 'closed' | 'error' + export function SerialConsolePage() { const instanceSelector = useInstanceSelector() const { project, instance } = instanceSelector - // unclear if this should be a ref, it could be normal state or even a useMemo - const wsRef = useRef(null) + const ws = useRef(null) + + const [connectionStatus, setConnectionStatus] = useState('connecting') - const [connectionStatus, setConnectionStatus] = useState< - 'connecting' | 'open' | 'closed' | 'error' - >('connecting') + const setOpen = useCallback(() => setConnectionStatus('open'), []) + const setClosed = useCallback(() => setConnectionStatus('closed'), []) + const setError = useCallback(() => setConnectionStatus('error'), []) useEffect(() => { // TODO: error handling if this connection fails - const { project, instance } = instanceSelector - wsRef.current = api.ws.instanceSerialConsoleStream(window.location.host + pathPrefix, { - path: { instance }, - query: { project, fromStart: 0 }, - }) - // TODO: add listeners for other statuses? - wsRef.current.addEventListener('open', () => { - setConnectionStatus('open') - }) - // this is pretty important :| - wsRef.current.binaryType = 'arraybuffer' - return () => wsRef.current?.close() - // TODO: why is instanceSelector not stable?!?!?!?!? it's memoized! - // }, [instanceSelector]) - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []) + if (!ws.current) { + const { project, instance } = instanceSelector + ws.current = api.ws.instanceSerialConsoleStream(window.location.host + pathPrefix, { + path: { instance }, + query: { project, fromStart: 0 }, + }) + ws.current.binaryType = 'arraybuffer' + + ws.current.addEventListener('open', setOpen) + ws.current.addEventListener('closed', setClosed) + ws.current.addEventListener('error', setError) + // this is pretty important :| + } + // 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 + // before it's even connected, we check not only that it is non-null, but + // also that it is OPEN before trying to kill it. This allows the effect to + // run twice with no ill effect. + // + // 1. effect runs, WS connection initialized and starts connecting + // 1a. cleanup runs, but nothing happens because socket was not open yet + // 2. effect runs, but `ws.current` is truthy, so nothing happens + return () => { + if (ws.current?.readyState === WebSocket.OPEN) { + ws.current.close() + ws.current.removeEventListener('open', setOpen) + ws.current.removeEventListener('closed', setClosed) + ws.current.removeEventListener('error', setError) + ws.current = null + } + } + }, [instanceSelector, setOpen, setClosed, setError]) const command = `oxide instance serial --project ${project} @@ -63,9 +82,7 @@ export function SerialConsolePage() {
{connectionStatus !== 'open' && } - - {wsRef.current && } - + {ws.current && }
From 1aa76c61eaea6077754416bea1391dbb3d441f8b Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 29 Mar 2023 11:48:10 -0500 Subject: [PATCH 09/12] separate event listener effect from connection init effect --- app/components/StatusBadge.tsx | 17 ---- .../instances/instance/SerialConsolePage.tsx | 84 ++++++++++++------- 2 files changed, 55 insertions(+), 46 deletions(-) diff --git a/app/components/StatusBadge.tsx b/app/components/StatusBadge.tsx index 0db19d128..74f0a7a74 100644 --- a/app/components/StatusBadge.tsx +++ b/app/components/StatusBadge.tsx @@ -58,20 +58,3 @@ export const SnapshotStatusBadge = (props: { {props.status} ) - -export type SerialConsoleState = 'connecting' | 'connected' | 'disconnected' - -const SERIAL_CONSOLE_COLORS: Record = { - connecting: 'notice', - connected: 'default', - disconnected: 'destructive', -} - -export const SerialConsoleStatusBadge = (props: { - status: SerialConsoleState - className?: string -}) => ( - - {props.status} - -) diff --git a/app/pages/project/instances/instance/SerialConsolePage.tsx b/app/pages/project/instances/instance/SerialConsolePage.tsx index 88976596b..3df33e03e 100644 --- a/app/pages/project/instances/instance/SerialConsolePage.tsx +++ b/app/pages/project/instances/instance/SerialConsolePage.tsx @@ -1,11 +1,12 @@ -import { Suspense, lazy, useCallback, useEffect, useRef, useState } from 'react' +import { Suspense, lazy, useEffect, useRef, useState } from 'react' import { Link } from 'react-router-dom' import { api } from '@oxide/api' +import type { BadgeColor } from '@oxide/ui' +import { Badge } from '@oxide/ui' import { PrevArrow12Icon, Spinner } from '@oxide/ui' import EquivalentCliCommand from 'app/components/EquivalentCliCommand' -import { SerialConsoleStatusBadge } from 'app/components/StatusBadge' import { useInstanceSelector } from 'app/hooks' import { pb } from 'app/util/path-builder' @@ -16,6 +17,20 @@ const pathPrefix = process.env.NODE_ENV === 'development' ? '/ws-serial-console' type WsState = 'connecting' | 'open' | 'closed' | 'error' +const statusColor: Record = { + connecting: 'notice', + open: 'default', + closed: 'notice', + error: 'destructive', +} + +const statusMessage: Record = { + connecting: 'connecting', + open: 'connected', + closed: 'disconnected', + error: 'error', +} + export function SerialConsolePage() { const instanceSelector = useInstanceSelector() const { project, instance } = instanceSelector @@ -24,10 +39,15 @@ export function SerialConsolePage() { const [connectionStatus, setConnectionStatus] = useState('connecting') - const setOpen = useCallback(() => setConnectionStatus('open'), []) - const setClosed = useCallback(() => setConnectionStatus('closed'), []) - const setError = useCallback(() => setConnectionStatus('error'), []) - + // 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 + // before it's even connected, in the cleanup callback we check not only that + // it is non-null, but also that it is OPEN before trying to kill it. This + // allows the effect to run twice with no ill effect. + // + // 1. effect runs, WS connection initialized and starts connecting + // 1a. cleanup runs, nothing happens because socket was not open yet + // 2. effect runs, but `ws.current` is truthy, so nothing happens useEffect(() => { // TODO: error handling if this connection fails if (!ws.current) { @@ -36,32 +56,38 @@ export function SerialConsolePage() { path: { instance }, query: { project, fromStart: 0 }, }) - ws.current.binaryType = 'arraybuffer' - - ws.current.addEventListener('open', setOpen) - ws.current.addEventListener('closed', setClosed) - ws.current.addEventListener('error', setError) - // this is pretty important :| + ws.current.binaryType = 'arraybuffer' // important! } - // 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 - // before it's even connected, we check not only that it is non-null, but - // also that it is OPEN before trying to kill it. This allows the effect to - // run twice with no ill effect. - // - // 1. effect runs, WS connection initialized and starts connecting - // 1a. cleanup runs, but nothing happens because socket was not open yet - // 2. effect runs, but `ws.current` is truthy, so nothing happens return () => { if (ws.current?.readyState === WebSocket.OPEN) { ws.current.close() - ws.current.removeEventListener('open', setOpen) - ws.current.removeEventListener('closed', setClosed) - ws.current.removeEventListener('error', setError) - ws.current = null } } - }, [instanceSelector, setOpen, setClosed, setError]) + }, [instanceSelector]) + + // 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 + // render. But that's fine, we can add and remove listeners all day. + // + // 1. effect runs, ws connection is there because the other effect has run, + // so listeners are attached + // 1a. cleanup runs, event listeners removed + // 2. effect runs again, event listeners attached again + useEffect(() => { + 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('error', setError) + + return () => { + ws.current?.removeEventListener('open', setOpen) + ws.current?.removeEventListener('closed', setClosed) + ws.current?.removeEventListener('error', setError) + } + }, []) const command = `oxide instance serial --project ${project} @@ -90,9 +116,9 @@ export function SerialConsolePage() {
- + + {statusMessage[connectionStatus]} +
From b0adb56593b0ca4f08112a8fc9578c4e09ffedbf Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 29 Mar 2023 12:09:42 -0500 Subject: [PATCH 10/12] clean up Terminal component --- app/components/Terminal.tsx | 72 ++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 38 deletions(-) diff --git a/app/components/Terminal.tsx b/app/components/Terminal.tsx index dd8f4320f..0cf8dd3a5 100644 --- a/app/components/Terminal.tsx +++ b/app/components/Terminal.tsx @@ -8,62 +8,58 @@ import 'xterm/css/xterm.css' import { DirectionDownIcon, DirectionUpIcon } from '@oxide/ui' import { classed } from '@oxide/util' -interface TerminalProps { - ws: WebSocket -} - -const options: ITerminalOptions = { - allowTransparency: false, - screenReaderMode: true, - fontFamily: '"GT America Mono", monospace', - fontSize: 13, - lineHeight: 1.2, - windowOptions: { - fullscreenWin: true, - refreshWin: true, - }, -} - const ScrollButton = classed.button`ml-4 flex h-8 w-8 items-center justify-center rounded border border-secondary hover:bg-hover` -function getTheme() { +function getOptions(): ITerminalOptions { const style = getComputedStyle(document.body) return { - background: style.getPropertyValue('--surface-default'), - foreground: style.getPropertyValue('--content-default'), - black: style.getPropertyValue('--surface-default'), - brightBlack: style.getPropertyValue('--surface-secondary'), - white: style.getPropertyValue('--content-default'), - brightWhite: style.getPropertyValue('--content-secondary'), - blue: style.getPropertyValue('--base-blue-500'), - brightBlue: style.getPropertyValue('--base-blue-900'), - green: style.getPropertyValue('--content-success'), - brightGreen: style.getPropertyValue('--content-success-secondary'), - red: style.getPropertyValue('--content-error'), - brightRed: style.getPropertyValue('--content-error-secondary'), - yellow: style.getPropertyValue('--content-notice'), - brightYellow: style.getPropertyValue('--content-notice-secondary'), - cursor: style.getPropertyValue('--content-default'), - cursorAccent: style.getPropertyValue('--content-accent'), + allowTransparency: false, + screenReaderMode: true, + fontFamily: '"GT America Mono", monospace', + fontSize: 13, + lineHeight: 1.2, + windowOptions: { + fullscreenWin: true, + refreshWin: true, + }, + theme: { + background: style.getPropertyValue('--surface-default'), + foreground: style.getPropertyValue('--content-default'), + black: style.getPropertyValue('--surface-default'), + brightBlack: style.getPropertyValue('--surface-secondary'), + white: style.getPropertyValue('--content-default'), + brightWhite: style.getPropertyValue('--content-secondary'), + blue: style.getPropertyValue('--base-blue-500'), + brightBlue: style.getPropertyValue('--base-blue-900'), + green: style.getPropertyValue('--content-success'), + brightGreen: style.getPropertyValue('--content-success-secondary'), + red: style.getPropertyValue('--content-error'), + brightRed: style.getPropertyValue('--content-error-secondary'), + yellow: style.getPropertyValue('--content-notice'), + brightYellow: style.getPropertyValue('--content-notice-secondary'), + cursor: style.getPropertyValue('--content-default'), + cursorAccent: style.getPropertyValue('--content-accent'), + }, } } +interface TerminalProps { + ws: WebSocket +} + export const Terminal = ({ ws }: TerminalProps) => { const [term, setTerm] = useState(null) const terminalRef = useRef(null) useEffect(() => { - const newTerm = new XTerm({ theme: getTheme(), ...options }) + const newTerm = new XTerm(getOptions()) // Persist terminal instance, initialize terminal setTerm(newTerm) - // Setup terminal addons const fitAddon = new FitAddon() newTerm.loadAddon(fitAddon) - - const attachAddon = new AttachAddon(ws) - newTerm.loadAddon(attachAddon) + newTerm.loadAddon(new AttachAddon(ws)) // Handle window resizing const resize = () => fitAddon.fit() From 982b2b78c5b29c6ed93f3b6e362cce08a093d1cf Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 29 Mar 2023 12:47:49 -0500 Subject: [PATCH 11/12] add comment about my sadness --- app/components/Terminal.tsx | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/app/components/Terminal.tsx b/app/components/Terminal.tsx index 0cf8dd3a5..181d5490b 100644 --- a/app/components/Terminal.tsx +++ b/app/components/Terminal.tsx @@ -49,12 +49,21 @@ interface TerminalProps { export const Terminal = ({ ws }: TerminalProps) => { const [term, setTerm] = useState(null) - const terminalRef = useRef(null) + const terminalRef = useRef(null) useEffect(() => { const newTerm = new XTerm(getOptions()) - // Persist terminal instance, initialize terminal + // TODO: the render triggered by this call is load-bearing and should not + // be. Moving initialization out to a useMemo like it should be + // + // const term = useMemo(() => new XTerm(getOptions()), []) + // + // introduces a bug where the serial console text does not show up until you + // click the terminal area or resize the window. It cannot be about making + // this effect run again, because the deps don't include newTerm. It must be + // something internal to XTerm. Overall I do not feel particularly good + // about this whole section. setTerm(newTerm) const fitAddon = new FitAddon() @@ -64,6 +73,7 @@ export const Terminal = ({ ws }: TerminalProps) => { // Handle window resizing const resize = () => fitAddon.fit() + // ref will always be defined by the time the effect runs, but make TS happy if (terminalRef.current) { newTerm.open(terminalRef.current) resize() @@ -74,8 +84,7 @@ export const Terminal = ({ ws }: TerminalProps) => { newTerm.dispose() window.removeEventListener('resize', resize) } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []) + }, [ws]) return ( <> From 7d6d8e28fb3ea6f40ed86b2562cc432787c86340 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Fri, 31 Mar 2023 10:02:38 -0500 Subject: [PATCH 12/12] temporarily turn on gcp asset upload for this branch --- .github/workflows/upload-assets.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/upload-assets.yaml b/.github/workflows/upload-assets.yaml index db40e8495..5bc1a73d7 100644 --- a/.github/workflows/upload-assets.yaml +++ b/.github/workflows/upload-assets.yaml @@ -3,7 +3,7 @@ name: Upload assets to dl.oxide.computer on: push: - branches: [main] + branches: [main, serial-console-ws] jobs: build-and-upload: