Skip to content

Commit 4e7b77e

Browse files
vzaidmanfacebook-github-bot
authored andcommitted
create a heartbeat for device, and not only for debugger (facebook#49618)
Summary: Pull Request resolved: facebook#49618 Changelog: [General][Internal] - create a heartbeat for device, and not only for debugger. Introducing a heartbeat for the connection between the proxy and the device similarly to the one between the proxy and the debugger. This will allow us to: * Most importantly I'd like to track the ping-pong roundtrip to the device as well, to see if we have any anomalies there * Terminate the connection if it is abandoned for 60seconds- this might have a real effect in some case where the device runs remotely * Also keep that connection alive if the other side disconnects after a period of inactivity. While a no-op in our case, this is an implementation detail. It is a no-op because the WebSocket on the Device is implemented by us and is not supposed to drop connections like the browser does. Reviewed By: robhogan Differential Revision: D69990715 fbshipit-source-id: 6bb3a2ed3eaffff9535aa2d0fc8cff0262af022f
1 parent 56ed15e commit 4e7b77e

File tree

2 files changed

+88
-36
lines changed

2 files changed

+88
-36
lines changed

packages/dev-middleware/src/inspector-proxy/InspectorProxy.js

Lines changed: 85 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,8 @@ const WS_DEBUGGER_URL = '/inspector/debug';
4040
const PAGES_LIST_JSON_URL = '/json';
4141
const PAGES_LIST_JSON_URL_2 = '/json/list';
4242
const PAGES_LIST_JSON_VERSION_URL = '/json/version';
43-
const DEBUGGER_TIMEOUT_MS = 60000;
44-
const DEBUGGER_HEARTBEAT_INTERVAL_MS = 10000;
43+
const HEARTBEAT_TIMEOUT_MS = 60000;
44+
const HEARTBEAT_INTERVAL_MS = 10000;
4545

4646
const INTERNAL_ERROR_CODE = 1011;
4747

@@ -126,7 +126,8 @@ export default class InspectorProxy implements InspectorProxyQueries {
126126
device.dangerouslyGetSocket()?.readyState === WS.OPEN
127127
) {
128128
this.#logger?.warn(
129-
`Waiting for a DevTools connection to app '%s' on device '%s'. If no connection occurs, try:
129+
`Waiting for a DevTools connection to app='%s' on device='%s'.
130+
Try again when it's established. If no connection occurs, try to:
130131
- Restart the app
131132
- Ensure a stable connection to the device
132133
- Ensure that the app is built in a mode that supports debugging`,
@@ -288,35 +289,58 @@ export default class InspectorProxy implements InspectorProxyQueries {
288289
this.#devices.set(deviceId, newDevice);
289290

290291
this.#logger?.info(
291-
"Connection established to app '%s' on device '%s'.",
292+
"Connection established to app='%s' on device='%s'.",
292293
appName,
293294
deviceName,
294295
);
295296

296297
debug(
297-
'Got new connection: name=%s, app=%s, device=%s, via=%s',
298+
"Got new device connection: name='%s', app=%s, device=%s, via=%s",
298299
deviceName,
299300
appName,
300301
deviceId,
301302
deviceRelativeBaseUrl.origin,
302303
);
303304

305+
const debuggerSessionIDs: DebuggerSessionIDs = {
306+
appId: newDevice?.getApp() || null,
307+
deviceId,
308+
deviceName: newDevice?.getName() || null,
309+
pageId: null,
310+
};
311+
312+
this.#startHeartbeat({
313+
socketName: 'Device',
314+
socket,
315+
intervalMs: HEARTBEAT_INTERVAL_MS,
316+
debuggerSessionIDs,
317+
timeoutEventName: 'device_timeout',
318+
heartbeatEventName: 'device_heartbeat',
319+
});
320+
304321
socket.on('close', (code: number, reason: string) => {
305322
this.#logger?.info(
306-
"Connection closed to app '%s' on device '%s' with code '%s' and reason '%s'.",
307-
appName,
323+
"Connection closed to device='%s' for app='%s' with code='%s' and reason='%s'.",
308324
deviceName,
325+
appName,
309326
String(code),
310327
reason,
311328
);
312329

330+
this.#eventReporter?.logEvent({
331+
type: 'device_connection_closed',
332+
code,
333+
reason,
334+
...debuggerSessionIDs,
335+
});
336+
313337
if (this.#devices.get(deviceId)?.dangerouslyGetSocket() === socket) {
314338
this.#devices.delete(deviceId);
315339
}
316340
});
317341
} catch (error) {
318342
this.#logger?.error(
319-
"Connection failed to be established with app '%s' on device '%s' with error:",
343+
"Connection failed to be established with app='%s' on device='%s' with error:",
320344
appName,
321345
deviceName,
322346
error,
@@ -367,21 +391,47 @@ export default class InspectorProxy implements InspectorProxyQueries {
367391
throw new Error('Unknown device with ID ' + deviceId);
368392
}
369393

370-
this.#logger?.info('Connection established to DevTools.');
394+
this.#logger?.info(
395+
"Connection established to DevTools for app='%s' on device='%s'.",
396+
device.getApp() || 'unknown',
397+
device.getName() || 'unknown',
398+
);
371399

372-
this.#startHeartbeat(
400+
this.#startHeartbeat({
401+
socketName: 'DevTools',
373402
socket,
374-
DEBUGGER_HEARTBEAT_INTERVAL_MS,
403+
intervalMs: HEARTBEAT_INTERVAL_MS,
375404
debuggerSessionIDs,
376-
);
405+
timeoutEventName: 'debugger_timeout',
406+
heartbeatEventName: 'debugger_heartbeat',
407+
});
377408

378409
device.handleDebuggerConnection(socket, pageId, {
379410
debuggerRelativeBaseUrl,
380411
userAgent: req.headers['user-agent'] ?? query.userAgent ?? null,
381412
});
413+
414+
socket.on('close', (code: number, reason: string) => {
415+
this.#logger?.info(
416+
"Connection closed to DevTools for app='%s' on device='%s' with code='%s' and reason='%s'.",
417+
device.getApp() || 'unknown',
418+
device.getName() || 'unknown',
419+
String(code),
420+
reason,
421+
);
422+
423+
this.#eventReporter?.logEvent({
424+
type: 'debugger_connection_closed',
425+
code,
426+
reason,
427+
...debuggerSessionIDs,
428+
});
429+
});
382430
} catch (error) {
383431
this.#logger?.error(
384-
'Connection failed to be established with DevTools with error:',
432+
"Connection failed to be established with DevTools for app='%s' on device='%s' with error:",
433+
device?.getApp() || 'unknown',
434+
device?.getName() || 'unknown',
385435
error,
386436
);
387437
socket.close(INTERNAL_ERROR_CODE, error?.toString() ?? 'Unknown error');
@@ -402,11 +452,21 @@ export default class InspectorProxy implements InspectorProxyQueries {
402452
// where proxies may drop idle connections (e.g., VS Code tunnels).
403453
//
404454
// https://datatracker.ietf.org/doc/html/rfc6455#section-5.5.2
405-
#startHeartbeat(
455+
#startHeartbeat({
456+
socketName,
457+
socket,
458+
intervalMs,
459+
debuggerSessionIDs,
460+
timeoutEventName,
461+
heartbeatEventName,
462+
}: {
463+
socketName: string,
406464
socket: WS,
407465
intervalMs: number,
408466
debuggerSessionIDs: DebuggerSessionIDs,
409-
) {
467+
timeoutEventName: 'debugger_timeout' | 'device_timeout',
468+
heartbeatEventName: 'debugger_heartbeat' | 'device_heartbeat',
469+
}) {
410470
let latestPingMs = Date.now();
411471
let terminateTimeout: ?Timeout;
412472

@@ -433,16 +493,17 @@ export default class InspectorProxy implements InspectorProxyQueries {
433493
socket.terminate();
434494

435495
this.#logger?.error(
436-
'Connection terminated with DevTools after not responding for %s seconds.',
437-
String(DEBUGGER_TIMEOUT_MS / 1000),
496+
"Connection terminated with %s for app='%s' on device='%s' after not responding for %s seconds.",
497+
socketName,
498+
String(HEARTBEAT_TIMEOUT_MS / 1000),
438499
);
439500

440501
this.#eventReporter?.logEvent({
441-
type: 'debugger_timeout',
442-
duration: DEBUGGER_TIMEOUT_MS,
502+
type: timeoutEventName,
503+
duration: HEARTBEAT_TIMEOUT_MS,
443504
...debuggerSessionIDs,
444505
});
445-
}, DEBUGGER_TIMEOUT_MS).unref();
506+
}, HEARTBEAT_TIMEOUT_MS).unref();
446507
}
447508

448509
latestPingMs = Date.now();
@@ -453,13 +514,15 @@ export default class InspectorProxy implements InspectorProxyQueries {
453514
const roundtripDuration = Date.now() - latestPingMs;
454515

455516
debug(
456-
'debugger ping-pong for %s took %dms.',
517+
'%s heartbeat ping-pong for %s on %s took %dms.',
518+
socketName,
457519
debuggerSessionIDs.appId,
520+
debuggerSessionIDs.deviceName,
458521
roundtripDuration,
459522
);
460523

461524
this.#eventReporter?.logEvent({
462-
type: 'debugger_heartbeat',
525+
type: heartbeatEventName,
463526
duration: roundtripDuration,
464527
...debuggerSessionIDs,
465528
});
@@ -473,17 +536,6 @@ export default class InspectorProxy implements InspectorProxyQueries {
473536
});
474537

475538
socket.on('close', (code: number, reason: string) => {
476-
this.#logger?.info(
477-
"Connection closed to DevTools with code '%s' and reason '%s'.",
478-
String(code),
479-
reason,
480-
);
481-
this.#eventReporter?.logEvent({
482-
type: 'debugger_connection_closed',
483-
code,
484-
reason,
485-
...debuggerSessionIDs,
486-
});
487539
terminateTimeout && clearTimeout(terminateTimeout);
488540
clearTimeout(pingTimeout);
489541
});

packages/dev-middleware/src/types/EventReporter.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,17 +94,17 @@ export type ReportableEvent =
9494
...DebuggerSessionIDs,
9595
}
9696
| {
97-
type: 'debugger_heartbeat',
97+
type: 'debugger_heartbeat' | 'device_heartbeat',
9898
duration: number,
9999
...DebuggerSessionIDs,
100100
}
101101
| {
102-
type: 'debugger_timeout',
102+
type: 'debugger_timeout' | 'device_timeout',
103103
duration: number,
104104
...DebuggerSessionIDs,
105105
}
106106
| {
107-
type: 'debugger_connection_closed',
107+
type: 'debugger_connection_closed' | 'device_connection_closed',
108108
code: number,
109109
reason: string,
110110
...DebuggerSessionIDs,

0 commit comments

Comments
 (0)