From a3d74c4548b24a24402282af6c4bd9af8257e89c Mon Sep 17 00:00:00 2001
From: Mihai Parparita <mihai@tailscale.com>
Date: Wed, 27 Jul 2022 15:11:17 -0700
Subject: [PATCH] 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>
---
 cmd/tsconnect/src/index.ts    | 30 +++++++++++++++++++++++++++++-
 cmd/tsconnect/src/wasm_js.ts  |  3 ++-
 cmd/tsconnect/wasm/wasm_js.go | 17 ++++++++++++++---
 3 files changed, 45 insertions(+), 5 deletions(-)

diff --git a/cmd/tsconnect/src/index.ts b/cmd/tsconnect/src/index.ts
index ad8f125f3..c9fac4170 100644
--- a/cmd/tsconnect/src/index.ts
+++ b/cmd/tsconnect/src/index.ts
@@ -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
diff --git a/cmd/tsconnect/src/wasm_js.ts b/cmd/tsconnect/src/wasm_js.ts
index 9cf18df98..924c1cc22 100644
--- a/cmd/tsconnect/src/wasm_js.ts
+++ b/cmd/tsconnect/src/wasm_js.ts
@@ -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
   }
 }
diff --git a/cmd/tsconnect/wasm/wasm_js.go b/cmd/tsconnect/wasm/wasm_js.go
index ee835ffb6..7559f431e 100644
--- a/cmd/tsconnect/wasm/wasm_js.go
+++ b/cmd/tsconnect/wasm/wasm_js.go
@@ -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 {