diff --git a/packages/tool-server/src/tools/profiler/react/react-profiler-fiber-tree.ts b/packages/tool-server/src/tools/profiler/react/react-profiler-fiber-tree.ts index af520d74..340cf8c8 100644 --- a/packages/tool-server/src/tools/profiler/react/react-profiler-fiber-tree.ts +++ b/packages/tool-server/src/tools/profiler/react/react-profiler-fiber-tree.ts @@ -5,15 +5,21 @@ import { type ReactProfilerSessionApi, } from "../../../blueprints/react-profiler-session"; import { HEARTBEAT_SCRIPT, FIBER_ROOT_TRACKER_SCRIPT } from "../../../utils/react-profiler/scripts"; - -const HOOK_NOT_PRESENT_ERRORS = new Set([ - "no __REACT_DEVTOOLS_GLOBAL_HOOK__", - "no renderers attached to hook", -]); - -const HOOK_MISSING_MESSAGE = - "React DevTools hook not present. Ensure the app is in development mode. " + - "Try calling react-profiler-start first to re-inject the hook."; +import { NO_DEVTOOLS_HOOK_ERROR, NO_RENDERERS_ATTACHED_ERROR } from "./react-profiler-start"; + +const HOOK_MISSING_ERROR = "no __REACT_DEVTOOLS_GLOBAL_HOOK__"; +const NO_RENDERERS_ERROR = "no renderers attached to hook"; +const HOOK_NOT_PRESENT_ERRORS = new Set([HOOK_MISSING_ERROR, NO_RENDERERS_ERROR]); + +// See `react-profiler-renders.ts` for the rationale — branch on the actual +// error code so "hook missing" (rebuild in dev mode) and "renderers not +// attached" (wait for first render / let start bootstrap) get accurate +// remediation instead of being collapsed into one misleading message. +function messageForHookError(code: string): string { + if (code === HOOK_MISSING_ERROR) return NO_DEVTOOLS_HOOK_ERROR; + if (code === NO_RENDERERS_ERROR) return NO_RENDERERS_ATTACHED_ERROR; + return `Fiber tree error: ${code}`; +} function buildFiberTreeScript(maxDepth: number, filter: string): string { return ` @@ -166,11 +172,7 @@ Fails if the React DevTools hook is not present or no fiber roots have been comm if (typeof parsed === "object" && parsed !== null && "error" in parsed) { const errorMsg = (parsed as { error: string }).error; - throw new Error( - HOOK_NOT_PRESENT_ERRORS.has(errorMsg) - ? HOOK_MISSING_MESSAGE - : `Fiber tree error: ${errorMsg}` - ); + throw new Error(messageForHookError(errorMsg)); } if (Array.isArray(parsed) && parsed.length === 0) { diff --git a/packages/tool-server/src/tools/profiler/react/react-profiler-renders.ts b/packages/tool-server/src/tools/profiler/react/react-profiler-renders.ts index 49d99b04..89f53495 100644 --- a/packages/tool-server/src/tools/profiler/react/react-profiler-renders.ts +++ b/packages/tool-server/src/tools/profiler/react/react-profiler-renders.ts @@ -5,6 +5,7 @@ import { type ReactProfilerSessionApi, } from "../../../blueprints/react-profiler-session"; import { HEARTBEAT_SCRIPT, FIBER_ROOT_TRACKER_SCRIPT } from "../../../utils/react-profiler/scripts"; +import { NO_DEVTOOLS_HOOK_ERROR, NO_RENDERERS_ATTACHED_ERROR } from "./react-profiler-start"; const COLLECT_RENDERS_SCRIPT = ` (function() { @@ -50,14 +51,19 @@ const COLLECT_RENDERS_SCRIPT = ` })() `; -const HOOK_NOT_PRESENT_ERRORS = new Set([ - "no __REACT_DEVTOOLS_GLOBAL_HOOK__", - "no renderers attached to hook", -]); - -const HOOK_MISSING_MESSAGE = - "React DevTools hook not present. Ensure the app is in development mode. " + - "Try calling react-profiler-start first to re-inject the hook."; +const HOOK_MISSING_ERROR = "no __REACT_DEVTOOLS_GLOBAL_HOOK__"; +const NO_RENDERERS_ERROR = "no renderers attached to hook"; +const HOOK_NOT_PRESENT_ERRORS = new Set([HOOK_MISSING_ERROR, NO_RENDERERS_ERROR]); + +// "Hook missing" and "renderers not attached" point at different runtime +// states and have different remediations. The two codes funnel into +// FIBER_ROOT_TRACKER_SCRIPT for the retry path, but the verbose throw +// branches on the actual code so the operator gets accurate guidance. +function messageForHookError(code: string): string { + if (code === HOOK_MISSING_ERROR) return NO_DEVTOOLS_HOOK_ERROR; + if (code === NO_RENDERERS_ERROR) return NO_RENDERERS_ATTACHED_ERROR; + return `React hook error: ${code}`; +} type ParsedRenders = | Record< @@ -166,11 +172,7 @@ Fails if the React DevTools hook is not present in the runtime or the app is not const errorStr = getErrorString(parsed); if (errorStr !== null) { - throw new Error( - HOOK_NOT_PRESENT_ERRORS.has(errorStr) - ? HOOK_MISSING_MESSAGE - : `React hook error: ${errorStr}` - ); + throw new Error(messageForHookError(errorStr)); } const entries: RenderEntry[] = Object.entries(parsed) diff --git a/packages/tool-server/src/tools/profiler/react/react-profiler-start.ts b/packages/tool-server/src/tools/profiler/react/react-profiler-start.ts index 1fca54a6..107462a3 100644 --- a/packages/tool-server/src/tools/profiler/react/react-profiler-start.ts +++ b/packages/tool-server/src/tools/profiler/react/react-profiler-start.ts @@ -24,6 +24,32 @@ import { type BootstrapResult, } from "../../../utils/react-profiler/devtools-bootstrap"; +/** + * Verbose explanations the operator sees when the runtime is not profileable. + * Centralised so every tool that detects "this app cannot be profiled" emits + * the same diagnosis instead of bespoke one-liners. + */ +export const NO_DEVTOOLS_HOOK_ERROR = + "React DevTools hook (__REACT_DEVTOOLS_GLOBAL_HOOK__) is not present in this app's JavaScript runtime. " + + "React profiling requires a development build with React DevTools enabled. " + + "Likely causes: (1) the app is a release/production build — DevTools is stripped to reduce bundle size; " + + "(2) you connected to the wrong JS runtime; (3) this isn't a React (Native) app. " + + "Fix: rebuild in debug/dev mode (e.g. `npx react-native run-ios` without --configuration Release; for Expo, run a dev client). " + + "Once the app is running with DevTools attached, call react-profiler-start again."; + +/** + * Returned when the DevTools hook IS present but no React renderer has + * registered against it. Distinct from NO_DEVTOOLS_HOOK_ERROR because the + * remediation differs: rebuilding in dev mode does nothing here — the user + * needs the renderer to attach (wait for first commit, or let + * react-profiler-start bootstrap the DevTools backend on bridgeless RN + * dev builds that lack an external DevTools client). + */ +export const NO_RENDERERS_ATTACHED_ERROR = + "React DevTools hook is present but no React renderer has registered yet. " + + "The hook is loaded but no fiber renderer has attached — typically because the app has not committed its first render, or the DevTools backend has not been bootstrapped on a bridgeless React Native dev build. " + + "Fix: ensure the app has rendered (interact with it once, then retry); if it stays empty, call react-profiler-start first — it will attempt to attach the DevTools backend automatically."; + const zodSchema = z.object({ port: z.coerce.number().default(8081).describe("Metro server port"), device_id: z @@ -141,9 +167,7 @@ Fails if the Hermes runtime is not reachable or the Metro CDP connection cannot let state = JSON.parse(stateJson) as ReadStateResult; if (!state.hookExists) { - throw new Error( - "React DevTools is not available in this app. This usually means the app is a production build. Ask the user to run a development build of the app, then retry." - ); + throw new Error(NO_DEVTOOLS_HOOK_ERROR); } // If the hook is present but no rendererInterface is registered, the diff --git a/packages/tool-server/src/tools/profiler/react/react-profiler-stop.ts b/packages/tool-server/src/tools/profiler/react/react-profiler-stop.ts index d41f3f7f..f4c827a6 100644 --- a/packages/tool-server/src/tools/profiler/react/react-profiler-stop.ts +++ b/packages/tool-server/src/tools/profiler/react/react-profiler-stop.ts @@ -186,15 +186,43 @@ Fails if no active profiling session exists or the CDP connection was lost durin ); } + // The CPU sampler is enabled by react-profiler-start only after the + // DevTools-hook checks pass. If `profilingActive` is false, start + // either threw before `Profiler.start` (e.g. release build with + // React DevTools stripped) or never ran. Calling `Profiler.stop` + // against an un-started Hermes sampler returns an empty profile + // that would later crash `profile.samples.length` with a generic + // TypeError — detect it up front and explain. + if (!api.profilingActive) { + throw new Error( + "No active profiling run to stop. The session exists but Hermes CPU sampling was never started — typically because react-profiler-start failed before reaching the sampler (often on release builds without React DevTools). " + + "Check the error react-profiler-start returned, address the underlying cause (rebuild in dev mode, reconnect the debugger, etc.), then call react-profiler-start again." + ); + } + api.profilingActive = false; // reset BEFORE the CDP call so state is clean even if it throws const cpuResult = (await cdp.send("Profiler.stop")) as { profile?: HermesCpuProfile; }; if (!cpuResult?.profile) { - throw new Error("Profiler returned no profile data."); + throw new Error( + "Hermes Profiler.stop returned no profile data. The CPU sampler may have been reset between start and stop (Metro reload, debugger disconnect). " + + "Call react-profiler-start to begin a fresh session." + ); } const profile = cpuResult.profile; + if ( + !Array.isArray(profile.samples) || + !Array.isArray(profile.nodes) || + !Array.isArray(profile.timeDeltas) + ) { + throw new Error( + "Hermes Profiler.stop returned a malformed profile (missing samples/nodes/timeDeltas). " + + "This usually means CPU sampling was never actually started for this session — most often a release build without React DevTools, or a Metro reload between start and stop. " + + "Call react-profiler-start on a dev build and retry." + ); + } // Single evaluate: stop the backend profiler, read the live buffer, and // resolve every referenced fiberID to a displayName in one round-trip. diff --git a/packages/tool-server/src/utils/debugger/scripts/inspect-at-point.ts b/packages/tool-server/src/utils/debugger/scripts/inspect-at-point.ts index b55ca158..7af77667 100644 --- a/packages/tool-server/src/utils/debugger/scripts/inspect-at-point.ts +++ b/packages/tool-server/src/utils/debugger/scripts/inspect-at-point.ts @@ -1,3 +1,26 @@ +/** + * Verbose error message returned when the React DevTools hook is missing. + * Exported so the tool layer can recognise the same diagnosis without string + * matching on bespoke phrasings. Mirrored by `react-profiler-start` so the + * operator sees one consistent explanation regardless of which entry point + * they hit first. + */ +export const INSPECT_NO_DEVTOOLS_HOOK_ERROR = + "React DevTools hook (__REACT_DEVTOOLS_GLOBAL_HOOK__) is not present in this app's JavaScript runtime. " + + "Component inspection requires a development build with React DevTools enabled. " + + "Likely causes: (1) the app is a release/production build — DevTools is stripped to reduce bundle size; " + + "(2) you connected to the wrong JS runtime; (3) this isn't a React (Native) app. " + + "Fix: rebuild in debug/dev mode (e.g. `npx react-native run-ios` without --configuration Release; for Expo, run a dev client)."; + +export const INSPECT_NO_RENDERER_ERROR = + "React DevTools hook is present but no renderer has registered yet. " + + "Component inspection requires the React renderer to be attached — wait for the app to render its first commit, then retry. " + + "If this persists, confirm the app is a React (Native) app running in development mode."; + +export const INSPECT_NO_FIBER_ROOT_ERROR = + "React DevTools is attached but no fiber root has mounted yet. " + + "Wait for the app to render its first frame and retry."; + /** * Generate a JS script that calls getInspectorDataForViewAtPoint at (x, y) * and pushes the result via __argent_callback binding with a requestId @@ -17,10 +40,28 @@ * then falls back to _debugSource ({ fileName, lineNumber, columnNumber } from * @babel/plugin-transform-react-jsx-source). Frames from _debugSource are flagged * with `original: true` since they already contain the real source path. + * + * Production-build guards: dereferencing __REACT_DEVTOOLS_GLOBAL_HOOK__ blindly + * would throw `Cannot read property 'renderers' of undefined` on release builds + * where DevTools is stripped. The script reports these conditions through the + * same __argent_callback error channel as `no host fiber`, so the tool surfaces + * a verbose diagnostic instead of a generic TypeError. */ export function makeInspectScript(x: number, y: number, requestId: string): string { + const noHookMsg = JSON.stringify(INSPECT_NO_DEVTOOLS_HOOK_ERROR); + const noRendererMsg = JSON.stringify(INSPECT_NO_RENDERER_ERROR); + const noRootMsg = JSON.stringify(INSPECT_NO_FIBER_ROOT_ERROR); return `(function() { - var hook = window.__REACT_DEVTOOLS_GLOBAL_HOOK__; + function __argent_fail(msg) { + __argent_callback(JSON.stringify({requestId:'${requestId}',type:'inspect_result',error:msg})); + } + try { + var hook = (typeof globalThis !== 'undefined' ? globalThis : window).__REACT_DEVTOOLS_GLOBAL_HOOK__; + if (!hook) { __argent_fail(${noHookMsg}); return; } + if (!hook.renderers || typeof hook.renderers.values !== 'function' || hook.renderers.size === 0) { + __argent_fail(${noRendererMsg}); return; + } + if (typeof hook.getFiberRoots !== 'function') { __argent_fail(${noRendererMsg}); return; } // Pick the renderer + root that hosts the real app UI. Secondary reconcilers // (react-native-skia, react-native-svg, ...) register their own renderer and @@ -51,7 +92,7 @@ export function makeInspectScript(x: number, y: number, requestId: string): stri var _legacy = hook.getFiberRoots(1); root = _legacy ? Array.from(_legacy)[0] : null; } - if (!renderer || !root) { __argent_callback(JSON.stringify({requestId:'${requestId}',type:'inspect_result',error:'no fiber roots'})); return; } + if (!renderer || !root || !root.current) { __argent_fail(${noRootMsg}); return; } var useFabric = typeof nativeFabricUIManager !== 'undefined'; @@ -129,5 +170,8 @@ export function makeInspectScript(x: number, y: number, requestId: string): stri } ); return 'ok'; + } catch (e) { + __argent_fail('Inspect script crashed: ' + (e && e.message ? e.message : String(e))); + } })()`; } diff --git a/packages/tool-server/src/utils/react-profiler/debug/dump.ts b/packages/tool-server/src/utils/react-profiler/debug/dump.ts index f5340e9a..dea49305 100644 --- a/packages/tool-server/src/utils/react-profiler/debug/dump.ts +++ b/packages/tool-server/src/utils/react-profiler/debug/dump.ts @@ -59,11 +59,37 @@ export async function writeDumpCompact( } /** - * Read a CPU profile from disk. + * Read a CPU profile from disk. Validates the shape inline so consumers + * (analyze, cpu-summary, cpu-query) get a verbose, actionable error instead + * of a generic `TypeError: Cannot read properties of undefined (reading + * 'length')` from `buildCpuSampleIndex` when an older or partial dump is + * loaded via `profiler-load`. */ export async function readCpuProfile(path: string): Promise { const json = await fs.readFile(path, "utf8"); - return JSON.parse(json) as HermesCpuProfile; + const parsed = JSON.parse(json) as Partial | null; + if (!parsed || typeof parsed !== "object") { + throw new Error( + `On-disk CPU profile at ${path} is missing or not an object. The session dump is corrupt or was written by an incompatible tool-server version; rerun react-profiler-start/stop to capture a fresh session.` + ); + } + if ( + !Array.isArray(parsed.samples) || + !Array.isArray(parsed.nodes) || + !Array.isArray(parsed.timeDeltas) + ) { + throw new Error( + `On-disk CPU profile at ${path} is malformed (missing samples/nodes/timeDeltas). ` + + `The session was likely recorded against a release build where Hermes CPU sampling never started, or the dump was truncated. ` + + `Rerun react-profiler-start on a dev build and retry.` + ); + } + if (typeof parsed.startTime !== "number" || typeof parsed.endTime !== "number") { + throw new Error( + `On-disk CPU profile at ${path} is missing startTime/endTime timestamps; the recording is incomplete and cannot be analysed.` + ); + } + return parsed as HermesCpuProfile; } export interface CommitTreeOnDisk { diff --git a/packages/tool-server/test/debugger/inspect-at-point-guards.test.ts b/packages/tool-server/test/debugger/inspect-at-point-guards.test.ts new file mode 100644 index 00000000..973d1d96 --- /dev/null +++ b/packages/tool-server/test/debugger/inspect-at-point-guards.test.ts @@ -0,0 +1,163 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { + INSPECT_NO_DEVTOOLS_HOOK_ERROR, + INSPECT_NO_FIBER_ROOT_ERROR, + INSPECT_NO_RENDERER_ERROR, + makeInspectScript, +} from "../../src/utils/debugger/scripts/inspect-at-point"; + +/** + * The script is generated as a JS source string injected into the Hermes + * runtime via CDP `Runtime.evaluate`. The guards we add here are the only + * thing standing between a release build (where __REACT_DEVTOOLS_GLOBAL_HOOK__ + * is stripped) and a generic `Cannot read property 'renderers' of undefined` + * TypeError surfacing to the operator. Eval the script against a controlled + * `globalThis` to verify each guard fires the verbose diagnostic. + */ + +interface InspectErrorPayload { + requestId: string; + type: "inspect_result"; + error: string; +} + +function evalIIFE(script: string): unknown { + return (0, eval)(script); +} + +function withGlobals(setup: () => void, body: () => void) { + const g = globalThis as Record; + const saved = { + hook: g.__REACT_DEVTOOLS_GLOBAL_HOOK__, + callback: g.__argent_callback, + nativeFabric: g.nativeFabricUIManager, + window: g.window, + }; + try { + setup(); + body(); + } finally { + g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = saved.hook; + g.__argent_callback = saved.callback; + g.nativeFabricUIManager = saved.nativeFabric; + g.window = saved.window; + } +} + +function makeCallbackSink(): { + spy: ReturnType; + lastPayload: () => InspectErrorPayload | null; +} { + const spy = vi.fn(); + return { + spy, + lastPayload: () => { + const lastCall = spy.mock.calls.at(-1); + if (!lastCall) return null; + return JSON.parse(lastCall[0] as string) as InspectErrorPayload; + }, + }; +} + +const REQ = "req-test-123"; + +afterEach(() => { + const g = globalThis as Record; + delete g.__REACT_DEVTOOLS_GLOBAL_HOOK__; + delete g.__argent_callback; +}); + +describe("makeInspectScript — production-build guards", () => { + it("emits verbose hook-missing error when __REACT_DEVTOOLS_GLOBAL_HOOK__ is undefined", () => { + const sink = makeCallbackSink(); + withGlobals( + () => { + const g = globalThis as Record; + g.__argent_callback = sink.spy; + delete g.__REACT_DEVTOOLS_GLOBAL_HOOK__; + }, + () => { + evalIIFE(makeInspectScript(100, 200, REQ)); + const payload = sink.lastPayload(); + expect(payload).not.toBeNull(); + expect(payload?.requestId).toBe(REQ); + expect(payload?.type).toBe("inspect_result"); + expect(payload?.error).toBe(INSPECT_NO_DEVTOOLS_HOOK_ERROR); + } + ); + }); + + it("emits verbose renderer-missing error when hook has no renderers map", () => { + const sink = makeCallbackSink(); + withGlobals( + () => { + const g = globalThis as Record; + g.__argent_callback = sink.spy; + g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = {}; + }, + () => { + evalIIFE(makeInspectScript(100, 200, REQ)); + const payload = sink.lastPayload(); + expect(payload?.error).toBe(INSPECT_NO_RENDERER_ERROR); + } + ); + }); + + it("emits verbose renderer-missing error when renderers map is empty", () => { + const sink = makeCallbackSink(); + withGlobals( + () => { + const g = globalThis as Record; + g.__argent_callback = sink.spy; + g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = { renderers: new Map() }; + }, + () => { + evalIIFE(makeInspectScript(100, 200, REQ)); + const payload = sink.lastPayload(); + expect(payload?.error).toBe(INSPECT_NO_RENDERER_ERROR); + } + ); + }); + + it("emits verbose fiber-root error when getFiberRoots returns empty", () => { + const sink = makeCallbackSink(); + withGlobals( + () => { + const g = globalThis as Record; + g.__argent_callback = sink.spy; + g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = { + renderers: new Map([[1, { rendererConfig: {} }]]), + getFiberRoots: () => new Set(), + }; + }, + () => { + evalIIFE(makeInspectScript(100, 200, REQ)); + const payload = sink.lastPayload(); + expect(payload?.error).toBe(INSPECT_NO_FIBER_ROOT_ERROR); + } + ); + }); + + it("emits verbose renderer-missing error when getFiberRoots is not a function", () => { + const sink = makeCallbackSink(); + withGlobals( + () => { + const g = globalThis as Record; + g.__argent_callback = sink.spy; + g.__REACT_DEVTOOLS_GLOBAL_HOOK__ = { + renderers: new Map([[1, { rendererConfig: {} }]]), + }; + }, + () => { + evalIIFE(makeInspectScript(100, 200, REQ)); + const payload = sink.lastPayload(); + expect(payload?.error).toBe(INSPECT_NO_RENDERER_ERROR); + } + ); + }); + + it("includes a production-build hint so operators know to rebuild", () => { + expect(INSPECT_NO_DEVTOOLS_HOOK_ERROR).toMatch(/release/i); + expect(INSPECT_NO_DEVTOOLS_HOOK_ERROR).toMatch(/development|dev/i); + }); +});