From e5e2fb8b2dbe1cae85aa5ad6ce45aef376016e43 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 10 Aug 2026 11:07:50 -0700 Subject: [PATCH] fix(desktop): persist renderer crashes to desktop.log + finish the render() isolation class MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three-part class closure for the React #310 / lost-renderer-crash family (#79428, follow-up to #80560 / #82763): 1. Diagnosability (#79428 defect B): error-boundary catches now persist to desktop.log with their component stack via a new fire-and-forget hermes:logs:renderer-error IPC (synchronous flush — the window may be dying). Every renderer-content window (main, secondary session, instance, HUD, quick entry, pet overlay) gets the error-level console capture that previously only the main window had, labeled per window. 'Open logs' on the crash dialog now reveals a file that actually contains the crash. 2. Recurrence guard: eslint no-restricted-syntax rule banning inline render() calls in JSX — the mechanism behind #80560. The rule immediately caught two live sites #82763's audit missed (floating panes, narrow-overlay reveal), both hosting plugin panes. 3. Fix those two missed sites with the same ContribRender mount. extracted console-capture/report formatting to electron/renderer-log.ts with unit tests; renderer console lines now carry the window label. --- apps/desktop/electron/main.ts | 36 ++++---- apps/desktop/electron/preload.ts | 3 + apps/desktop/electron/renderer-log.test.ts | 77 ++++++++++++++++ apps/desktop/electron/renderer-log.ts | 90 +++++++++++++++++++ apps/desktop/eslint.config.mjs | 10 +++ .../desktop/src/components/error-boundary.tsx | 16 +++- .../tree/renderer/floating-panes.tsx | 4 +- .../tree/renderer/narrow-overlays.tsx | 6 +- apps/desktop/src/global.d.ts | 7 ++ 9 files changed, 229 insertions(+), 20 deletions(-) create mode 100644 apps/desktop/electron/renderer-log.test.ts create mode 100644 apps/desktop/electron/renderer-log.ts diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index d1f3dd1b49b2f..daa3bf8fd0c04 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -173,6 +173,7 @@ import { revalidatePooledRemoteBackends, revalidateRemoteConnection } from './remote-liveness' +import { attachRendererConsoleCapture, formatRendererBoundaryReport } from './renderer-log' import { buildSessionWindowUrl, chatWindowWebPreferences, @@ -8916,6 +8917,7 @@ function spawnSecondaryWindow({ sessionId, watch }: { sessionId?: string; watch? streamThrottle.register(win) wireCommonWindowHandlers(win, zoomWiringForWindowKind('chat')) + attachRendererConsoleCapture(win, 'session-window', rememberLog) loadWindowUrl( win, @@ -9001,6 +9003,7 @@ function createInstanceWindow() { instanceWindows.delete(win) }) + attachRendererConsoleCapture(win, 'instance', rememberLog) loadWindowUrl(win, DEV_SERVER || pathToFileURL(resolveRendererIndex()).toString(), 'Instance window') return win @@ -9120,6 +9123,7 @@ function spawnPetOverlayWindow(bounds) { } }) + attachRendererConsoleCapture(win, 'pet-overlay', rememberLog) loadWindowUrl(win, petOverlayUrl(), 'Pet overlay') return win @@ -9483,6 +9487,7 @@ function spawnHudWindow(sessionId, profile) { broadcastHudState(false) }) + attachRendererConsoleCapture(win, 'hud', rememberLog) loadWindowUrl(win, hudUrl(sessionId, profile), 'HUD') return win @@ -9688,6 +9693,7 @@ function spawnQuickEntryWindow() { } }) + attachRendererConsoleCapture(win, 'quick-entry', rememberLog) loadWindowUrl(win, quickEntryUrl(), 'Quick entry') return win @@ -9970,21 +9976,10 @@ function createWindow() { // Electron always passes the event first. The canonical (Electron 36+) shape // is (event, messageDetails); the deprecated positional shape is - // (event, level, message, line, sourceId). Handle both. `level` is numeric - // (0..3), where 3 === error. - mainWindow.webContents.on('console-message', (_event, detailsOrLevel, message, line, sourceId) => { - const details = detailsOrLevel && typeof detailsOrLevel === 'object' ? detailsOrLevel : null - const level = details ? details.level : detailsOrLevel - - if (level !== 3) { - return - } - - const text = details ? details.message : message - const src = details ? details.sourceUrl : sourceId - const lineNo = details ? details.lineNumber : line - rememberLog(`[renderer console] ${text} (${src}:${lineNo})`) - }) + // (event, level, message, line, sourceId). Handled in renderer-log.ts, which + // every renderer-content window shares (#79428: crashes in secondary/HUD/ + // quick-entry windows used to vanish without a trace). + attachRendererConsoleCapture(mainWindow, 'main', rememberLog) loadWindowUrl(mainWindow, DEV_SERVER || pathToFileURL(resolveRendererIndex()).toString(), 'Renderer') @@ -11549,6 +11544,17 @@ ipcMain.handle('hermes:logs:reveal', async () => { ipcMain.handle('hermes:logs:recent', async () => ({ path: DESKTOP_LOG_PATH, lines: hermesLog.slice(-200) })) +// Renderer error-boundary catches (#79428 defect B): the component stack only +// exists in renderer memory, so the boundary posts it here and we persist it +// via the desktop.log pipeline. `on`, not `handle` — the sender may be mid- +// crash and must not await. Flush immediately: a crashing window can be gone +// before the debounced flush timer fires. +ipcMain.on('hermes:logs:renderer-error', (_event, report) => { + const { label, boundary, message, componentStack } = report && typeof report === 'object' ? report : {} + rememberLog(formatRendererBoundaryReport(label, boundary, message, componentStack)) + flushDesktopLogBufferSync() +}) + function isExecutableFile(filePath) { if (!filePath || !path.isAbsolute(filePath)) { return false diff --git a/apps/desktop/electron/preload.ts b/apps/desktop/electron/preload.ts index e1ee4c0061278..f2b16d1760576 100644 --- a/apps/desktop/electron/preload.ts +++ b/apps/desktop/electron/preload.ts @@ -199,6 +199,9 @@ contextBridge.exposeInMainWorld('hermesDesktop', { }, revealLogs: () => ipcRenderer.invoke('hermes:logs:reveal'), getRecentLogs: () => ipcRenderer.invoke('hermes:logs:recent'), + // Fire-and-forget: persists a renderer error-boundary catch (with component + // stack) to desktop.log so crashes survive the window (#79428). + reportRendererError: report => ipcRenderer.send('hermes:logs:renderer-error', report), readDir: dirPath => ipcRenderer.invoke('hermes:fs:readDir', dirPath), gitRoot: startPath => ipcRenderer.invoke('hermes:fs:gitRoot', startPath), revealPath: targetPath => ipcRenderer.invoke('hermes:fs:reveal', targetPath), diff --git a/apps/desktop/electron/renderer-log.test.ts b/apps/desktop/electron/renderer-log.test.ts new file mode 100644 index 0000000000000..cb45b1edbb0db --- /dev/null +++ b/apps/desktop/electron/renderer-log.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it, vi } from 'vitest' + +import { attachRendererConsoleCapture, formatRendererBoundaryReport, formatRendererConsoleLine } from './renderer-log' + +describe('formatRendererConsoleLine', () => { + it('formats the canonical Electron 36+ details shape at error level', () => { + const line = formatRendererConsoleLine('hud', { + level: 3, + message: 'Minified React error #310', + sourceUrl: 'file:///app/index.js', + lineNumber: 13 + }) + + expect(line).toBe('[renderer console:hud] Minified React error #310 (file:///app/index.js:13)') + }) + + it('formats the deprecated positional shape at error level', () => { + const line = formatRendererConsoleLine('main', 3, 'boom', 7, 'file:///app/vendor.js') + + expect(line).toBe('[renderer console:main] boom (file:///app/vendor.js:7)') + }) + + it('drops non-error levels in both shapes', () => { + expect(formatRendererConsoleLine('main', { level: 1, message: 'x', sourceUrl: 's', lineNumber: 1 })).toBeNull() + expect(formatRendererConsoleLine('main', 2, 'warn', 1, 's')).toBeNull() + }) +}) + +describe('attachRendererConsoleCapture', () => { + it('logs error-level messages and skips the rest', () => { + const log = vi.fn() + let handler: ((...args: unknown[]) => void) | undefined + const win = { + webContents: { + on: (_event: string, listener: (...args: unknown[]) => void) => { + handler = listener + } + } + } + + attachRendererConsoleCapture(win, 'quick-entry', log) + + handler?.({}, { level: 3, message: 'crash', sourceUrl: 'src', lineNumber: 2 }) + handler?.({}, { level: 0, message: 'debug', sourceUrl: 'src', lineNumber: 3 }) + + expect(log).toHaveBeenCalledTimes(1) + expect(log).toHaveBeenCalledWith('[renderer console:quick-entry] crash (src:2)') + }) +}) + +describe('formatRendererBoundaryReport', () => { + it('carries window label, boundary label, message, and component stack', () => { + const report = formatRendererBoundaryReport( + 'main', + 'root', + 'Minified React error #310', + '\n at Gde (index.js:13)\n at C_ (index.js:13)' + ) + + expect(report).toContain('[renderer crash:main] [error-boundary:root] Minified React error #310') + expect(report).toContain('at Gde (index.js:13)') + }) + + it('survives a malformed payload and clamps oversized fields', () => { + const report = formatRendererBoundaryReport(undefined, null, 'x'.repeat(10_000), 'y'.repeat(10_000)) + + expect(report).toContain('[renderer crash:unknown] [error-boundary:unknown]') + expect(report.length).toBeLessThan(7_000) + }) + + it('omits the stack block when there is no component stack', () => { + const report = formatRendererBoundaryReport('main', 'root', 'boom', '') + + expect(report).toBe('[renderer crash:main] [error-boundary:root] boom') + expect(report).not.toContain('\n') + }) +}) diff --git a/apps/desktop/electron/renderer-log.ts b/apps/desktop/electron/renderer-log.ts new file mode 100644 index 0000000000000..095d5a00b6369 --- /dev/null +++ b/apps/desktop/electron/renderer-log.ts @@ -0,0 +1,90 @@ +/** + * Renderer console/error capture shared by every renderer-content window. + * + * Historically only the primary window (`createWindow()`) attached a + * `console-message` hook, so a renderer crash in ANY other window — secondary + * session windows, instance windows, the HUD, quick entry, the pet overlay — + * evaporated with the window: nothing in desktop.log, nothing to attach to a + * bug report (#79428 defect B). The React error boundary logs crashes via + * `console.error`, so windows without the hook also lost every boundary catch. + * + * `attachRendererConsoleCapture` is the one owner of that hook. Every window + * that loads our renderer must go through it; the label says which window the + * line came from so multi-window reports are diagnosable. Windows that load + * EXTERNAL content (OAuth portals) must NOT attach — third-party pages can log + * tokens or PII we never want on disk. + */ + +interface ConsoleMessageDetails { + level: number + message: string + sourceUrl: string + lineNumber: number +} + +interface WebContentsLike { + on(event: 'console-message', listener: (...args: unknown[]) => void): unknown +} + +interface WindowLike { + webContents: WebContentsLike +} + +/** Normalize Electron's two `console-message` signatures into one line, or + * null for non-error levels. Canonical (Electron 36+): `(event, details)`; + * deprecated positional: `(event, level, message, line, sourceId)`. + * `level` is numeric 0..3, where 3 === error. */ +export function formatRendererConsoleLine( + label: string, + detailsOrLevel: unknown, + message?: unknown, + line?: unknown, + sourceId?: unknown +): string | null { + const details = + detailsOrLevel && typeof detailsOrLevel === 'object' ? (detailsOrLevel as ConsoleMessageDetails) : null + const level = details ? details.level : detailsOrLevel + + if (level !== 3) { + return null + } + + const text = details ? details.message : message + const src = details ? details.sourceUrl : sourceId + const lineNo = details ? details.lineNumber : line + + return `[renderer console:${label}] ${String(text)} (${String(src)}:${String(lineNo)})` +} + +/** Attach the error-level console hook to a renderer window. `log` is the + * desktop.log sink (rememberLog in main.ts). */ +export function attachRendererConsoleCapture(win: WindowLike, label: string, log: (line: string) => void): void { + win.webContents.on('console-message', (_event, detailsOrLevel, message, line, sourceId) => { + const formatted = formatRendererConsoleLine(label, detailsOrLevel, message, line, sourceId) + + if (formatted !== null) { + log(formatted) + } + }) +} + +/** Format a renderer error-boundary report (hermes:logs:renderer-error IPC) + * for desktop.log. Boundary catches carry the component stack — the one piece + * of context a minified console line loses — so persist it alongside. + * Inputs are renderer-supplied: clamp so a hostile/buggy payload cannot bloat + * the log. */ +export function formatRendererBoundaryReport( + label: unknown, + boundary: unknown, + message: unknown, + componentStack: unknown +): string { + const clamp = (value: unknown, max: number): string => String(value ?? '').slice(0, max) + + const head = `[renderer crash:${clamp(label, 64) || 'unknown'}] [error-boundary:${clamp(boundary, 64) || 'unknown'}] ${ + clamp(message, 2000) || '(no message)' + }` + const stack = clamp(componentStack, 4000).trim() + + return stack ? `${head}\n${stack}` : head +} diff --git a/apps/desktop/eslint.config.mjs b/apps/desktop/eslint.config.mjs index 7e02d1d793676..5d86f89ad25a0 100644 --- a/apps/desktop/eslint.config.mjs +++ b/apps/desktop/eslint.config.mjs @@ -74,6 +74,16 @@ export default [ 'CallExpression[callee.name="useEffect"] > ArrowFunctionExpression[body.type="BlockStatement"]:has(CallExpression[callee.name="setMutableRef"])', message: 'Do not mirror reactive values into refs via useEffect (setMutableRef included). Read $atom.get() directly in callbacks instead — refs synced from atoms lag one render and cause stale-read bugs.' + }, + { + // {contribution.render()} anywhere inside JSX — calling a render + // callback inline makes its hooks belong to the HOST component, so + // loading/replacing a plugin changes the host's hook count → React + // #310 (#80560, crashed every Windows user with a desktop plugin). + // Mount it as a child instead: . + selector: 'JSXExpressionContainer CallExpression[callee.property.name="render"]', + message: + 'Do not call render() callbacks inline in JSX — the callback\u2019s hooks become the host\u2019s and plugin load/replace changes the host hook count (React #310). Mount it as a component: from @/contrib/react/boundary.' } ] } diff --git a/apps/desktop/src/components/error-boundary.tsx b/apps/desktop/src/components/error-boundary.tsx index 0597d777c8a6a..19b64dcab7742 100644 --- a/apps/desktop/src/components/error-boundary.tsx +++ b/apps/desktop/src/components/error-boundary.tsx @@ -50,8 +50,22 @@ export class ErrorBoundary extends Component - {pane.render?.()} + {pane.render && } )} diff --git a/apps/desktop/src/components/pane-shell/tree/renderer/narrow-overlays.tsx b/apps/desktop/src/components/pane-shell/tree/renderer/narrow-overlays.tsx index 3d75388d77e9b..3c695cf19b083 100644 --- a/apps/desktop/src/components/pane-shell/tree/renderer/narrow-overlays.tsx +++ b/apps/desktop/src/components/pane-shell/tree/renderer/narrow-overlays.tsx @@ -9,7 +9,7 @@ import { useStore } from '@nanostores/react' import { useEffect, useMemo, useRef, useState } from 'react' -import { ContribBoundary } from '@/contrib/react/boundary' +import { ContribBoundary, ContribRender } from '@/contrib/react/boundary' import { useContributions } from '@/contrib/react/use-contributions' import type { Contribution } from '@/contrib/types' import { ESCAPE_PRIORITY, isTopEscapeLayer, pushEscapeLayer } from '@/lib/escape-layers' @@ -138,7 +138,9 @@ export function NarrowOverlays() { // width) instead of a fat fixed 20rem — capped for tiny screens. style={{ width: `min(${(revealed.data as { width?: string } | undefined)?.width ?? '18rem'}, 85vw)` }} > - {revealed.render?.()} + + {revealed.render && } + )} diff --git a/apps/desktop/src/global.d.ts b/apps/desktop/src/global.d.ts index 16351f112d31d..43116d0335cd5 100644 --- a/apps/desktop/src/global.d.ts +++ b/apps/desktop/src/global.d.ts @@ -198,6 +198,13 @@ declare global { } revealLogs: () => Promise<{ ok: boolean; path: string; error?: string }> getRecentLogs: () => Promise<{ path: string; lines: string[] }> + /** Persist a renderer error-boundary catch to desktop.log (fire-and-forget). */ + reportRendererError?: (report: { + label: string + boundary: string + message: string + componentStack: string + }) => void readDir: (path: string) => Promise gitRoot?: (path: string) => Promise // Reveal a path in the OS file manager (Finder / Explorer).