Fix the blank VPN tab, and stop one page taking the console with it
On a machine with wireguard-tools installed but no tunnel up — core, which is the machine the console opens on — the tunnel list was a nil slice, which is null in JSON, and the page took its length. That throws, React unmounts the whole tree, and what is left is a blank window with nothing to go on. Two fixes, because either alone leaves the failure possible. The parse returns an empty list rather than nil, with a test that pins the marshalled shape; and the page treats a missing list as an empty one, so a node still running an older build cannot blank the console either. And the reason a small mistake had such a large blast radius: nothing caught it. Each page now renders inside a boundary, so a fault costs that page and says what it was, while the rail, the switcher and every other tab carry on. The terminals are deliberately left outside it — they hold live WebSockets, and remounting them to recover from somebody else's error would drop the sessions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
acb83d3a4a
commit
32fdbf169a
+3
-1
@@ -138,7 +138,9 @@ func Look(ctx context.Context) Report {
|
|||||||
//
|
//
|
||||||
// The private key is in the second column and is read past without being kept.
|
// The private key is in the second column and is read past without being kept.
|
||||||
func parseDump(out string) []Tunnel {
|
func parseDump(out string) []Tunnel {
|
||||||
var tunnels []Tunnel
|
// An empty list, never nil: this is JSON to a browser that will take its length, and a
|
||||||
|
// null there is a blank page rather than "no tunnels".
|
||||||
|
tunnels := []Tunnel{}
|
||||||
byName := map[string]int{}
|
byName := map[string]int{}
|
||||||
sc := bufio.NewScanner(strings.NewReader(out))
|
sc := bufio.NewScanner(strings.NewReader(out))
|
||||||
sc.Buffer(make([]byte, 0, 64<<10), 1<<20)
|
sc.Buffer(make([]byte, 0, 64<<10), 1<<20)
|
||||||
|
|||||||
@@ -172,3 +172,22 @@ func TestNeverConnectedHasNoTimestamp(t *testing.T) {
|
|||||||
t.Errorf("json = %s, want the handshake of a peer that has one", b)
|
t.Errorf("json = %s, want the handshake of a peer that has one", b)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A machine with wireguard-tools installed but no tunnel up produces an empty dump. That has
|
||||||
|
// to marshal as [] and not as null: the page takes the length of it, and null blanks the
|
||||||
|
// whole console rather than saying "no tunnels".
|
||||||
|
func TestEmptyDumpIsAnEmptyList(t *testing.T) {
|
||||||
|
for _, dump := range []string{"", "\n", " \n\n"} {
|
||||||
|
got := parseDump(dump)
|
||||||
|
if got == nil {
|
||||||
|
t.Fatalf("parseDump(%q) = nil, want an empty list", dump)
|
||||||
|
}
|
||||||
|
b, err := json.Marshal(got)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if string(b) != "[]" {
|
||||||
|
t.Errorf("parseDump(%q) marshals to %s, want []", dump, b)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+3
-1
@@ -27,7 +27,7 @@ import { MachineStatus, setMachine } from './api'
|
|||||||
import { LIST_FLOOR, useRefreshMs } from './prefs'
|
import { LIST_FLOOR, useRefreshMs } from './prefs'
|
||||||
import Power from './Power'
|
import Power from './Power'
|
||||||
import { applyAccent } from './accents'
|
import { applyAccent } from './accents'
|
||||||
import { Btn, Toasts, askText, askYes, hasUnsaved, okToLeave, toast } from './ui'
|
import { Btn, PageBoundary, Toasts, askText, askYes, hasUnsaved, okToLeave, toast } from './ui'
|
||||||
import { fmtLeft, useTimeLeft } from './guest'
|
import { fmtLeft, useTimeLeft } from './guest'
|
||||||
|
|
||||||
// Inline outline icon so it matches the monochrome glyphs in the rail (emoji would be full colour).
|
// Inline outline icon so it matches the monochrome glyphs in the rail (emoji would be full colour).
|
||||||
@@ -320,6 +320,7 @@ export default function Shell({ me: hubMe, onLoggedOut }: { me: Me; onLoggedOut:
|
|||||||
<div className="backdrop" onClick={() => setDrawer(false)} />
|
<div className="backdrop" onClick={() => setDrawer(false)} />
|
||||||
|
|
||||||
<main className="main" key={machine ?? 'hub'}>
|
<main className="main" key={machine ?? 'hub'}>
|
||||||
|
<PageBoundary page={GROUPS.find((g) => g.id === view)?.label ?? view}>
|
||||||
{view === 'settings' && <Settings me={hubMe} />}
|
{view === 'settings' && <Settings me={hubMe} />}
|
||||||
{view === 'users' && <Users me={hubMe} />}
|
{view === 'users' && <Users me={hubMe} />}
|
||||||
{view === 'kvm' && <KVM key={machine ?? 'hub'} machine={machine} title={title} />}
|
{view === 'kvm' && <KVM key={machine ?? 'hub'} machine={machine} title={title} />}
|
||||||
@@ -338,6 +339,7 @@ export default function Shell({ me: hubMe, onLoggedOut }: { me: Me; onLoggedOut:
|
|||||||
{view === 'vpn' && <VPN />}
|
{view === 'vpn' && <VPN />}
|
||||||
{view === 'zfs' && <ZFS />}
|
{view === 'zfs' && <ZFS />}
|
||||||
{view === 'files' && <Files onOpenInTerminal={runInTerminal} />}
|
{view === 'files' && <Files onOpenInTerminal={runInTerminal} />}
|
||||||
|
</PageBoundary>
|
||||||
{termView && <TermTabs terms={ordered} active={active} creating={creating}
|
{termView && <TermTabs terms={ordered} active={active} creating={creating}
|
||||||
onSelect={setActive} onNew={create} onRename={rename} onClose={close} onMove={moveTab}
|
onSelect={setActive} onNew={create} onRename={rename} onClose={close} onMove={moveTab}
|
||||||
onFocusTerm={() => window.setTimeout(() => { const id = activeRef.current; if (id) handles.current[id]?.armCtrl(false) }, 0)} />}
|
onFocusTerm={() => window.setTimeout(() => { const id = activeRef.current; if (id) handles.current[id]?.armCtrl(false) }, 0)} />}
|
||||||
|
|||||||
+12
-7
@@ -48,14 +48,15 @@ function Peer({ p, server }: { p: VPNPeer; server: boolean }) {
|
|||||||
|
|
||||||
function Tunnel({ t }: { t: VPNTunnel }) {
|
function Tunnel({ t }: { t: VPNTunnel }) {
|
||||||
const server = t.kind === 'server'
|
const server = t.kind === 'server'
|
||||||
const online = t.peers.filter((p) => p.online).length
|
const peers = t.peers ?? []
|
||||||
|
const online = peers.filter((p) => p.online).length
|
||||||
return (
|
return (
|
||||||
<>
|
<>
|
||||||
<div className="dash-bar">
|
<div className="dash-bar">
|
||||||
<h2 className="section">{t.name}</h2>
|
<h2 className="section">{t.name}</h2>
|
||||||
<span className="badge">{server ? 'devices dial in' : t.kind === 'client' ? 'this machine dials out' : 'tunnel'}</span>
|
<span className="badge">{server ? 'devices dial in' : t.kind === 'client' ? 'this machine dials out' : 'tunnel'}</span>
|
||||||
<span className="spacer" />
|
<span className="spacer" />
|
||||||
<span className="muted small">{server ? `${online} of ${t.peers.length} online` : t.exit ? `via ${t.exit}` : ''}</span>
|
<span className="muted small">{server ? `${online} of ${peers.length} online` : t.exit ? `via ${t.exit}` : ''}</span>
|
||||||
</div>
|
</div>
|
||||||
<section className="card">
|
<section className="card">
|
||||||
<div className="props">
|
<div className="props">
|
||||||
@@ -66,8 +67,8 @@ function Tunnel({ t }: { t: VPNTunnel }) {
|
|||||||
{t.fwmark && <div><span className="k">Firewall mark</span><span className="mono">{t.fwmark}</span></div>}
|
{t.fwmark && <div><span className="k">Firewall mark</span><span className="mono">{t.fwmark}</span></div>}
|
||||||
</div>
|
</div>
|
||||||
</section>
|
</section>
|
||||||
{t.peers.map((p) => <Peer key={p.public_key} p={p} server={server} />)}
|
{peers.map((p) => <Peer key={p.public_key} p={p} server={server} />)}
|
||||||
{t.peers.length === 0 && <section className="card"><div className="empty">No peers are configured on this tunnel.</div></section>}
|
{peers.length === 0 && <section className="card"><div className="empty">No peers are configured on this tunnel.</div></section>}
|
||||||
</>
|
</>
|
||||||
)
|
)
|
||||||
}
|
}
|
||||||
@@ -88,9 +89,13 @@ export default function VPN() {
|
|||||||
|
|
||||||
if (!d) return <div className="dash">{err ? <div className="notice">{err === 'not found' ? "This machine's Ham Lab is older than the VPN tab. Update it from Machines." : err}</div> : <Loading label="Reading the tunnels…" />}</div>
|
if (!d) return <div className="dash">{err ? <div className="notice">{err === 'not found' ? "This machine's Ham Lab is older than the VPN tab. Update it from Machines." : err}</div> : <Loading label="Reading the tunnels…" />}</div>
|
||||||
|
|
||||||
|
// A machine running an older build answers with null rather than an empty list, and taking
|
||||||
|
// the length of that is what a blank page is made of.
|
||||||
|
const tunnels = d.tunnels ?? []
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div className="dash net">
|
<div className="dash net">
|
||||||
{d.tunnels.length === 0 && (
|
{tunnels.length === 0 && (
|
||||||
<>
|
<>
|
||||||
<div className="dash-bar"><h2 className="section">VPN</h2></div>
|
<div className="dash-bar"><h2 className="section">VPN</h2></div>
|
||||||
<section className="card"><div className="empty">{d.note || 'Nothing here.'}</div>
|
<section className="card"><div className="empty">{d.note || 'Nothing here.'}</div>
|
||||||
@@ -100,8 +105,8 @@ export default function VPN() {
|
|||||||
</section>
|
</section>
|
||||||
</>
|
</>
|
||||||
)}
|
)}
|
||||||
{d.tunnels.map((t) => <Tunnel key={t.name} t={t} />)}
|
{tunnels.map((t) => <Tunnel key={t.name} t={t} />)}
|
||||||
{d.tunnels.length > 0 && (
|
{tunnels.length > 0 && (
|
||||||
<div className="muted small method" style={{ marginTop: 12 }}>
|
<div className="muted small method" style={{ marginTop: 12 }}>
|
||||||
Read from the kernel with <span className="mono">wg show</span>; names come from the configuration beside it.
|
Read from the kernel with <span className="mono">wg show</span>; names come from the configuration beside it.
|
||||||
Online means a handshake in the last three minutes — WireGuard is silent when idle, so there is no connection to be in.
|
Online means a handshake in the last three minutes — WireGuard is silent when idle, so there is no connection to be in.
|
||||||
|
|||||||
+32
-1
@@ -1,4 +1,4 @@
|
|||||||
import { ReactNode, useEffect, useRef, useState } from 'react'
|
import { Component, ReactNode, useEffect, useRef, useState } from 'react'
|
||||||
|
|
||||||
// ---- spinners ----
|
// ---- spinners ----
|
||||||
export function Spinner({ size = 14 }: { size?: number }) {
|
export function Spinner({ size = 14 }: { size?: number }) {
|
||||||
@@ -155,3 +155,34 @@ export async function okToLeave(): Promise<boolean> {
|
|||||||
if (!unsaved || (await askYes(`Discard unsaved changes to ${unsaved}?`, 'Discard'))) { unsaved = ''; return true }
|
if (!unsaved || (await askYes(`Discard unsaved changes to ${unsaved}?`, 'Discard'))) { unsaved = ''; return true }
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A page that throws takes the whole console down with it: React unmounts the tree, and what
|
||||||
|
// is left is a blank window with no clue in it. The VPN tab did exactly that on a machine
|
||||||
|
// whose tunnel list came back null. So each page is rendered inside one of these, and a fault
|
||||||
|
// costs that page rather than the session — the rail, the switcher and every other tab still
|
||||||
|
// work, and the error says which page and what it said.
|
||||||
|
export class PageBoundary extends Component<{ page: string; children: ReactNode }, { err: Error | null }> {
|
||||||
|
constructor(props: { page: string; children: ReactNode }) {
|
||||||
|
super(props)
|
||||||
|
this.state = { err: null }
|
||||||
|
}
|
||||||
|
static getDerivedStateFromError(err: Error) { return { err } }
|
||||||
|
componentDidCatch(err: Error) { console.error('page failed:', err) }
|
||||||
|
// A different page is a different mount; clear the fault when the page changes.
|
||||||
|
componentDidUpdate(prev: { page: string }) { if (prev.page !== this.props.page && this.state.err) this.setState({ err: null }) }
|
||||||
|
render() {
|
||||||
|
if (!this.state.err) return this.props.children
|
||||||
|
return (
|
||||||
|
<div className="dash">
|
||||||
|
<div className="dash-bar"><h2 className="section">{this.props.page}</h2></div>
|
||||||
|
<section className="card">
|
||||||
|
<div className="error">This page hit a fault and stopped drawing. The rest of the console is unaffected.</div>
|
||||||
|
<div className="muted small mono" style={{ marginTop: 8 }}>{String(this.state.err)}</div>
|
||||||
|
<div className="svc-actions" style={{ marginTop: 10 }}>
|
||||||
|
<button className="btn" onClick={() => this.setState({ err: null })}>Try again</button>
|
||||||
|
</div>
|
||||||
|
</section>
|
||||||
|
</div>
|
||||||
|
)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user