mirror of
https://github.com/stablyai/orca.git
synced 2026-10-23 16:03:49 +00:00
fix(pty): restore the webContents liveness guard dropped by #15927 (STA-5373)
Release blocker. #15927 replaced a two-condition renderer-liveness guard with a one-condition one, dropping the `webContents.isDestroyed()` half: v1.4.188 src/main/ipc/pty.ts webContents.isDestroyed x4 main src/main/ipc/pty.ts webContents.isDestroyed x0 A window can be alive while its webContents is already destroyed — teardown, a renderer crash, a reload in flight. `webContents.send()` throws in that state, and neither affected call site is inside a try/catch: the daemon-death fan-out at ipc/pty.ts:4067 and reportUnavailablePtyWrite at :7236. main-process-error-guards exempts only EIO/EPIPE and re-throws everything else on the next tick, so the whole app exits and takes every terminal with it. The original guard (STA-2373, #10065) also carried a typeof check so a stubbed or partially torn down webContents could not make the liveness check itself throw. Both halves are restored. Verified in both directions: reverting to the shipped one-condition version fails three of the seven new tests, including the send path. That the refactor was behaviour-preserving everywhere else is why this slipped — the guard was the one place where collapsing two conditions into "renderer gone" was not equivalent.
This commit is contained in:
@@ -0,0 +1,65 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
import { isRendererGone, rendererWebContents, sendToRenderer } from './pty-renderer-surface'
|
||||
|
||||
/**
|
||||
* Regression cover for STA-5373: #15927 replaced a two-condition renderer-liveness
|
||||
* guard with a one-condition one, dropping the `webContents.isDestroyed()` half.
|
||||
*
|
||||
* That is fatal rather than cosmetic. `webContents.send()` throws on a destroyed
|
||||
* webContents, the daemon-death fan-out calling it is not inside a try/catch, and an
|
||||
* uncaught throw in the Electron main process exits the app — killing every terminal.
|
||||
* A live window with dead webContents is the exact state a renderer crash or an
|
||||
* in-flight reload produces.
|
||||
*/
|
||||
function fakeWindow(options: {
|
||||
windowDestroyed?: boolean
|
||||
contentsDestroyed?: boolean
|
||||
omitIsDestroyed?: boolean
|
||||
send?: () => void
|
||||
}): never {
|
||||
const contents = {
|
||||
send: options.send ?? (() => {}),
|
||||
...(options.omitIsDestroyed ? {} : { isDestroyed: () => options.contentsDestroyed === true })
|
||||
}
|
||||
return {
|
||||
isDestroyed: () => options.windowDestroyed === true,
|
||||
webContents: contents
|
||||
} as never
|
||||
}
|
||||
|
||||
describe('pty renderer surface', () => {
|
||||
it('treats a live window with destroyed webContents as gone', () => {
|
||||
expect(isRendererGone(fakeWindow({ contentsDestroyed: true }))).toBe(true)
|
||||
})
|
||||
|
||||
it('does not send to a live window whose webContents is destroyed', () => {
|
||||
const send = vi.fn()
|
||||
sendToRenderer(fakeWindow({ contentsDestroyed: true, send }), 'pty:data', { id: 'x' })
|
||||
expect(send).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still sends when both the window and its webContents are alive', () => {
|
||||
const send = vi.fn()
|
||||
sendToRenderer(fakeWindow({ send }), 'pty:data', { id: 'x' })
|
||||
expect(send).toHaveBeenCalledExactlyOnceWith('pty:data', { id: 'x' })
|
||||
})
|
||||
|
||||
it('treats a destroyed window as gone without consulting webContents', () => {
|
||||
expect(isRendererGone(fakeWindow({ windowDestroyed: true }))).toBe(true)
|
||||
})
|
||||
|
||||
it('treats a missing renderer as gone', () => {
|
||||
expect(isRendererGone(null)).toBe(true)
|
||||
expect(rendererWebContents(null)).toBeNull()
|
||||
})
|
||||
|
||||
it('survives a webContents that does not expose isDestroyed', () => {
|
||||
// Why: the original fix carried a typeof guard, so a partially torn down or stubbed
|
||||
// webContents cannot make the liveness check itself throw.
|
||||
expect(() => isRendererGone(fakeWindow({ omitIsDestroyed: true }))).not.toThrow()
|
||||
})
|
||||
|
||||
it('withholds webContents from callers once it is destroyed', () => {
|
||||
expect(rendererWebContents(fakeWindow({ contentsDestroyed: true }))).toBeNull()
|
||||
})
|
||||
})
|
||||
@@ -14,9 +14,25 @@ import type { BrowserWindow, WebContents } from 'electron'
|
||||
* already guards on `isDestroyed()` and skips — so model it as `null` and say so.
|
||||
*/
|
||||
|
||||
/** True when there is no renderer, or it is gone. Callers already treat these the same. */
|
||||
/**
|
||||
* True when there is no renderer, or it is gone. Callers already treat these the same.
|
||||
*
|
||||
* Why webContents is checked separately (STA-2373, re-broken by #15927 as STA-5373): a
|
||||
* window can be alive while its webContents is already destroyed — teardown, a renderer
|
||||
* crash, a reload in flight. `webContents.send()` then THROWS, and the daemon-death
|
||||
* fan-out that calls it is not inside a try/catch, so an uncaught throw in the main
|
||||
* process kills the whole app and every terminal with it.
|
||||
*
|
||||
* Why the typeof guard: preserved from the original fix — a stubbed or partially torn
|
||||
* down webContents may not expose the method, and this must never be the thing that
|
||||
* throws.
|
||||
*/
|
||||
export function isRendererGone(window: BrowserWindow | null): boolean {
|
||||
return window === null || window.isDestroyed()
|
||||
if (window === null || window.isDestroyed()) {
|
||||
return true
|
||||
}
|
||||
const contents = window.webContents as WebContents | undefined
|
||||
return !contents || (typeof contents.isDestroyed === 'function' && contents.isDestroyed())
|
||||
}
|
||||
|
||||
/** Send to the renderer if one is listening. Absent renderer drops the message, as a destroyed one does. */
|
||||
|
||||
Reference in New Issue
Block a user