fix(desktop): persist renderer crashes to desktop.log + finish the render() isolation class
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.
This commit is contained in:
parent
9b1a2a14ca
commit
e5e2fb8b2d
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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),
|
||||
|
|
|
|||
|
|
@ -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')
|
||||
})
|
||||
})
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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: <ContribRender render={c.render} />.
|
||||
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: <ContribRender render={...} /> from @/contrib/react/boundary.'
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -50,8 +50,22 @@ export class ErrorBoundary extends Component<ErrorBoundaryProps, ErrorBoundarySt
|
|||
}
|
||||
|
||||
componentDidCatch(error: Error, info: ErrorInfo) {
|
||||
const tag = this.props.label ? `[error-boundary:${this.props.label}]` : '[error-boundary]'
|
||||
const label = this.props.label ?? ''
|
||||
const tag = label ? `[error-boundary:${label}]` : '[error-boundary]'
|
||||
console.error(tag, error, info.componentStack)
|
||||
// Persist to desktop.log via Electron (#79428): console.error only reaches
|
||||
// the main process for windows with a console hook, is minified, and loses
|
||||
// the component stack. This survives the window and names the component.
|
||||
try {
|
||||
window.hermesDesktop?.reportRendererError?.({
|
||||
label: new URLSearchParams(window.location.search).get('win') ?? 'main',
|
||||
boundary: label || 'unlabeled',
|
||||
message: error.message,
|
||||
componentStack: info.componentStack ?? ''
|
||||
})
|
||||
} catch {
|
||||
// Logging must never take the boundary down with it.
|
||||
}
|
||||
this.props.onError?.(error, info)
|
||||
|
||||
if (this.props.label === 'root' && isTransientAssistantUiLookupError(error) && this.takeAutoRecoveryAttempt()) {
|
||||
|
|
|
|||
|
|
@ -14,7 +14,7 @@ import { type PointerEvent as ReactPointerEvent, useCallback, useEffect, useRef,
|
|||
import { HUD_SURFACE } from '@/app/floating-hud'
|
||||
import { TITLEBAR_HEIGHT } from '@/app/shell/titlebar'
|
||||
import { Codicon } from '@/components/ui/codicon'
|
||||
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 { readJson, writeJson } from '@/lib/storage'
|
||||
|
|
@ -178,7 +178,7 @@ function FloatingPane({ pane }: { pane: Contribution }) {
|
|||
|
||||
{!collapsed && (
|
||||
<div className="min-h-0 flex-1 overflow-auto">
|
||||
<ContribBoundary id={pane.id}>{pane.render?.()}</ContribBoundary>
|
||||
<ContribBoundary id={pane.id}>{pane.render && <ContribRender render={pane.render} />}</ContribBoundary>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -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)` }}
|
||||
>
|
||||
<ContribBoundary id={revealed.id}>{revealed.render?.()}</ContribBoundary>
|
||||
<ContribBoundary id={revealed.id}>
|
||||
{revealed.render && <ContribRender render={revealed.render} />}
|
||||
</ContribBoundary>
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
|
|
|
|||
|
|
@ -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<HermesReadDirResult>
|
||||
gitRoot?: (path: string) => Promise<string | null>
|
||||
// Reveal a path in the OS file manager (Finder / Explorer).
|
||||
|
|
|
|||
Loading…
Reference in New Issue