From 1bcbed31da89c23ab6db77f32b1e95bfea4c0260 Mon Sep 17 00:00:00 2001 From: Edmond Chui Date: Wed, 5 Jun 2024 18:59:19 -0700 Subject: [PATCH] add retry mechanism before terminating an unresponsive debugger socket Summary: Changelog: [General][Added] - New messages for debugger disconnections [General][Changed] - Temporary disconnections with the debugger no longer terminates the debugger immediately The debugger is currently disconnected if a ping-pong message is missed. This causes the debugger to be unusable if it happens to be lagging, e.g. when the initialisation is competing with the flood of log spam T191394188 There are a few ways to fix this as discused with motiz88 and robhogan: 1. Ensure the websocket has a chance to respond, e.g. in via web worker 1. Lengthen the time allowed for the pong resopnse I've done some digging to find the root cause of the UI being blocked in CDT, However, profiling shows that most of the work is not simple to break up, i.e. the number of expensive re-layout calls. Diving into that rabbit hole could mean accidentally writing React. Because we ping every 10 seconds, we could get un/lucky where CDT happens to be busy _at that exact moment_, making this a flaky symptom to fix, even if we lengthen the allowed time-to-respond. In this diff, a more forgiving mechanism is introduced, i.e. CDT is allowed to miss a ping-pong roundtrip 3 times before the websocket connection is terminated. This allows a bit more breathing room for CDT's initialisation during log spam while maintaining the same ping-pong interval for VS Code to keep the auto SSH tunnel alive. Differential Revision: D58220230 --- .../src/inspector-proxy/InspectorProxy.js | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js index 6df32d2ea5acff..b531b8962b996b 100644 --- a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js +++ b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js @@ -23,6 +23,8 @@ import type {IncomingMessage, ServerResponse} from 'http'; import type {Timeout} from 'timers'; import Device from './Device'; +import {logger} from '@react-native-community/cli-tools'; +import chalk from 'chalk'; import nullthrows from 'nullthrows'; // Import these from node:timers to get the correct Flow types. // $FlowFixMe[cannot-resolve-module] libdef missing in RN OSS @@ -38,6 +40,7 @@ const PAGES_LIST_JSON_URL = '/json'; const PAGES_LIST_JSON_URL_2 = '/json/list'; const PAGES_LIST_JSON_VERSION_URL = '/json/version'; const MAX_PONG_LATENCY_MS = 5000; +const MAX_MISSED_PONG_COUNT = 3; const DEBUGGER_HEARTBEAT_INTERVAL_MS = 10000; const INTERNAL_ERROR_CODE = 1011; @@ -297,6 +300,7 @@ export default class InspectorProxy implements InspectorProxyQueries { // https://datatracker.ietf.org/doc/html/rfc6455#section-5.5.2 #startHeartbeat(socket: WS, intervalMs: number) { let terminateTimeout = null; + let missedPongCount = 0; const pingTimeout: Timeout = setTimeout(() => { if (socket.readyState !== WS.OPEN) { @@ -309,6 +313,16 @@ export default class InspectorProxy implements InspectorProxyQueries { if (socket.readyState !== WS.OPEN) { return; } + if (missedPongCount < MAX_MISSED_PONG_COUNT) { + missedPongCount++; + + logger.warn( + `Temporarily lost connection to the debugger. Reconnecting in ${intervalMs / 1000} seconds (attempt ${missedPongCount} of ${MAX_MISSED_PONG_COUNT})…`, + ); + pingTimeout.refresh(); + return; + } + // We don't use close() here because that initiates a closing handshake, // which will not complete if the other end has gone away - 'close' // would not be emitted. @@ -320,11 +334,24 @@ export default class InspectorProxy implements InspectorProxyQueries { }, intervalMs).unref(); socket.on('pong', () => { + if (missedPongCount > 0) { + logger.info('Connection to the debugger restored.'); + missedPongCount = 0; + } terminateTimeout && clearTimeout(terminateTimeout); pingTimeout.refresh(); }); socket.on('close', () => { + if (missedPongCount > 0) { + logger.error( + `Couldn't reconnect to the debugger after ${missedPongCount} attempt${missedPongCount > 1 ? 's' : ''}.`, + ); + missedPongCount = 0; + } + logger.info( + `The debugger has disconnected. You may restart the debugger by typing ${chalk.bold('j')} in the terminal.`, + ); terminateTimeout && clearTimeout(terminateTimeout); clearTimeout(pingTimeout); });