From a91cafd8cd2a842191bc1a073b97583453f5ca26 Mon Sep 17 00:00:00 2001 From: Vitali Zaidman Date: Thu, 20 Feb 2025 10:07:19 -0800 Subject: [PATCH] add debuggerSessionIDs fields to debugger events (#49552) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/49552 Changelog: [General][Internal] - expand debugger events to have DebuggerSessionIDs Also moved the handling of these to a shared function Reviewed By: huntie Differential Revision: D69917817 fbshipit-source-id: 2374ac5b5dc0040b0e15028ab89fbe78026bc296 --- .../dev-middleware/src/createDevMiddleware.js | 3 +- .../src/inspector-proxy/InspectorProxy.js | 49 +++++++++++++------ .../src/middleware/openDebuggerMiddleware.js | 48 +++++++++--------- .../dev-middleware/src/types/EventReporter.js | 23 +++++---- 4 files changed, 74 insertions(+), 49 deletions(-) diff --git a/packages/dev-middleware/src/createDevMiddleware.js b/packages/dev-middleware/src/createDevMiddleware.js index e51df70ff1e..b2c21c2d8c7 100644 --- a/packages/dev-middleware/src/createDevMiddleware.js +++ b/packages/dev-middleware/src/createDevMiddleware.js @@ -148,7 +148,8 @@ function createWrappedEventReporter( switch (event.type) { case 'profiling_target_registered': logger?.info( - `Profiling build target "${event.appId}" registered for debugging`, + "Profiling build target '%s' registered for debugging", + event.appId ?? 'unknown', ); break; case 'fusebox_console_notice': diff --git a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js index 3ff3687647a..b52589e58f8 100644 --- a/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js +++ b/packages/dev-middleware/src/inspector-proxy/InspectorProxy.js @@ -9,7 +9,7 @@ * @oncall react_native */ -import type {EventReporter} from '../types/EventReporter'; +import type {DebuggerSessionIDs, EventReporter} from '../types/EventReporter'; import type {Experiments} from '../types/Experiments'; import type {Logger} from '../types/Logger'; import type {CreateCustomMessageHandlerFn} from './CustomMessageHandler'; @@ -342,26 +342,38 @@ export default class InspectorProxy implements InspectorProxyQueries { }); // $FlowFixMe[value-as-type] wss.on('connection', async (socket: WS, req) => { - try { - const query = url.parse(req.url || '', true).query || {}; - const deviceId = query.device; - const pageId = query.page; - const debuggerRelativeBaseUrl = - getBaseUrlFromRequest(req) ?? this.#serverBaseUrl; - const appId = this.#devices.get(deviceId)?.getApp() || 'unknown'; + const query = url.parse(req.url || '', true).query || {}; + const deviceId = query.device; + const pageId = query.page; + const debuggerRelativeBaseUrl = + getBaseUrlFromRequest(req) ?? this.#serverBaseUrl; + const device: Device | void = deviceId + ? this.#devices.get(deviceId) + : undefined; + const debuggerSessionIDs: DebuggerSessionIDs = { + appId: device?.getApp() || null, + deviceId, + deviceName: device?.getName() || null, + pageId, + }; + + try { if (deviceId == null || pageId == null) { throw new Error('Incorrect URL - must provide device and page IDs'); } - const device = this.#devices.get(deviceId); if (device == null) { throw new Error('Unknown device with ID ' + deviceId); } this.#logger?.info('Connection established to DevTools.'); - this.#startHeartbeat(socket, DEBUGGER_HEARTBEAT_INTERVAL_MS, appId); + this.#startHeartbeat( + socket, + DEBUGGER_HEARTBEAT_INTERVAL_MS, + debuggerSessionIDs, + ); device.handleDebuggerConnection(socket, pageId, { debuggerRelativeBaseUrl, @@ -377,6 +389,7 @@ export default class InspectorProxy implements InspectorProxyQueries { type: 'connect_debugger_frontend', status: 'error', error, + ...debuggerSessionIDs, }); } }); @@ -389,7 +402,11 @@ export default class InspectorProxy implements InspectorProxyQueries { // where proxies may drop idle connections (e.g., VS Code tunnels). // // https://datatracker.ietf.org/doc/html/rfc6455#section-5.5.2 - #startHeartbeat(socket: WS, intervalMs: number, appId: string) { + #startHeartbeat( + socket: WS, + intervalMs: number, + debuggerSessionIDs: DebuggerSessionIDs, + ) { let latestPingMs = Date.now(); let terminateTimeout: ?Timeout; @@ -423,7 +440,7 @@ export default class InspectorProxy implements InspectorProxyQueries { this.#eventReporter?.logEvent({ type: 'debugger_timeout', duration: DEBUGGER_TIMEOUT_MS, - appId, + ...debuggerSessionIDs, }); }, DEBUGGER_TIMEOUT_MS).unref(); } @@ -435,12 +452,16 @@ export default class InspectorProxy implements InspectorProxyQueries { socket.on('pong', () => { const roundtripDuration = Date.now() - latestPingMs; - debug('debugger ping-pong for %s took %dms.', appId, roundtripDuration); + debug( + 'debugger ping-pong for %s took %dms.', + debuggerSessionIDs.appId, + roundtripDuration, + ); this.#eventReporter?.logEvent({ type: 'debugger_heartbeat', duration: roundtripDuration, - appId, + ...debuggerSessionIDs, }); terminateTimeout?.refresh(); diff --git a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js index 7061abbbcbc..f67c5c553db 100644 --- a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js +++ b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js @@ -10,6 +10,7 @@ */ import type {InspectorProxyQueries} from '../inspector-proxy/InspectorProxy'; +import type {PageDescription} from '../inspector-proxy/types'; import type {BrowserLauncher} from '../types/BrowserLauncher'; import type {EventReporter} from '../types/EventReporter'; import type {Experiments} from '../types/Experiments'; @@ -57,20 +58,16 @@ export default function openDebuggerMiddleware({ req.method === 'POST' || (experiments.enableOpenDebuggerRedirect && req.method === 'GET') ) { - const {query} = url.parse(req.url, true); - const { - appId, - device, - launchId, - target: targetId, - }: { + const paresedUrl = url.parse(req.url, true); + const query: { /** @deprecated Will only match legacy Hermes targets */ appId?: string, + /** @deprecated Will only match legacy Hermes targets */ device?: string, launchId?: string, target?: string, ... - } = query; + } = paresedUrl.query; const targets = inspectorProxy .getPageDescriptions({requestorRelativeBaseUrl: new URL(serverBaseUrl)}) @@ -91,27 +88,29 @@ export default function openDebuggerMiddleware({ return betterReloadingSupport; }); - let target; + let target: PageDescription | void; const launchType: 'launch' | 'redirect' = req.method === 'POST' ? 'launch' : 'redirect'; if ( - typeof targetId === 'string' || - typeof appId === 'string' || - typeof device === 'string' + typeof query.target === 'string' || + typeof query.appId === 'string' || + typeof query.device === 'string' ) { logger?.info( (launchType === 'launch' ? 'Launching' : 'Redirecting to') + ' DevTools...', ); + target = targets.find( _target => - (targetId == null || _target.id === targetId) && - (appId == null || - (_target.appId === appId && + (query.target == null || _target.id === query.target) && + (query.appId == null || + (_target.appId === query.appId && _target.title === LEGACY_SYNTHETIC_PAGE_TITLE)) && - (device == null || _target.reactNative.logicalDeviceId === device), + (query.device == null || + _target.reactNative.logicalDeviceId === query.device), ); } else if (targets.length > 0) { logger?.info( @@ -147,7 +146,7 @@ export default function openDebuggerMiddleware({ experiments, target.webSocketDebuggerUrl, serverBaseUrl, - {launchId, useFuseboxEntryPoint}, + {launchId: query.launchId, useFuseboxEntryPoint}, ), ); res.writeHead(200); @@ -159,7 +158,11 @@ export default function openDebuggerMiddleware({ experiments, target.webSocketDebuggerUrl, serverBaseUrl, - {relative: true, launchId, useFuseboxEntryPoint}, + { + relative: true, + launchId: query.launchId, + useFuseboxEntryPoint, + }, ), }); res.end(); @@ -171,10 +174,11 @@ export default function openDebuggerMiddleware({ type: 'launch_debugger_frontend', launchType, status: 'success', - appId: appId ?? null, - deviceId: device ?? null, - resolvedTargetDescription: target.description, - resolvedTargetAppId: target.appId, + appId: target.appId, + deviceId: target.reactNative.logicalDeviceId, + pageId: target.id, + deviceName: target.deviceName, + targetDescription: target.description, prefersFuseboxFrontend: useFuseboxEntryPoint ?? false, }); return; diff --git a/packages/dev-middleware/src/types/EventReporter.js b/packages/dev-middleware/src/types/EventReporter.js index 1c93da7b868..006ce5d389c 100644 --- a/packages/dev-middleware/src/types/EventReporter.js +++ b/packages/dev-middleware/src/types/EventReporter.js @@ -13,10 +13,11 @@ type SuccessResult = { ...Props, }; -type ErrorResult = { +type ErrorResult = { status: 'error', error: ErrorT, prefersFuseboxFrontend?: ?boolean, + ...Props, }; type CodedErrorResult = { @@ -25,10 +26,10 @@ type CodedErrorResult = { errorDetails?: string, }; -type DebuggerSessionIDs = { - appId: string, - deviceName: string, - deviceId: string, +export type DebuggerSessionIDs = { + appId: string | null, + deviceName: string | null, + deviceId: string | null, pageId: string | null, }; @@ -38,11 +39,9 @@ export type ReportableEvent = launchType: 'launch' | 'redirect', ... | SuccessResult<{ - appId: string | null, - deviceId: string | null, - resolvedTargetDescription: string, - resolvedTargetAppId: string, + targetDescription: string, prefersFuseboxFrontend: boolean, + ...DebuggerSessionIDs, }> | ErrorResult | CodedErrorResult<'NO_APPS_FOUND'>, @@ -54,7 +53,7 @@ export type ReportableEvent = ...DebuggerSessionIDs, frontendUserAgent: string | null, }> - | ErrorResult, + | ErrorResult, } | { type: 'debugger_command', @@ -97,12 +96,12 @@ export type ReportableEvent = | { type: 'debugger_heartbeat', duration: number, - appId: string, + ...DebuggerSessionIDs, } | { type: 'debugger_timeout', duration: number, - appId: string, + ...DebuggerSessionIDs, }; /**