fix(desktop): single-owner console capture + HUD lifecycle coverage

Reconcile the salvaged #81533 lifecycle helper with the renderer-log
console pipeline that landed in #83535 (the two PRs raced):

- window-renderer-lifecycle.ts no longer handles console-message —
  renderer-log.ts is the single owner (per-window labels, boundary
  reports). One owner means no double-logged errors on windows wearing
  both, and OAuth/portal windows (lifecycle-wired for process events)
  cannot spill third-party page console output into desktop.log.
- wake indicator window gets attachRendererConsoleCapture, keeping the
  console coverage it previously got from the helper.
- HUD window (added after the PR branched) gets log-only lifecycle
  coverage — it was the one renderer window the PR couldn't have known
  about.
- Tests updated: lifecycle helper asserts it attaches NO console-message
  listener; parser tests live in renderer-log.test.ts.
This commit is contained in:
Teknium 2026-08-10 18:00:18 -07:00
parent 0c1a11ada6
commit 0a60b164f5
4 changed files with 19 additions and 84 deletions

View File

@ -9538,6 +9538,9 @@ function spawnHudWindow(sessionId, profile) {
})
attachRendererConsoleCapture(win, 'hud', rememberLog)
// Log-only lifecycle (#81290): the HUD is a compact auxiliary surface the
// user can re-toggle; a dead renderer should be diagnosable, not resurrected.
installWindowRendererLifecycle(win, { kind: 'hud', callbacks: { log: rememberLog } })
loadWindowUrl(win, hudUrl(sessionId, profile), 'HUD')
return win

View File

@ -2,6 +2,7 @@ import { pathToFileURL } from 'node:url'
import { BrowserWindow, screen } from 'electron'
import { attachRendererConsoleCapture } from './renderer-log'
import {
normalizeWakeIndicatorState,
selectWakeIndicatorDisplay,
@ -106,6 +107,9 @@ export function createWakeIndicatorWindowController({
// Log-only renderer lifecycle (#81290): the wake cue is ambient and
// macOS-only; its loss belongs in desktop.log, never resurrected.
installWindowRendererLifecycle(next, { kind: 'wake', callbacks: { log } })
// Console errors go through the shared capture (renderer-log.ts owns
// console-message; the lifecycle helper deliberately does not).
attachRendererConsoleCapture(next, 'wake', log)
next.webContents.on('did-finish-load', sendState)
next.once('ready-to-show', () => {

View File

@ -3,7 +3,6 @@ import assert from 'node:assert/strict'
import { test } from 'vitest'
import {
consoleMessageLog,
describeRendererLifecycleEvent,
installWindowRendererLifecycle,
pruneReloadTimes,
@ -296,21 +295,18 @@ test('did-fail-load on the main frame is logged, not reloaded', () => {
assert.equal(logs.length, 1)
})
test('console-message at error level is logged, lower levels ignored', () => {
test('console-message events are NOT handled here (renderer-log.ts is the single owner)', () => {
const win = makeFakeWindow()
const { logs, options } = makeOptions(win, 'secondary')
installWindowRendererLifecycle(win, options)
// Modern shape: (event, messageDetails).
// OAuth/portal windows install this helper for process events; their pages
// must not be able to spill console output (tokens/PII) into desktop.log.
win.webContents.emit('console-message', {}, { level: 3, message: 'boom', sourceUrl: 'file:///app.js', lineNumber: 42 })
// Deprecated positional shape: (event, level, message, line, sourceId).
win.webContents.emit('console-message', {}, 3, 'legacy boom', 7, 'file:///legacy.js')
win.webContents.emit('console-message', {}, { level: 1, message: 'info', sourceUrl: 'file:///app.js', lineNumber: 1 })
assert.equal(logs.length, 2)
assert.match(logs[0], /\[renderer:secondary console\] boom \(file:\/\/\/app\.js:42\)/)
assert.match(logs[1], /\[renderer:secondary console\] legacy boom \(file:\/\/\/legacy\.js:7\)/)
assert.equal(win.webContents.listenerCount('console-message'), 0)
assert.equal(logs.length, 0)
})
test('onCrashLoopSuppressed fires when the budget trips (main sandbox-relaunch hook)', async () => {
@ -371,16 +367,6 @@ test('dispose removes every listener (no stacking on window recreation)', () =>
assert.equal(win.webContents.listenerCount('render-process-gone'), before - 1)
})
test('consoleMessageLog parses both Electron argument shapes', () => {
const modern = consoleMessageLog([{}, { level: 3, message: 'm', sourceUrl: 's', lineNumber: 1 }])
assert.deepEqual(modern, { level: 3, message: 'm', sourceUrl: 's', lineNumber: 1 })
const legacy = consoleMessageLog([{}, 3, 'm', 7, 's'])
assert.deepEqual(legacy, { level: 3, message: 'm', sourceUrl: 's', lineNumber: 7 })
})
test('describeRendererLifecycleEvent sanitizes unknown fields', () => {
assert.equal(describeRendererLifecycleEvent({ kind: 'secondary', event: 'render-process-gone' }), '[renderer:secondary] render-process-gone reason=? exitCode=?')
assert.equal(describeRendererLifecycleEvent({ kind: 'secondary', event: 'render-process-gone', reason: 'crashed', exitCode: undefined }), '[renderer:secondary] render-process-gone reason=crashed exitCode=?')

View File

@ -22,8 +22,12 @@
// - `did-fail-load` on the MAIN frame → log only (no blind reload: a repeatable
// startup failure would boot-loop; the backend startup path already surfaces
// the actionable error).
// - `console-message` at error level → log renderer console errors, same as the
// primary window did.
//
// Console-message capture is deliberately NOT here: renderer-log.ts owns it
// (per-window labels, boundary-report formatting). Keeping one owner avoids
// double-logging on windows that have both, and keeps third-party pages
// (OAuth/portal windows, which install this helper for process events) from
// spilling their console output — potentially tokens/PII — into desktop.log.
export interface RendererLifecycleDetails {
reason?: string
@ -33,7 +37,7 @@ export interface RendererLifecycleDetails {
export interface RendererLifecycleEvent {
kind: string
event: 'render-process-gone' | 'unresponsive' | 'did-fail-load' | 'console-message'
event: 'render-process-gone' | 'unresponsive' | 'did-fail-load'
reason?: string
exitCode?: number | string | undefined
isDestroyed?: boolean
@ -43,12 +47,6 @@ export interface RendererLifecycleEvent {
errorCode?: number | string | undefined
/** did-fail-load: the URL that failed. */
url?: string
/** console-message: renderer error text. */
message?: string
/** console-message: source URL of the console message. */
sourceUrl?: string
/** console-message: line number of the console message. */
lineNumber?: number | string | undefined
}
export interface ReloadPolicyDecision {
@ -160,13 +158,6 @@ export function shouldReloadAfterRendererGone(details: {
export function describeRendererLifecycleEvent(event: RendererLifecycleEvent): string {
const kind = String(event.kind || '?')
if (event.event === 'console-message') {
const src = String(event.sourceUrl || '?')
const line = event.lineNumber === undefined ? '?' : String(event.lineNumber)
return `[renderer:${kind} console] ${String(event.message || '(empty)')} (${src}:${line})`
}
if (event.event === 'unresponsive') {
return `[renderer:${kind}] webContents became unresponsive`
}
@ -185,35 +176,6 @@ export function describeRendererLifecycleEvent(event: RendererLifecycleEvent): s
return `[renderer:${kind}] render-process-gone reason=${reason} exitCode=${exitCode}${teardown}`
}
// Electron ≥36 passes (event, messageDetails); older/deprecated shape is
// (event, level, message, line, sourceId). Handle both, matching the primary
// window's previous console-message handling.
export function consoleMessageLog(args: readonly unknown[]): {
level: number
message: string
sourceUrl: string
lineNumber: number | string | undefined
} {
const details = args[1] && typeof args[1] === 'object' ? (args[1] as Record<string, unknown>) : null
const level = details ? Number(details.level ?? 0) : Number(args[1] ?? 0)
const modernLine = details?.lineNumber
return {
level,
message: details ? String(details.message ?? '') : String(args[2] ?? ''),
sourceUrl: details ? String(details.sourceUrl ?? '') : String(args[4] ?? ''),
lineNumber: details
? typeof modernLine === 'number'
? modernLine
: modernLine === undefined
? undefined
: String(modernLine)
: typeof args[3] === 'number'
? args[3]
: String(args[3] ?? '')
}
}
/**
* Attach renderer lifecycle listeners to a window. Returns a dispose() that
* removes every listener (window recreation must not stack handlers).
@ -302,28 +264,9 @@ export function installWindowRendererLifecycle(
}
}
const onConsoleMessage = (...args: unknown[]) => {
const parsed = consoleMessageLog(args)
if (parsed.level !== 3) {
return
}
log(
describeRendererLifecycleEvent({
kind,
event: 'console-message',
message: parsed.message,
sourceUrl: parsed.sourceUrl,
lineNumber: parsed.lineNumber
})
)
}
contents.on('render-process-gone', onRendererGone)
contents.on('unresponsive', onUnresponsive)
contents.on('did-fail-load', onDidFailLoad)
contents.on('console-message', onConsoleMessage)
let disposed = false
@ -338,7 +281,6 @@ export function installWindowRendererLifecycle(
contents.removeListener('render-process-gone', onRendererGone)
contents.removeListener('unresponsive', onUnresponsive)
contents.removeListener('did-fail-load', onDidFailLoad)
contents.removeListener('console-message', onConsoleMessage)
}
}
}