From a3580b5465b1f0c62eee05e1b7f6b38b115f2a06 Mon Sep 17 00:00:00 2001
From: Adam Shiervani <adam.shiervani@gmail.com>
Date: Tue, 25 Mar 2025 14:54:04 +0100
Subject: [PATCH] Improve error handling when `RTCPeerConnection` throws (#289)

* fix(WebRTC): improve error handling during peer connection creation and add connection error overlay

* refactor: update peer connection state handling and improve type definitions across components
---
 ui/src/components/Header.tsx                  |  2 +-
 .../components/PeerConnectionStatusCard.tsx   | 11 +++---
 ui/src/components/USBStateStatus.tsx          | 10 +++---
 ui/src/routes/devices.$id.tsx                 | 36 ++++++++++---------
 4 files changed, 34 insertions(+), 25 deletions(-)

diff --git a/ui/src/components/Header.tsx b/ui/src/components/Header.tsx
index 452a19c..cdbc3c4 100644
--- a/ui/src/components/Header.tsx
+++ b/ui/src/components/Header.tsx
@@ -36,7 +36,7 @@ export default function DashboardNavbar({
   picture,
   kvmName,
 }: NavbarProps) {
-  const peerConnectionState = useRTCStore(state => state.peerConnection?.connectionState);
+  const peerConnectionState = useRTCStore(state => state.peerConnectionState);
   const setUser = useUserStore(state => state.setUser);
   const navigate = useNavigate();
   const onLogout = useCallback(async () => {
diff --git a/ui/src/components/PeerConnectionStatusCard.tsx b/ui/src/components/PeerConnectionStatusCard.tsx
index 07e91cd..98025cd 100644
--- a/ui/src/components/PeerConnectionStatusCard.tsx
+++ b/ui/src/components/PeerConnectionStatusCard.tsx
@@ -9,19 +9,22 @@ const PeerConnectionStatusMap = {
   failed: "Connection failed",
   closed: "Closed",
   new: "Connecting",
-};
+} as Record<RTCPeerConnectionState | "error" | "closing", string>;
 
 export type PeerConnections = keyof typeof PeerConnectionStatusMap;
 
-type StatusProps = Record<PeerConnections, {
+type StatusProps = Record<
+  PeerConnections,
+  {
     statusIndicatorClassName: string;
-  }>;
+  }
+>;
 
 export default function PeerConnectionStatusCard({
   state,
   title,
 }: {
-  state?: PeerConnections;
+  state?: RTCPeerConnectionState | null;
   title?: string;
 }) {
   if (!state) return null;
diff --git a/ui/src/components/USBStateStatus.tsx b/ui/src/components/USBStateStatus.tsx
index d8e86c6..8feb458 100644
--- a/ui/src/components/USBStateStatus.tsx
+++ b/ui/src/components/USBStateStatus.tsx
@@ -8,11 +8,14 @@ import { HidState } from "@/hooks/stores";
 
 type USBStates = HidState["usbState"];
 
-type StatusProps = Record<USBStates, {
+type StatusProps = Record<
+  USBStates,
+  {
     icon: React.FC<{ className: string | undefined }>;
     iconClassName: string;
     statusIndicatorClassName: string;
-  }>;
+  }
+>;
 
 const USBStateMap: Record<USBStates, string> = {
   configured: "Connected",
@@ -27,9 +30,8 @@ export default function USBStateStatus({
   peerConnectionState,
 }: {
   state: USBStates;
-  peerConnectionState?: RTCPeerConnectionState;
+  peerConnectionState: RTCPeerConnectionState | null;
 }) {
-
   const StatusCardProps: StatusProps = {
     configured: {
       icon: ({ className }) => (
diff --git a/ui/src/routes/devices.$id.tsx b/ui/src/routes/devices.$id.tsx
index 05b0322..50fc79f 100644
--- a/ui/src/routes/devices.$id.tsx
+++ b/ui/src/routes/devices.$id.tsx
@@ -126,7 +126,7 @@ export default function KvmIdRoute() {
 
   const setIsTurnServerInUse = useRTCStore(state => state.setTurnServerInUse);
   const peerConnection = useRTCStore(state => state.peerConnection);
-
+  const peerConnectionState = useRTCStore(state => state.peerConnectionState);
   const setPeerConnectionState = useRTCStore(state => state.setPeerConnectionState);
   const setMediaMediaStream = useRTCStore(state => state.setMediaStream);
   const setPeerConnection = useRTCStore(state => state.setPeerConnection);
@@ -153,6 +153,7 @@ export default function KvmIdRoute() {
       // However, the onconnectionstatechange event doesn't fire when close() is called manually
       // So we need to explicitly update our state to maintain consistency
       // I don't know why this is happening, but this is the best way I can think of to handle it
+      // ALSO, this will render the connection error overlay linking to docs
       setPeerConnectionState("closed");
     },
     [peerConnection, setPeerConnectionState],
@@ -255,12 +256,19 @@ export default function KvmIdRoute() {
     setStartedConnectingAt(new Date());
     setConnectedAt(null);
 
-    const pc = new RTCPeerConnection({
-      // We only use STUN or TURN servers if we're in the cloud
-      ...(isInCloud && iceConfig?.iceServers
-        ? { iceServers: [iceConfig?.iceServers] }
-        : {}),
-    });
+    let pc: RTCPeerConnection;
+    try {
+      pc = new RTCPeerConnection({
+        // We only use STUN or TURN servers if we're in the cloud
+        ...(isInCloud && iceConfig?.iceServers
+          ? { iceServers: [iceConfig?.iceServers] }
+          : {}),
+      });
+    } catch (e) {
+      console.error(`Error creating peer connection: ${e}`);
+      closePeerConnection();
+      return;
+    }
 
     // Set up event listeners and data channels
     pc.onconnectionstatechange = () => {
@@ -296,8 +304,10 @@ export default function KvmIdRoute() {
       setPeerConnection(pc);
     } catch (e) {
       console.error(`Error creating offer: ${e}`);
+      closePeerConnection();
     }
   }, [
+    closePeerConnection,
     iceConfig?.iceServers,
     sdp,
     setDiskChannel,
@@ -315,9 +325,8 @@ export default function KvmIdRoute() {
     if (location.pathname.includes("other-session")) return;
 
     // If we're already connected or connecting, we don't need to connect
-    if (
-      ["connected", "connecting", "new"].includes(peerConnection?.connectionState ?? "")
-    ) {
+    // We have to use the state from the store, because the peerConnection.connectionState doesnt trigger a value change, if called manually from .close()
+    if (["connected", "connecting", "new"].includes(peerConnectionState ?? "")) {
       return;
     }
 
@@ -331,12 +340,7 @@ export default function KvmIdRoute() {
       connectWebRTC();
     }, 3000);
     return () => clearInterval(interval);
-  }, [
-    connectWebRTC,
-    connectionFailed,
-    location.pathname,
-    peerConnection?.connectionState,
-  ]);
+  }, [connectWebRTC, connectionFailed, location.pathname, peerConnectionState]);
 
   // On boot, if the connection state is undefined, we connect to the WebRTC
   useEffect(() => {