From 1b8954e9f32935f1cb8fe8ac4f7c8e2458bf30c4 Mon Sep 17 00:00:00 2001 From: Siyuan Miao Date: Mon, 24 Mar 2025 23:20:08 +0100 Subject: [PATCH 1/7] chore: fix linting issues of web_tls.go --- web_tls.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/web_tls.go b/web_tls.go index fff9253..b1bb60e 100644 --- a/web_tls.go +++ b/web_tls.go @@ -38,7 +38,7 @@ func RunWebSecureServer() { TLSConfig: &tls.Config{ // TODO: cache certificate in persistent storage GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - hostname := WebSecureSelfSignedDefaultDomain + var hostname string if info.ServerName != "" { hostname = info.ServerName } else { @@ -58,7 +58,6 @@ func RunWebSecureServer() { if err != nil { panic(err) } - return } func createSelfSignedCert(hostname string) *tls.Certificate { From a3580b5465b1f0c62eee05e1b7f6b38b115f2a06 Mon Sep 17 00:00:00 2001 From: Adam Shiervani Date: Tue, 25 Mar 2025 14:54:04 +0100 Subject: [PATCH 2/7] 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; export type PeerConnections = keyof typeof PeerConnectionStatusMap; -type StatusProps = Record; + } +>; 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; iconClassName: string; statusIndicatorClassName: string; - }>; + } +>; const USBStateMap: Record = { 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(() => { From aed453cc8cf7da19cd26a97f4cf1bf7b1291ac7b Mon Sep 17 00:00:00 2001 From: SuperQ Date: Wed, 12 Mar 2025 16:29:45 +0100 Subject: [PATCH 3/7] chore: Enable more linters Enable more golangci-lint linters. * `forbidigo` to stop use of non-logger console printing. * `goimports` to make sure `import` blocks are formatted nicely. * `misspell` to catch spelling mistakes. * `whitespace` to catch whitespace issues. Signed-off-by: SuperQ --- .golangci.yml | 14 ++++++++++++-- prometheus.go | 4 ---- serial.go | 1 - web.go | 2 +- web_tls.go | 8 ++++---- 5 files changed, 17 insertions(+), 12 deletions(-) diff --git a/.golangci.yml b/.golangci.yml index ddf4443..95a1cb8 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,12 +1,22 @@ --- linters: enable: - # - goimports - # - misspell + - forbidigo + - goimports + - misspell # - revive + - whitespace issues: exclude-rules: - path: _test.go linters: - errcheck + +linters-settings: + forbidigo: + forbid: + - p: ^fmt\.Print.*$ + msg: Do not commit print statements. Use logger package. + - p: ^log\.(Fatal|Panic|Print)(f|ln)?.*$ + msg: Do not commit log statements. Use logger package. diff --git a/prometheus.go b/prometheus.go index 8ebf259..5d4c5e7 100644 --- a/prometheus.go +++ b/prometheus.go @@ -1,15 +1,11 @@ package kvm import ( - "net/http" - "github.com/prometheus/client_golang/prometheus" versioncollector "github.com/prometheus/client_golang/prometheus/collectors/version" "github.com/prometheus/common/version" ) -var promHandler http.Handler - func initPrometheus() { // A Prometheus metrics endpoint. version.Version = builtAppVersion diff --git a/serial.go b/serial.go index a4ab7d5..31fd553 100644 --- a/serial.go +++ b/serial.go @@ -66,7 +66,6 @@ func runATXControl() { newLedPWRState != ledPWRState || newBtnRSTState != btnRSTState || newBtnPWRState != btnPWRState { - logger.Debugf("Status changed: HDD LED: %v, PWR LED: %v, RST BTN: %v, PWR BTN: %v", newLedHDDState, newLedPWRState, newBtnRSTState, newBtnPWRState) diff --git a/web.go b/web.go index b35a2db..9201e7b 100644 --- a/web.go +++ b/web.go @@ -16,6 +16,7 @@ import ( "golang.org/x/crypto/bcrypt" ) +//nolint:typecheck //go:embed all:static var staticFiles embed.FS @@ -419,7 +420,6 @@ func handleSetup(c *gin.Context) { // Set the cookie c.SetCookie("authToken", config.LocalAuthToken, 7*24*60*60, "/", "", false, true) - } else { // For noPassword mode, ensure the password field is empty config.HashedPassword = "" diff --git a/web_tls.go b/web_tls.go index fff9253..976cff6 100644 --- a/web_tls.go +++ b/web_tls.go @@ -8,10 +8,10 @@ import ( "crypto/x509" "crypto/x509/pkix" "encoding/pem" - "log" "math/big" "net" "net/http" + "os" "strings" "sync" "time" @@ -38,7 +38,7 @@ func RunWebSecureServer() { TLSConfig: &tls.Config{ // TODO: cache certificate in persistent storage GetCertificate: func(info *tls.ClientHelloInfo) (*tls.Certificate, error) { - hostname := WebSecureSelfSignedDefaultDomain + var hostname string if info.ServerName != "" { hostname = info.ServerName } else { @@ -58,7 +58,6 @@ func RunWebSecureServer() { if err != nil { panic(err) } - return } func createSelfSignedCert(hostname string) *tls.Certificate { @@ -72,7 +71,8 @@ func createSelfSignedCert(hostname string) *tls.Certificate { priv, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) if err != nil { - log.Fatalf("Failed to generate private key: %v", err) + logger.Errorf("Failed to generate private key: %v", err) + os.Exit(1) } keyUsage := x509.KeyUsageDigitalSignature From df0d083a28552a0d0a9e693cfb7a91fe0062f28f Mon Sep 17 00:00:00 2001 From: Cameron Fleming Date: Sat, 29 Mar 2025 21:13:59 +0000 Subject: [PATCH 4/7] chore: Update README Discord Link Corrects Discord link in the help section. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 1b516d7..5d0e9d7 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ We welcome contributions from the community! Whether it's improving the firmware ## I need help -The best place to search for answers is our [Documentation](https://jetkvm.com/docs). If you can't find the answer there, check our [Discord Server](https://discord.gg/8MaAhua7NW). +The best place to search for answers is our [Documentation](https://jetkvm.com/docs). If you can't find the answer there, check our [Discord Server](https://jetkvm.com/discord). ## I want to report an issue From 1e9adf81d433a23202be881de7a8cb4cbfca9953 Mon Sep 17 00:00:00 2001 From: Siyuan Miao Date: Thu, 3 Apr 2025 18:16:41 +0200 Subject: [PATCH 5/7] chore: skip websocket client if net isn't up or time sync hasn't complete --- cloud.go | 26 +++++++++++++++++++++----- main.go | 8 +++----- ntp.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/cloud.go b/cloud.go index a30a14c..4b9c2b4 100644 --- a/cloud.go +++ b/cloud.go @@ -90,11 +90,6 @@ func handleCloudRegister(c *gin.Context) { return } - if config.CloudToken == "" { - cloudLogger.Info("Starting websocket client due to adoption") - go RunWebsocketClient() - } - config.CloudToken = tokenResp.SecretToken provider, err := oidc.NewProvider(c, "https://accounts.google.com") @@ -130,6 +125,7 @@ func runWebsocketClient() error { time.Sleep(5 * time.Second) return fmt.Errorf("cloud token is not set") } + wsURL, err := url.Parse(config.CloudURL) if err != nil { return fmt.Errorf("failed to parse config.CloudURL: %w", err) @@ -253,6 +249,26 @@ func handleSessionRequest(ctx context.Context, c *websocket.Conn, req WebRTCSess func RunWebsocketClient() { for { + // If the cloud token is not set, we don't need to run the websocket client. + if config.CloudToken == "" { + time.Sleep(5 * time.Second) + continue + } + + // If the network is not up, well, we can't connect to the cloud. + if !networkState.Up { + cloudLogger.Warn("waiting for network to be up, will retry in 3 seconds") + time.Sleep(3 * time.Second) + continue + } + + // If the system time is not synchronized, the API request will fail anyway because the TLS handshake will fail. + if isTimeSyncNeeded() && !timeSyncSuccess { + cloudLogger.Warn("system time is not synced, will retry in 3 seconds") + time.Sleep(3 * time.Second) + continue + } + err := runWebsocketClient() if err != nil { cloudLogger.Errorf("websocket client error: %v", err) diff --git a/main.go b/main.go index 6a55595..aeb3d85 100644 --- a/main.go +++ b/main.go @@ -72,11 +72,9 @@ func Main() { if config.TLSMode != "" { go RunWebSecureServer() } - // If the cloud token isn't set, the client won't be started by default. - // However, if the user adopts the device via the web interface, handleCloudRegister will start the client. - if config.CloudToken != "" { - go RunWebsocketClient() - } + // As websocket client already checks if the cloud token is set, we can start it here. + go RunWebsocketClient() + initSerialPort() sigs := make(chan os.Signal, 1) signal.Notify(sigs, syscall.SIGINT, syscall.SIGTERM) diff --git a/ntp.go b/ntp.go index 39ea7af..27ec100 100644 --- a/ntp.go +++ b/ntp.go @@ -5,6 +5,7 @@ import ( "fmt" "net/http" "os/exec" + "strconv" "time" "github.com/beevik/ntp" @@ -20,13 +21,41 @@ const ( ) var ( + builtTimestamp string timeSyncRetryInterval = 0 * time.Second + timeSyncSuccess = false defaultNTPServers = []string{ "time.cloudflare.com", "time.apple.com", } ) +func isTimeSyncNeeded() bool { + if builtTimestamp == "" { + logger.Warnf("Built timestamp is not set, time sync is needed") + return true + } + + ts, err := strconv.Atoi(builtTimestamp) + if err != nil { + logger.Warnf("Failed to parse built timestamp: %v", err) + return true + } + + // builtTimestamp is UNIX timestamp in seconds + builtTime := time.Unix(int64(ts), 0) + now := time.Now() + + logger.Tracef("Built time: %v, now: %v", builtTime, now) + + if now.Sub(builtTime) < 0 { + logger.Warnf("System time is behind the built time, time sync is needed") + return true + } + + return false +} + func TimeSyncLoop() { for { if !networkState.checked { @@ -40,6 +69,9 @@ func TimeSyncLoop() { continue } + // check if time sync is needed, but do nothing for now + isTimeSyncNeeded() + logger.Infof("Syncing system time") start := time.Now() err := SyncSystemTime() @@ -56,6 +88,7 @@ func TimeSyncLoop() { continue } + timeSyncSuccess = true logger.Infof("Time sync successful, now is: %v, time taken: %v", time.Now(), time.Since(start)) time.Sleep(timeSyncInterval) // after the first sync is done } From f3b5011d65ade31b34aad01a2c1e670582810f28 Mon Sep 17 00:00:00 2001 From: Siyuan Miao Date: Thu, 3 Apr 2025 19:06:21 +0200 Subject: [PATCH 6/7] feat(cloud): add metrics for cloud connections --- cloud.go | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 127 insertions(+) diff --git a/cloud.go b/cloud.go index 4b9c2b4..be53b08 100644 --- a/cloud.go +++ b/cloud.go @@ -10,6 +10,8 @@ import ( "time" "github.com/coder/websocket/wsjson" + "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/promauto" "github.com/coreos/go-oidc/v3/oidc" @@ -36,6 +38,97 @@ const ( CloudWebSocketPingInterval = 15 * time.Second ) +var ( + metricCloudConnectionStatus = promauto.NewGauge( + prometheus.GaugeOpts{ + Name: "jetkvm_cloud_connection_status", + Help: "The status of the cloud connection", + }, + ) + metricCloudConnectionEstablishedTimestamp = promauto.NewGauge( + prometheus.GaugeOpts{ + Name: "jetkvm_cloud_connection_established_timestamp", + Help: "The timestamp when the cloud connection was established", + }, + ) + metricCloudConnectionLastPingTimestamp = promauto.NewGauge( + prometheus.GaugeOpts{ + Name: "jetkvm_cloud_connection_last_ping_timestamp", + Help: "The timestamp when the last ping response was received", + }, + ) + metricCloudConnectionLastPingDuration = promauto.NewGauge( + prometheus.GaugeOpts{ + Name: "jetkvm_cloud_connection_last_ping_duration", + Help: "The duration of the last ping response", + }, + ) + metricCloudConnectionPingDuration = promauto.NewHistogram( + prometheus.HistogramOpts{ + Name: "jetkvm_cloud_connection_ping_duration", + Help: "The duration of the ping response", + Buckets: []float64{ + 0.1, 0.5, 1, 10, + }, + }, + ) + metricCloudConnectionTotalPingCount = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "jetkvm_cloud_connection_total_ping_count", + Help: "The total number of pings sent to the cloud", + }, + ) + metricCloudConnectionSessionRequestCount = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "jetkvm_cloud_connection_session_total_request_count", + Help: "The total number of session requests received from the cloud", + }, + ) + metricCloudConnectionSessionRequestDuration = promauto.NewHistogram( + prometheus.HistogramOpts{ + Name: "jetkvm_cloud_connection_session_request_duration", + Help: "The duration of session requests", + Buckets: []float64{ + 0.1, 0.5, 1, 10, + }, + }, + ) + metricCloudConnectionLastSessionRequestTimestamp = promauto.NewGauge( + prometheus.GaugeOpts{ + Name: "jetkvm_cloud_connection_last_session_request_timestamp", + Help: "The timestamp of the last session request", + }, + ) + metricCloudConnectionLastSessionRequestDuration = promauto.NewGauge( + prometheus.GaugeOpts{ + Name: "jetkvm_cloud_connection_last_session_request_duration", + Help: "The duration of the last session request", + }, + ) + metricCloudConnectionFailureCount = promauto.NewCounter( + prometheus.CounterOpts{ + Name: "jetkvm_cloud_connection_failure_count", + Help: "The number of times the cloud connection has failed", + }, + ) +) + +func cloudResetMetrics(established bool) { + metricCloudConnectionLastPingTimestamp.Set(-1) + metricCloudConnectionLastPingDuration.Set(-1) + + metricCloudConnectionLastSessionRequestTimestamp.Set(-1) + metricCloudConnectionLastSessionRequestDuration.Set(-1) + + if established { + metricCloudConnectionEstablishedTimestamp.SetToCurrentTime() + metricCloudConnectionStatus.Set(1) + } else { + metricCloudConnectionEstablishedTimestamp.Set(-1) + metricCloudConnectionStatus.Set(-1) + } +} + func handleCloudRegister(c *gin.Context) { var req CloudRegisterRequest @@ -130,15 +223,18 @@ func runWebsocketClient() error { if err != nil { return fmt.Errorf("failed to parse config.CloudURL: %w", err) } + if wsURL.Scheme == "http" { wsURL.Scheme = "ws" } else { wsURL.Scheme = "wss" } + header := http.Header{} header.Set("X-Device-ID", GetDeviceID()) header.Set("Authorization", "Bearer "+config.CloudToken) dialCtx, cancelDial := context.WithTimeout(context.Background(), CloudWebSocketConnectTimeout) + defer cancelDial() c, _, err := websocket.Dial(dialCtx, wsURL.String(), &websocket.DialOptions{ HTTPHeader: header, @@ -148,17 +244,35 @@ func runWebsocketClient() error { } defer c.CloseNow() //nolint:errcheck cloudLogger.Infof("websocket connected to %s", wsURL) + + // set the metrics when we successfully connect to the cloud. + cloudResetMetrics(true) + runCtx, cancelRun := context.WithCancel(context.Background()) defer cancelRun() go func() { for { time.Sleep(CloudWebSocketPingInterval) + + // set the timer for the ping duration + timer := prometheus.NewTimer(prometheus.ObserverFunc(func(v float64) { + metricCloudConnectionLastPingDuration.Set(v) + metricCloudConnectionPingDuration.Observe(v) + })) + err := c.Ping(runCtx) + if err != nil { cloudLogger.Warnf("websocket ping error: %v", err) cancelRun() return } + + // dont use `defer` here because we want to observe the duration of the ping + timer.ObserveDuration() + + metricCloudConnectionTotalPingCount.Inc() + metricCloudConnectionLastPingTimestamp.SetToCurrentTime() } }() for { @@ -180,6 +294,8 @@ func runWebsocketClient() error { cloudLogger.Infof("new session request: %v", req.OidcGoogle) cloudLogger.Tracef("session request info: %v", req) + metricCloudConnectionSessionRequestCount.Inc() + metricCloudConnectionLastSessionRequestTimestamp.SetToCurrentTime() err = handleSessionRequest(runCtx, c, req) if err != nil { cloudLogger.Infof("error starting new session: %v", err) @@ -189,6 +305,12 @@ func runWebsocketClient() error { } func handleSessionRequest(ctx context.Context, c *websocket.Conn, req WebRTCSessionRequest) error { + timer := prometheus.NewTimer(prometheus.ObserverFunc(func(v float64) { + metricCloudConnectionLastSessionRequestDuration.Set(v) + metricCloudConnectionSessionRequestDuration.Observe(v) + })) + defer timer.ObserveDuration() + oidcCtx, cancelOIDC := context.WithTimeout(ctx, CloudOidcRequestTimeout) defer cancelOIDC() provider, err := oidc.NewProvider(oidcCtx, "https://accounts.google.com") @@ -249,6 +371,9 @@ func handleSessionRequest(ctx context.Context, c *websocket.Conn, req WebRTCSess func RunWebsocketClient() { for { + // reset the metrics when we start the websocket client. + cloudResetMetrics(false) + // If the cloud token is not set, we don't need to run the websocket client. if config.CloudToken == "" { time.Sleep(5 * time.Second) @@ -272,6 +397,8 @@ func RunWebsocketClient() { err := runWebsocketClient() if err != nil { cloudLogger.Errorf("websocket client error: %v", err) + metricCloudConnectionStatus.Set(0) + metricCloudConnectionFailureCount.Inc() time.Sleep(5 * time.Second) } } From 8268b20f325abb23cdeb833204754856c0d82339 Mon Sep 17 00:00:00 2001 From: Adam Shiervani Date: Thu, 3 Apr 2025 19:32:14 +0200 Subject: [PATCH 7/7] refactor: Update WebRTC connection handling and overlays (#320) * refactor: Update WebRTC connection handling and overlays * fix: Update comments for WebRTC connection handling in KvmIdRoute * chore: Clean up import statements in devices.$id.tsx --- ui/src/components/Header.tsx | 6 +- ui/src/components/USBStateStatus.tsx | 2 +- ui/src/components/VideoOverlay.tsx | 67 ++++- ui/src/components/WebRTCVideo.tsx | 101 +++++--- ui/src/routes/devices.$id.other-session.tsx | 4 +- ui/src/routes/devices.$id.tsx | 263 ++++++++++---------- 6 files changed, 263 insertions(+), 180 deletions(-) diff --git a/ui/src/components/Header.tsx b/ui/src/components/Header.tsx index cdbc3c4..03a907e 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.peerConnectionState); + const peerConnection = useRTCStore(state => state.peerConnection); const setUser = useUserStore(state => state.setUser); const navigate = useNavigate(); const onLogout = useCallback(async () => { @@ -82,14 +82,14 @@ export default function DashboardNavbar({
diff --git a/ui/src/components/USBStateStatus.tsx b/ui/src/components/USBStateStatus.tsx index 8feb458..f0b2cb2 100644 --- a/ui/src/components/USBStateStatus.tsx +++ b/ui/src/components/USBStateStatus.tsx @@ -30,7 +30,7 @@ export default function USBStateStatus({ peerConnectionState, }: { state: USBStates; - peerConnectionState: RTCPeerConnectionState | null; + peerConnectionState?: RTCPeerConnectionState | null; }) { const StatusCardProps: StatusProps = { configured: { diff --git a/ui/src/components/VideoOverlay.tsx b/ui/src/components/VideoOverlay.tsx index f13b6ce..0620af4 100644 --- a/ui/src/components/VideoOverlay.tsx +++ b/ui/src/components/VideoOverlay.tsx @@ -1,6 +1,6 @@ import React from "react"; import { ExclamationTriangleIcon } from "@heroicons/react/24/solid"; -import { ArrowRightIcon } from "@heroicons/react/16/solid"; +import { ArrowPathIcon, ArrowRightIcon } from "@heroicons/react/16/solid"; import { motion, AnimatePresence } from "framer-motion"; import { LuPlay } from "react-icons/lu"; @@ -25,12 +25,12 @@ interface LoadingOverlayProps { show: boolean; } -export function LoadingOverlay({ show }: LoadingOverlayProps) { +export function LoadingVideoOverlay({ show }: LoadingOverlayProps) { return ( {show && ( {show && ( + +
+
+ +
+

+ {text} +

+
+ + + )} + + ); +} + +interface ConnectionErrorOverlayProps { + show: boolean; + setupPeerConnection: () => Promise; +} + +export function ConnectionErrorOverlay({ + show, + setupPeerConnection, +}: ConnectionErrorOverlayProps) { + return ( + + {show && ( + @@ -87,14 +125,21 @@ export function ConnectionErrorOverlay({ show }: ConnectionErrorOverlayProps) {
  • Try restarting both the device and your computer
    • -
    +
    +
    diff --git a/ui/src/components/WebRTCVideo.tsx b/ui/src/components/WebRTCVideo.tsx index 911c5ea..5d8fb55 100644 --- a/ui/src/components/WebRTCVideo.tsx +++ b/ui/src/components/WebRTCVideo.tsx @@ -19,9 +19,8 @@ import { useJsonRpc } from "@/hooks/useJsonRpc"; import { HDMIErrorOverlay, + LoadingVideoOverlay, NoAutoplayPermissionsOverlay, - ConnectionErrorOverlay, - LoadingOverlay, } from "./VideoOverlay"; export default function WebRTCVideo() { @@ -46,15 +45,13 @@ export default function WebRTCVideo() { // RTC related states const peerConnection = useRTCStore(state => state.peerConnection); - const peerConnectionState = useRTCStore(state => state.peerConnectionState); // HDMI and UI states const hdmiState = useVideoStore(state => state.hdmiState); const hdmiError = ["no_lock", "no_signal", "out_of_range"].includes(hdmiState); - const isLoading = !hdmiError && !isPlaying; - const isConnectionError = ["error", "failed", "disconnected", "closed"].includes( - peerConnectionState || "", - ); + const isVideoLoading = !isPlaying; + + // console.log("peerConnection?.connectionState", peerConnection?.connectionState); // Keyboard related states const { setIsNumLockActive, setIsCapsLockActive, setIsScrollLockActive } = @@ -379,25 +376,52 @@ export default function WebRTCVideo() { } }, []); + const addStreamToVideoElm = useCallback( + (mediaStream: MediaStream) => { + if (!videoElm.current) return; + const videoElmRefValue = videoElm.current; + console.log("Adding stream to video element", videoElmRefValue); + videoElmRefValue.srcObject = mediaStream; + updateVideoSizeStore(videoElmRefValue); + }, + [updateVideoSizeStore], + ); + + useEffect( + function updateVideoStreamOnNewTrack() { + if (!peerConnection) return; + const abortController = new AbortController(); + const signal = abortController.signal; + + peerConnection.addEventListener( + "track", + (e: RTCTrackEvent) => { + console.log("Adding stream to video element"); + addStreamToVideoElm(e.streams[0]); + }, + { signal }, + ); + + return () => { + abortController.abort(); + }; + }, + [addStreamToVideoElm, peerConnection], + ); + useEffect( function updateVideoStream() { if (!mediaStream) return; - if (!videoElm.current) return; - if (peerConnection?.iceConnectionState !== "connected") return; - - setTimeout(() => { - if (videoElm?.current) { - videoElm.current.srcObject = mediaStream; - } - }, 0); - updateVideoSizeStore(videoElm.current); + console.log("Updating video stream from mediaStream"); + // We set the as early as possible + addStreamToVideoElm(mediaStream); }, [ setVideoClientSize, - setVideoSize, mediaStream, updateVideoSizeStore, - peerConnection?.iceConnectionState, + peerConnection, + addStreamToVideoElm, ], ); @@ -474,6 +498,8 @@ export default function WebRTCVideo() { const local = resetMousePosition; window.addEventListener("blur", local, { signal }); document.addEventListener("visibilitychange", local, { signal }); + const preventContextMenu = (e: MouseEvent) => e.preventDefault(); + videoElmRefValue.addEventListener("contextmenu", preventContextMenu, { signal }); return () => { abortController.abort(); @@ -517,17 +543,17 @@ export default function WebRTCVideo() { ); const hasNoAutoPlayPermissions = useMemo(() => { - if (peerConnectionState !== "connected") return false; + if (peerConnection?.connectionState !== "connected") return false; if (isPlaying) return false; if (hdmiError) return false; if (videoHeight === 0 || videoWidth === 0) return false; return true; - }, [peerConnectionState, isPlaying, hdmiError, videoHeight, videoWidth]); + }, [peerConnection?.connectionState, isPlaying, hdmiError, videoHeight, videoWidth]); return (
    -
    +
    videoElm.current?.requestFullscreen({ @@ -575,28 +601,29 @@ export default function WebRTCVideo() { "cursor-none": settings.mouseMode === "absolute" && settings.isCursorHidden, - "opacity-0": isLoading || isConnectionError || hdmiError, + "opacity-0": isVideoLoading || hdmiError, "animate-slideUpFade border border-slate-800/30 opacity-0 shadow dark:border-slate-300/20": isPlaying, }, )} /> -
    -
    - - - - { - videoElm.current?.play(); - }} - /> + {peerConnection?.connectionState == "connected" && ( +
    +
    + + + { + videoElm.current?.play(); + }} + /> +
    -
    + )}
    diff --git a/ui/src/routes/devices.$id.other-session.tsx b/ui/src/routes/devices.$id.other-session.tsx index 2805666..16cb479 100644 --- a/ui/src/routes/devices.$id.other-session.tsx +++ b/ui/src/routes/devices.$id.other-session.tsx @@ -6,7 +6,7 @@ import LogoBlue from "@/assets/logo-blue.svg"; import LogoWhite from "@/assets/logo-white.svg"; interface ContextType { - connectWebRTC: () => Promise; + setupPeerConnection: () => Promise; } /* TODO: Migrate to using URLs instead of the global state. To simplify the refactoring, we'll keep the global state for now. */ @@ -16,7 +16,7 @@ export default function OtherSessionRoute() { // Function to handle closing the modal const handleClose = () => { - outletContext?.connectWebRTC().then(() => navigate("..")); + outletContext?.setupPeerConnection().then(() => navigate("..")); }; return ( diff --git a/ui/src/routes/devices.$id.tsx b/ui/src/routes/devices.$id.tsx index 50fc79f..d2662fc 100644 --- a/ui/src/routes/devices.$id.tsx +++ b/ui/src/routes/devices.$id.tsx @@ -45,6 +45,10 @@ import Modal from "../components/Modal"; import { useDeviceUiNavigation } from "../hooks/useAppNavigation"; import { FeatureFlagProvider } from "../providers/FeatureFlagProvider"; import notifications from "../notifications"; +import { + ConnectionErrorOverlay, + LoadingConnectionOverlay, +} from "../components/VideoOverlay"; import { SystemVersionInfo } from "./devices.$id.settings.general.update"; import { DeviceStatus } from "./welcome-local"; @@ -126,8 +130,6 @@ 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); const setDiskChannel = useRTCStore(state => state.setDiskChannel); @@ -135,78 +137,55 @@ export default function KvmIdRoute() { const setTransceiver = useRTCStore(state => state.setTransceiver); const location = useLocation(); - const [connectionAttempts, setConnectionAttempts] = useState(0); - - const [startedConnectingAt, setStartedConnectingAt] = useState(null); - const [connectedAt, setConnectedAt] = useState(null); - const [connectionFailed, setConnectionFailed] = useState(false); const navigate = useNavigate(); const { otaState, setOtaState, setModalView } = useUpdateStore(); + const [loadingMessage, setLoadingMessage] = useState("Connecting to device..."); const closePeerConnection = useCallback( function closePeerConnection() { + console.log("Closing peer connection"); + + setConnectionFailed(true); + connectionFailedRef.current = true; + peerConnection?.close(); - // "closed" is a valid RTCPeerConnection state according to the WebRTC spec - // https://developer.mozilla.org/en-US/docs/Web/API/RTCPeerConnection/connectionState#closed - // 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"); + signalingAttempts.current = 0; }, - [peerConnection, setPeerConnectionState], + [peerConnection], ); + // We need to track connectionFailed in a ref to avoid stale closure issues + // This is necessary because syncRemoteSessionDescription is a callback that captures + // the connectionFailed value at creation time, but we need the latest value + // when the function is actually called. Without this ref, the function would use + // a stale value of connectionFailed in some conditions. + // + // We still need the state variable for UI rendering, so we sync the ref with the state. + // This pattern is a workaround for what useEvent hook would solve more elegantly + // (which would give us a callback that always has access to latest state without re-creation). + const connectionFailedRef = useRef(false); useEffect(() => { - const connectionAttemptsThreshold = 30; - if (connectionAttempts > connectionAttemptsThreshold) { - console.log(`Connection failed after ${connectionAttempts} attempts.`); - setConnectionFailed(true); - closePeerConnection(); - } - }, [connectionAttempts, closePeerConnection]); - - useEffect(() => { - // Skip if already connected - if (connectedAt) return; - - // Skip if connection is declared as failed - if (connectionFailed) return; - - const interval = setInterval(() => { - console.log("Checking connection status"); - - // Skip if connection hasn't started - if (!startedConnectingAt) return; - - const elapsedTime = Math.floor( - new Date().getTime() - startedConnectingAt.getTime(), - ); - - // Fail connection if it's been over X seconds since we started connecting - if (elapsedTime > 60 * 1000) { - console.error(`Connection failed after ${elapsedTime} ms.`); - setConnectionFailed(true); - closePeerConnection(); - } - }, 1000); - - return () => clearInterval(interval); - }, [closePeerConnection, connectedAt, connectionFailed, startedConnectingAt]); - - const sdp = useCallback( - async (event: RTCPeerConnectionIceEvent, pc: RTCPeerConnection) => { - if (!pc) return; - if (event.candidate !== null) return; + connectionFailedRef.current = connectionFailed; + }, [connectionFailed]); + const signalingAttempts = useRef(0); + const syncRemoteSessionDescription = useCallback( + async function syncRemoteSessionDescription(pc: RTCPeerConnection) { try { + if (!pc) return; + const sd = btoa(JSON.stringify(pc.localDescription)); const sessionUrl = isOnDevice ? `${DEVICE_API}/webrtc/session` : `${CLOUD_API}/webrtc/session`; + + console.log("Trying to get remote session description"); + setLoadingMessage( + `Getting remote session description... ${signalingAttempts.current > 0 ? `(attempt ${signalingAttempts.current + 1})` : ""}`, + ); const res = await api.POST(sessionUrl, { sd, // When on device, we don't need to specify the device id, as it's already known @@ -214,73 +193,109 @@ export default function KvmIdRoute() { }); const json = await res.json(); - - if (isOnDevice) { - if (res.status === 401) { - return navigate("/login-local"); - } + if (res.status === 401) return navigate(isOnDevice ? "/login-local" : "/login"); + if (!res.ok) { + console.error("Error getting SDP", { status: res.status, json }); + throw new Error("Error getting SDP"); } - if (isInCloud) { - // The cloud API returns a 401 if the user is not logged in - // Most likely the session has expired - if (res.status === 401) return navigate("/login"); + console.log("Successfully got Remote Session Description. Setting."); + setLoadingMessage("Setting remote session description..."); - // If can be a few things - // - In cloud mode, the cloud api would return a 404, if the device hasn't contacted the cloud yet - // - In device mode, the device api would timeout, the fetch would throw an error, therefore the catch block would be hit - // Regardless, we should close the peer connection and let the useInterval handle reconnecting - if (!res.ok) { - closePeerConnection(); - console.error(`Error setting SDP - Status: ${res.status}}`, json); - return; - } - } + const decodedSd = atob(json.sd); + const parsedSd = JSON.parse(decodedSd); + pc.setRemoteDescription(new RTCSessionDescription(parsedSd)); - pc.setRemoteDescription( - new RTCSessionDescription(JSON.parse(atob(json.sd))), - ).catch(e => console.log(`Error setting remote description: ${e}`)); + await new Promise((resolve, reject) => { + console.log("Waiting for remote description to be set"); + const maxAttempts = 10; + const interval = 1000; + let attempts = 0; + + const checkInterval = setInterval(() => { + attempts++; + // When vivaldi has disabled "Broadcast IP for Best WebRTC Performance", this never connects + if (pc.sctp?.state === "connected") { + console.log("Remote description set"); + clearInterval(checkInterval); + resolve(true); + } else if (attempts >= maxAttempts) { + console.log( + `Failed to get remote description after ${maxAttempts} attempts`, + ); + closePeerConnection(); + clearInterval(checkInterval); + reject( + new Error( + `Failed to get remote description after ${maxAttempts} attempts`, + ), + ); + } else { + console.log("Waiting for remote description to be set"); + } + }, interval); + }); } catch (error) { - console.error(`Error setting SDP: ${error}`); - closePeerConnection(); + console.error("Error getting SDP", { error }); + console.log("Connection failed", connectionFailedRef.current); + if (connectionFailedRef.current) return; + if (signalingAttempts.current < 5) { + signalingAttempts.current++; + await new Promise(resolve => setTimeout(resolve, 500)); + console.log("Attempting to get SDP again", signalingAttempts.current); + syncRemoteSessionDescription(pc); + } else { + closePeerConnection(); + } } }, [closePeerConnection, navigate, params.id], ); - const connectWebRTC = useCallback(async () => { - console.log("Attempting to connect WebRTC"); - - // Track connection status to detect failures and show error overlay - setConnectionAttempts(x => x + 1); - setStartedConnectingAt(new Date()); - setConnectedAt(null); + const setupPeerConnection = useCallback(async () => { + console.log("Setting up peer connection"); + setConnectionFailed(false); + setLoadingMessage("Connecting to device..."); let pc: RTCPeerConnection; try { + console.log("Creating peer connection"); + setLoadingMessage("Creating peer connection..."); pc = new RTCPeerConnection({ // We only use STUN or TURN servers if we're in the cloud ...(isInCloud && iceConfig?.iceServers ? { iceServers: [iceConfig?.iceServers] } : {}), }); + console.log("Peer connection created", pc); + setLoadingMessage("Peer connection created"); } catch (e) { console.error(`Error creating peer connection: ${e}`); - closePeerConnection(); + setTimeout(() => { + closePeerConnection(); + }, 1000); return; } // Set up event listeners and data channels pc.onconnectionstatechange = () => { - // If the connection state is connected, we reset the connection attempts. - if (pc.connectionState === "connected") { - setConnectionAttempts(0); - setConnectedAt(new Date()); - } - setPeerConnectionState(pc.connectionState); + console.log("Connection state changed", pc.connectionState); }; - pc.onicecandidate = event => sdp(event, pc); + pc.onicegatheringstatechange = event => { + const pc = event.currentTarget as RTCPeerConnection; + console.log("ICE Gathering State Changed", pc.iceGatheringState); + if (pc.iceGatheringState === "complete") { + console.log("ICE Gathering completed"); + setLoadingMessage("ICE Gathering completed"); + + // We can now start the https/ws connection to get the remote session description from the KVM device + syncRemoteSessionDescription(pc); + } else if (pc.iceGatheringState === "gathering") { + console.log("ICE Gathering Started"); + setLoadingMessage("Gathering ICE candidates..."); + } + }; pc.ontrack = function (event) { setMediaMediaStream(event.streams[0]); @@ -298,56 +313,32 @@ export default function KvmIdRoute() { setDiskChannel(diskDataChannel); }; + setPeerConnection(pc); + try { const offer = await pc.createOffer(); await pc.setLocalDescription(offer); - setPeerConnection(pc); } catch (e) { - console.error(`Error creating offer: ${e}`); + console.error(`Error creating offer: ${e}`, new Date().toISOString()); closePeerConnection(); } }, [ closePeerConnection, iceConfig?.iceServers, - sdp, setDiskChannel, setMediaMediaStream, setPeerConnection, - setPeerConnectionState, setRpcDataChannel, setTransceiver, + syncRemoteSessionDescription, ]); - useEffect(() => { - console.log("Attempting to connect WebRTC"); - - // If we're in an other session, we don't need to connect - if (location.pathname.includes("other-session")) return; - - // If we're already connected or connecting, we don't need to connect - // 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; - } - - // In certain cases, we want to never connect again. This happens when we've tried for a long time and failed - if (connectionFailed) { - console.log("Connection failed. We won't attempt to connect again."); - return; - } - - const interval = setInterval(() => { - connectWebRTC(); - }, 3000); - return () => clearInterval(interval); - }, [connectWebRTC, connectionFailed, location.pathname, peerConnectionState]); - // On boot, if the connection state is undefined, we connect to the WebRTC useEffect(() => { if (peerConnection?.connectionState === undefined) { - connectWebRTC(); + setupPeerConnection(); } - }, [connectWebRTC, peerConnection?.connectionState]); + }, [setupPeerConnection, peerConnection?.connectionState]); // Cleanup effect const clearInboundRtpStats = useRTCStore(state => state.clearInboundRtpStats); @@ -601,7 +592,27 @@ export default function KvmIdRoute() { kvmName={deviceName || "JetKVM Device"} /> -
    +
    +
    +
    + + +
    +
    +
    @@ -618,7 +629,7 @@ export default function KvmIdRoute() { > {/* The 'used by other session' modal needs to have access to the connectWebRTC function */} - +