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
This commit is contained in:
Adam Shiervani 2025-03-25 14:54:04 +01:00 committed by GitHub
parent 3b711db781
commit a3580b5465
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 34 additions and 25 deletions

View File

@ -36,7 +36,7 @@ export default function DashboardNavbar({
picture, picture,
kvmName, kvmName,
}: NavbarProps) { }: NavbarProps) {
const peerConnectionState = useRTCStore(state => state.peerConnection?.connectionState); const peerConnectionState = useRTCStore(state => state.peerConnectionState);
const setUser = useUserStore(state => state.setUser); const setUser = useUserStore(state => state.setUser);
const navigate = useNavigate(); const navigate = useNavigate();
const onLogout = useCallback(async () => { const onLogout = useCallback(async () => {

View File

@ -9,19 +9,22 @@ const PeerConnectionStatusMap = {
failed: "Connection failed", failed: "Connection failed",
closed: "Closed", closed: "Closed",
new: "Connecting", new: "Connecting",
}; } as Record<RTCPeerConnectionState | "error" | "closing", string>;
export type PeerConnections = keyof typeof PeerConnectionStatusMap; export type PeerConnections = keyof typeof PeerConnectionStatusMap;
type StatusProps = Record<PeerConnections, { type StatusProps = Record<
PeerConnections,
{
statusIndicatorClassName: string; statusIndicatorClassName: string;
}>; }
>;
export default function PeerConnectionStatusCard({ export default function PeerConnectionStatusCard({
state, state,
title, title,
}: { }: {
state?: PeerConnections; state?: RTCPeerConnectionState | null;
title?: string; title?: string;
}) { }) {
if (!state) return null; if (!state) return null;

View File

@ -8,11 +8,14 @@ import { HidState } from "@/hooks/stores";
type USBStates = HidState["usbState"]; type USBStates = HidState["usbState"];
type StatusProps = Record<USBStates, { type StatusProps = Record<
USBStates,
{
icon: React.FC<{ className: string | undefined }>; icon: React.FC<{ className: string | undefined }>;
iconClassName: string; iconClassName: string;
statusIndicatorClassName: string; statusIndicatorClassName: string;
}>; }
>;
const USBStateMap: Record<USBStates, string> = { const USBStateMap: Record<USBStates, string> = {
configured: "Connected", configured: "Connected",
@ -27,9 +30,8 @@ export default function USBStateStatus({
peerConnectionState, peerConnectionState,
}: { }: {
state: USBStates; state: USBStates;
peerConnectionState?: RTCPeerConnectionState; peerConnectionState: RTCPeerConnectionState | null;
}) { }) {
const StatusCardProps: StatusProps = { const StatusCardProps: StatusProps = {
configured: { configured: {
icon: ({ className }) => ( icon: ({ className }) => (

View File

@ -126,7 +126,7 @@ export default function KvmIdRoute() {
const setIsTurnServerInUse = useRTCStore(state => state.setTurnServerInUse); const setIsTurnServerInUse = useRTCStore(state => state.setTurnServerInUse);
const peerConnection = useRTCStore(state => state.peerConnection); const peerConnection = useRTCStore(state => state.peerConnection);
const peerConnectionState = useRTCStore(state => state.peerConnectionState);
const setPeerConnectionState = useRTCStore(state => state.setPeerConnectionState); const setPeerConnectionState = useRTCStore(state => state.setPeerConnectionState);
const setMediaMediaStream = useRTCStore(state => state.setMediaStream); const setMediaMediaStream = useRTCStore(state => state.setMediaStream);
const setPeerConnection = useRTCStore(state => state.setPeerConnection); 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 // However, the onconnectionstatechange event doesn't fire when close() is called manually
// So we need to explicitly update our state to maintain consistency // 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 // 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"); setPeerConnectionState("closed");
}, },
[peerConnection, setPeerConnectionState], [peerConnection, setPeerConnectionState],
@ -255,12 +256,19 @@ export default function KvmIdRoute() {
setStartedConnectingAt(new Date()); setStartedConnectingAt(new Date());
setConnectedAt(null); setConnectedAt(null);
const pc = new RTCPeerConnection({ let pc: RTCPeerConnection;
// We only use STUN or TURN servers if we're in the cloud try {
...(isInCloud && iceConfig?.iceServers pc = new RTCPeerConnection({
? { iceServers: [iceConfig?.iceServers] } // 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 // Set up event listeners and data channels
pc.onconnectionstatechange = () => { pc.onconnectionstatechange = () => {
@ -296,8 +304,10 @@ export default function KvmIdRoute() {
setPeerConnection(pc); setPeerConnection(pc);
} catch (e) { } catch (e) {
console.error(`Error creating offer: ${e}`); console.error(`Error creating offer: ${e}`);
closePeerConnection();
} }
}, [ }, [
closePeerConnection,
iceConfig?.iceServers, iceConfig?.iceServers,
sdp, sdp,
setDiskChannel, setDiskChannel,
@ -315,9 +325,8 @@ export default function KvmIdRoute() {
if (location.pathname.includes("other-session")) return; if (location.pathname.includes("other-session")) return;
// If we're already connected or connecting, we don't need to connect // If we're already connected or connecting, we don't need to connect
if ( // We have to use the state from the store, because the peerConnection.connectionState doesnt trigger a value change, if called manually from .close()
["connected", "connecting", "new"].includes(peerConnection?.connectionState ?? "") if (["connected", "connecting", "new"].includes(peerConnectionState ?? "")) {
) {
return; return;
} }
@ -331,12 +340,7 @@ export default function KvmIdRoute() {
connectWebRTC(); connectWebRTC();
}, 3000); }, 3000);
return () => clearInterval(interval); return () => clearInterval(interval);
}, [ }, [connectWebRTC, connectionFailed, location.pathname, peerConnectionState]);
connectWebRTC,
connectionFailed,
location.pathname,
peerConnection?.connectionState,
]);
// On boot, if the connection state is undefined, we connect to the WebRTC // On boot, if the connection state is undefined, we connect to the WebRTC
useEffect(() => { useEffect(() => {