cmd/tsconnect: add basic panic handling

The go wasm process exiting is a sign of an unhandled panic. Also
add a explicit recover() call in the notify callback, that's where most
logic bugs are likely to happen (and they may not be fatal).

Also fixes the one panic that was encountered (nill pointer dereference
when generating the JS view of the netmap).

Fixes #5132

Signed-off-by: Mihai Parparita <mihai@tailscale.com>
This commit is contained in:
Mihai Parparita 2022-07-27 15:11:17 -07:00 committed by Mihai Parparita
parent 4dbdb19c26
commit a3d74c4548
3 changed files with 45 additions and 5 deletions

View File

@ -12,7 +12,8 @@ WebAssembly.instantiateStreaming(
fetch(`./dist/${wasmUrl}`),
go.importObject
).then((result) => {
go.run(result.instance)
// The Go process should never exit, if it does then it's an unhandled panic.
go.run(result.instance).then(() => handleGoPanic())
const ipn = newIPN({
// Persist IPN state in sessionStorage in development, so that we don't need
// to re-authorize every time we reload the page.
@ -22,5 +23,32 @@ WebAssembly.instantiateStreaming(
notifyState: notifyState.bind(null, ipn),
notifyNetMap: notifyNetMap.bind(null, ipn),
notifyBrowseToURL: notifyBrowseToURL.bind(null, ipn),
notifyPanicRecover: handleGoPanic,
})
})
function handleGoPanic(err?: string) {
if (DEBUG && err) {
console.error("Go panic", err)
}
if (panicNode) {
panicNode.remove()
}
panicNode = document.createElement("div")
panicNode.className =
"rounded bg-red-500 p-2 absolute top-2 right-2 text-white font-bold text-right cursor-pointer"
panicNode.textContent = "Tailscale has encountered an error."
const panicDetailNode = document.createElement("div")
panicDetailNode.className = "text-sm font-normal"
panicDetailNode.textContent = "Click to reload"
panicNode.appendChild(panicDetailNode)
panicNode.addEventListener("click", () => location.reload(), {
once: true,
})
document.body.appendChild(panicNode)
setTimeout(() => {
panicNode!.remove()
}, 10000)
}
let panicNode: HTMLDivElement | undefined

View File

@ -38,6 +38,7 @@ declare global {
notifyState: (state: IPNState) => void
notifyNetMap: (netMapStr: string) => void
notifyBrowseToURL: (url: string) => void
notifyPanicRecover: (err: string) => void
}
type IPNNetMap = {
@ -57,7 +58,7 @@ declare global {
}
type IPNNetMapPeerNode = IPNNetMapNode & {
online: boolean
online?: boolean
tailscaleSSHEnabled: boolean
}
}

View File

@ -114,6 +114,7 @@ func newIPN(jsConfig js.Value) map[string]any {
notifyState(state: int): void,
notifyNetMap(netMap: object): void,
notifyBrowseToURL(url: string): void,
notifyPanicRecover(err: string): void,
})`)
return nil
}
@ -166,6 +167,16 @@ func (i *jsIPN) run(jsCallbacks js.Value) {
notifyState(ipn.NoState)
i.lb.SetNotifyCallback(func(n ipn.Notify) {
// Panics in the notify callback are likely due to be due to bugs in
// this bridging module (as opposed to actual bugs in Tailscale) and
// thus may be recoverable. Let the UI know, and allow the user to
// choose if they want to reload the page.
defer func() {
if r := recover(); r != nil {
fmt.Println("Panic recovered:", r)
jsCallbacks.Call("notifyPanicRecover", fmt.Sprint(r))
}
}()
log.Printf("NOTIFY: %+v", n)
if n.State != nil {
notifyState(*n.State)
@ -189,7 +200,7 @@ func (i *jsIPN) run(jsCallbacks js.Value) {
MachineKey: p.Machine.String(),
NodeKey: p.Key.String(),
},
Online: *p.Online,
Online: p.Online,
TailscaleSSHEnabled: p.Hostinfo.TailscaleSSHEnabled(),
}
}),
@ -352,8 +363,8 @@ type jsNetMapSelfNode struct {
type jsNetMapPeerNode struct {
jsNetMapNode
Online bool `json:"online"`
TailscaleSSHEnabled bool `json:"tailscaleSSHEnabled"`
Online *bool `json:"online,omitempty"`
TailscaleSSHEnabled bool `json:"tailscaleSSHEnabled"`
}
type jsStateStore struct {