Skip to content

Commit

Permalink
add retry mechanism before terminating an unresponsive debugger socket (
Browse files Browse the repository at this point in the history
#44811)

Summary:
Pull Request resolved: #44811

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
  • Loading branch information
EdmondChuiHW committed Jun 6, 2024
1 parent a569c82 commit 6e262d3
Showing 1 changed file with 27 additions and 0 deletions.
27 changes: 27 additions & 0 deletions packages/dev-middleware/src/inspector-proxy/InspectorProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand Down Expand Up @@ -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) {
Expand All @@ -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.
Expand All @@ -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);
});
Expand Down

0 comments on commit 6e262d3

Please sign in to comment.