From d4a10c7b417cda143e8e4393344bab86eeb41ff7 Mon Sep 17 00:00:00 2001
From: Antoine Clausse <antoine.clausse@overleaf.com>
Date: Mon, 20 Jan 2025 11:04:30 +0100
Subject: [PATCH] [web] Socket diagnostics updates (#22951)

* Increase threshold for "latency in red color"

* Fix online status in Chrome and Safari

* Add "Auto ping" checkbox

* Put `/socket-diagnostics` behind `AuthenticationController.requireLogin`

* Set logs to `logger.info` when debugging

* Add `publicId` and `clientId` to logs

* Fix disconnect logs when debugging

* Refresh UI every second. Display red "Ping Count" if unanswered for 3s

* Update services/web/frontend/js/features/socket-diagnostics/components/socket-diagnostics.tsx

Co-authored-by: Jakob Ackermann <jakob.ackermann@overleaf.com>

* Update services/web/frontend/js/features/socket-diagnostics/components/socket-diagnostics.tsx

Co-authored-by: Jakob Ackermann <jakob.ackermann@overleaf.com>

* `npm run format:fix`

---------

Co-authored-by: Jakob Ackermann <jakob.ackermann@overleaf.com>
GitOrigin-RevId: 9faf2abdac51fa4b87c67d8fe89c4125d01d826f
---
 services/real-time/app/js/Router.js           | 31 ++++++--
 services/web/app/src/router.mjs               |  6 +-
 .../components/socket-diagnostics.tsx         | 48 +++++++++----
 .../socket-diagnostics/components/types.ts    |  1 +
 .../components/use-socket-manager.ts          | 70 ++++++++++++-------
 5 files changed, 109 insertions(+), 47 deletions(-)

diff --git a/services/real-time/app/js/Router.js b/services/real-time/app/js/Router.js
index 37845015b0..756732112d 100644
--- a/services/real-time/app/js/Router.js
+++ b/services/real-time/app/js/Router.js
@@ -127,7 +127,10 @@ module.exports = Router = {
 
       if (client) {
         client.on('error', function (err) {
-          logger.err({ clientErr: err }, 'socket.io client error')
+          logger.err(
+            { clientErr: err, publicId: client.publicId, clientId: client.id },
+            'socket.io client error'
+          )
           if (client.connected) {
             client.emit('reconnectGracefully')
             client.disconnect()
@@ -174,6 +177,7 @@ module.exports = Router = {
 
       if (isDebugging) {
         client.connectedAt = Date.now()
+        client.isDebugging = true
       }
 
       if (!isDebugging) {
@@ -205,10 +209,17 @@ module.exports = Router = {
       })
       metrics.gauge('socket-io.clients', io.sockets.clients().length)
 
-      logger.debug(
-        { session, clientId: client.id, isDebugging },
-        'client connected'
-      )
+      const info = {
+        session,
+        publicId: client.publicId,
+        clientId: client.id,
+        isDebugging,
+      }
+      if (isDebugging) {
+        logger.info(info, 'client connected')
+      } else {
+        logger.debug(info, 'client connected')
+      }
 
       let user
       if (session && session.passport && session.passport.user) {
@@ -237,7 +248,10 @@ module.exports = Router = {
           return Router._handleInvalidArguments(client, 'debug', arguments)
         }
 
-        logger.debug({ clientId: client.id }, 'received debug message')
+        logger.info(
+          { publicId: client.publicId, clientId: client.id },
+          'received debug message'
+        )
 
         const response = {
           serverTime: Date.now(),
@@ -281,7 +295,10 @@ module.exports = Router = {
         if (client.isDebugging) {
           const duration = Date.now() - client.connectedAt
           metrics.timing('socket-io.debugging.duration', duration)
-          logger.debug({ duration }, 'debug client disconnected')
+          logger.info(
+            { duration, publicId: client.publicId, clientId: client.id },
+            'debug client disconnected'
+          )
         }
 
         WebsocketController.leaveProject(io, client, function (err) {
diff --git a/services/web/app/src/router.mjs b/services/web/app/src/router.mjs
index 125fdfd385..ab322fc8f4 100644
--- a/services/web/app/src/router.mjs
+++ b/services/web/app/src/router.mjs
@@ -232,7 +232,11 @@ async function initialize(webRouter, privateApiRouter, publicApiRouter) {
 
   webRouter.get('/account-suspended', UserPagesController.accountSuspended)
 
-  webRouter.get('/socket-diagnostics', SocketDiagnostics.index)
+  webRouter.get(
+    '/socket-diagnostics',
+    AuthenticationController.requireLogin(),
+    SocketDiagnostics.index
+  )
 
   if (Settings.enableLegacyLogin) {
     AuthenticationController.addEndpointToLoginWhitelist('/login/legacy')
diff --git a/services/web/frontend/js/features/socket-diagnostics/components/socket-diagnostics.tsx b/services/web/frontend/js/features/socket-diagnostics/components/socket-diagnostics.tsx
index 03d16427cf..cbf16bcb4e 100644
--- a/services/web/frontend/js/features/socket-diagnostics/components/socket-diagnostics.tsx
+++ b/services/web/frontend/js/features/socket-diagnostics/components/socket-diagnostics.tsx
@@ -1,4 +1,4 @@
-import React from 'react'
+import React, { useEffect } from 'react'
 import type { ConnectionStatus } from './types'
 import { useSocketManager } from './use-socket-manager'
 import {
@@ -9,6 +9,7 @@ import {
 } from './diagnostic-component'
 import { Container } from 'react-bootstrap-5'
 import MaterialIcon from '@/shared/components/material-icon'
+import OLFormCheckbox from '@/features/ui/components/ol/ol-form-checkbox'
 
 type NetworkInformation = {
   downlink: number
@@ -38,9 +39,26 @@ const NavigatorInfo = () => {
   )
 }
 
+const useCurrentTime = () => {
+  const [time, setTime] = React.useState(new Date())
+  useEffect(() => {
+    const interval = setInterval(() => setTime(new Date()), 1000)
+    return () => clearInterval(interval)
+  }, [])
+  return time
+}
+
 export const SocketDiagnostics = () => {
-  const { socketState, debugInfo, disconnectSocket, forceReconnect, socket } =
-    useSocketManager()
+  const {
+    socketState,
+    debugInfo,
+    disconnectSocket,
+    forceReconnect,
+    socket,
+    autoping,
+    setAutoping,
+  } = useSocketManager()
+  const now = useCurrentTime()
 
   const getConnectionState = (): ConnectionStatus => {
     if (socketState.connected) return 'connected'
@@ -49,9 +67,13 @@ export const SocketDiagnostics = () => {
   }
 
   const lastReceivedS = debugInfo.lastReceived
-    ? Math.round((Date.now() - debugInfo.lastReceived) / 1000)
+    ? Math.round((now.getTime() - debugInfo.lastReceived) / 1000)
     : null
 
+  const isLate =
+    !!debugInfo.unansweredSince &&
+    now.getTime() - debugInfo.unansweredSince >= 3000
+
   return (
     <Container>
       <h1>Socket Diagnostics</h1>
@@ -78,6 +100,12 @@ export const SocketDiagnostics = () => {
           <MaterialIcon type="speed" /> Connection Stats
         </h3>
         <div className="space-y-2">
+          <OLFormCheckbox
+            label="Auto ping"
+            id="autoping"
+            checked={autoping}
+            onChange={e => setAutoping(e.target.checked)}
+          />
           <DiagnosticItem
             icon="network_ping"
             label="Ping Count"
@@ -92,13 +120,7 @@ export const SocketDiagnostics = () => {
                 )}
               </>
             }
-            type={
-              lastReceivedS !== null
-                ? lastReceivedS < 4
-                  ? 'success'
-                  : 'danger'
-                : undefined
-            }
+            type={isLate === null ? undefined : isLate ? 'danger' : 'success'}
           />
 
           <DiagnosticItem
@@ -117,7 +139,7 @@ export const SocketDiagnostics = () => {
             }
             type={
               debugInfo.latency
-                ? debugInfo.latency < 150
+                ? debugInfo.latency < 450
                   ? 'success'
                   : 'danger'
                 : undefined
@@ -149,7 +171,7 @@ export const SocketDiagnostics = () => {
           <DiagnosticItem
             icon="schedule"
             label="Current time"
-            value={new Date().toUTCString()}
+            value={now.toUTCString()}
           />
           <DiagnosticItem
             icon="hourglass"
diff --git a/services/web/frontend/js/features/socket-diagnostics/components/types.ts b/services/web/frontend/js/features/socket-diagnostics/components/types.ts
index edfc9bdf44..159948af29 100644
--- a/services/web/frontend/js/features/socket-diagnostics/components/types.ts
+++ b/services/web/frontend/js/features/socket-diagnostics/components/types.ts
@@ -12,6 +12,7 @@ export interface DebugInfo {
   clockDelta: number | null
   onLine: boolean | null
   client: Client | null
+  unansweredSince: number | null
   lastReceived: number | null
 }
 
diff --git a/services/web/frontend/js/features/socket-diagnostics/components/use-socket-manager.ts b/services/web/frontend/js/features/socket-diagnostics/components/use-socket-manager.ts
index 4ff97cc7c1..a0035052f5 100644
--- a/services/web/frontend/js/features/socket-diagnostics/components/use-socket-manager.ts
+++ b/services/web/frontend/js/features/socket-diagnostics/components/use-socket-manager.ts
@@ -5,6 +5,7 @@ import type { DebugInfo, SocketState } from './types'
 
 export function useSocketManager() {
   const [socket, setSocket] = useState<Socket | null>(null)
+  const [autoping, setAutoping] = useState<boolean>(false)
 
   const [socketState, setSocketState] = useState<SocketState>({
     connected: false,
@@ -20,6 +21,7 @@ export function useSocketManager() {
     onLine: null,
     clockDelta: null,
     client: null,
+    unansweredSince: null,
     lastReceived: null,
   })
 
@@ -65,30 +67,46 @@ export function useSocketManager() {
     connectSocket()
   }, [connectSocket])
 
+  const sendPing = useCallback(() => {
+    if (socket?.socket.connected) {
+      const time = Date.now()
+      setDebugInfo(prev => ({
+        ...prev,
+        sent: prev.sent + 1,
+        unansweredSince: prev.unansweredSince ?? time,
+      }))
+      socket.emit('debug', { time }, (info: any) => {
+        const beforeTime = info.data.time
+        const now = Date.now()
+        const latency = now - beforeTime
+        const clockDelta = (beforeTime + beforeTime) / 2 - info.serverTime
+        setDebugInfo(prev => ({
+          ...prev,
+          received: prev.received + 1,
+          latency,
+          maxLatency: Math.max(prev.maxLatency ?? 0, latency),
+          clockDelta,
+          client: info.client,
+          lastReceived: now,
+          unansweredSince: null,
+        }))
+      })
+    }
+  }, [socket])
+
+  useEffect(() => {
+    if (!socket || !autoping) return
+
+    const statsInterval = setInterval(sendPing, 2000)
+
+    return () => {
+      clearInterval(statsInterval)
+    }
+  }, [socket, autoping, sendPing])
+
   useEffect(() => {
     if (!socket) return
 
-    const statsInterval = setInterval(() => {
-      if (socket.socket.connected) {
-        setDebugInfo(prev => ({ ...prev, sent: prev.sent + 1 }))
-        socket.emit('debug', { time: Date.now() }, (info: any) => {
-          const beforeTime = info.data.time
-          const now = Date.now()
-          const latency = now - beforeTime
-          const clockDelta = (beforeTime + beforeTime) / 2 - info.serverTime
-          setDebugInfo(prev => ({
-            ...prev,
-            received: prev.received + 1,
-            latency,
-            maxLatency: Math.max(prev.maxLatency ?? 0, latency),
-            clockDelta,
-            client: info.client,
-            lastReceived: now,
-          }))
-        })
-      }
-    }, 2000)
-
     socket.on('connect', () => {
       setSocketState(prev => ({
         ...prev,
@@ -97,6 +115,7 @@ export function useSocketManager() {
         lastSuccess: Date.now(),
         lastError: '',
       }))
+      sendPing()
     })
 
     socket.on('disconnect', (reason: string) => {
@@ -119,16 +138,13 @@ export function useSocketManager() {
     socket.socket.connect()
 
     return () => {
-      clearInterval(statsInterval)
       socket.disconnect()
     }
-  }, [socket])
+  }, [sendPing, socket])
 
   useEffect(() => {
     const updateNetworkInfo = () => {
-      if ('connection' in navigator) {
-        setDebugInfo(prev => ({ ...prev, onLine: navigator.onLine }))
-      }
+      setDebugInfo(prev => ({ ...prev, onLine: navigator.onLine }))
     }
 
     window.addEventListener('online', updateNetworkInfo)
@@ -148,5 +164,7 @@ export function useSocketManager() {
     disconnectSocket,
     forceReconnect,
     socket,
+    autoping,
+    setAutoping,
   }
 }