Suppress GPU shutdown crash prompts (#2759)
Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
parent
133861497f
commit
b5b29526a2
|
|
@ -9,6 +9,7 @@ describe('shouldRecordProcessGoneCrash', () => {
|
|||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: 'killed',
|
||||
exitCode: 15,
|
||||
expectedTeardown: 'renderer-reload'
|
||||
|
|
@ -17,17 +18,29 @@ describe('shouldRecordProcessGoneCrash', () => {
|
|||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'child',
|
||||
processType: 'GPU',
|
||||
reason: 'killed',
|
||||
exitCode: 15,
|
||||
expectedTeardown: 'app-shutdown'
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('records real crash reasons even during expected lifecycle teardown', () => {
|
||||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: 'killed',
|
||||
exitCode: 9,
|
||||
expectedTeardown: 'app-shutdown'
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('records real crash reasons during expected renderer-only teardown', () => {
|
||||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: 'crashed',
|
||||
exitCode: 5,
|
||||
expectedTeardown: 'renderer-reload'
|
||||
|
|
@ -36,6 +49,7 @@ describe('shouldRecordProcessGoneCrash', () => {
|
|||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: 'oom',
|
||||
exitCode: null,
|
||||
expectedTeardown: 'renderer-reload'
|
||||
|
|
@ -43,10 +57,47 @@ describe('shouldRecordProcessGoneCrash', () => {
|
|||
).toBe(true)
|
||||
})
|
||||
|
||||
it('suppresses crash-shaped GPU child exits during expected app shutdown', () => {
|
||||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'child',
|
||||
processType: 'GPU',
|
||||
reason: 'crashed',
|
||||
exitCode: 5,
|
||||
expectedTeardown: 'app-shutdown'
|
||||
})
|
||||
).toBe(false)
|
||||
})
|
||||
|
||||
it('still records crash-shaped non-GPU child exits during expected app shutdown', () => {
|
||||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'child',
|
||||
processType: 'Utility',
|
||||
reason: 'crashed',
|
||||
exitCode: 5,
|
||||
expectedTeardown: 'app-shutdown'
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('still records crash-shaped renderer exits during expected app shutdown', () => {
|
||||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: 'crashed',
|
||||
exitCode: 5,
|
||||
expectedTeardown: 'app-shutdown'
|
||||
})
|
||||
).toBe(true)
|
||||
})
|
||||
|
||||
it('skips SIGTERM killed events outside expected lifecycle teardown', () => {
|
||||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: 'killed',
|
||||
exitCode: 15,
|
||||
expectedTeardown: 'none'
|
||||
|
|
@ -58,6 +109,7 @@ describe('shouldRecordProcessGoneCrash', () => {
|
|||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: 'killed',
|
||||
exitCode: 9,
|
||||
expectedTeardown: 'none'
|
||||
|
|
@ -69,6 +121,7 @@ describe('shouldRecordProcessGoneCrash', () => {
|
|||
expect(
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'child',
|
||||
processType: 'GPU',
|
||||
reason: 'killed',
|
||||
exitCode: 9,
|
||||
expectedTeardown: 'renderer-reload'
|
||||
|
|
|
|||
|
|
@ -3,15 +3,26 @@ export type ExpectedTeardownScope = 'none' | 'renderer-reload' | 'app-shutdown'
|
|||
|
||||
export function shouldRecordProcessGoneCrash({
|
||||
source,
|
||||
processType,
|
||||
reason,
|
||||
exitCode,
|
||||
expectedTeardown
|
||||
}: {
|
||||
source: ProcessGoneSource
|
||||
processType: string
|
||||
reason: string
|
||||
exitCode: number | null
|
||||
expectedTeardown: ExpectedTeardownScope
|
||||
}): boolean {
|
||||
// Why: Chromium's GPU helper can emit a crash-shaped exit while Electron is
|
||||
// already intentionally exiting/relaunching; that is shutdown noise.
|
||||
if (
|
||||
expectedTeardown === 'app-shutdown' &&
|
||||
source === 'child' &&
|
||||
processType.toLowerCase() === 'gpu'
|
||||
) {
|
||||
return false
|
||||
}
|
||||
// Why: Electron reports intentional reload/update/quit teardown as `killed`.
|
||||
// Real renderer OOMs and Chromium crashes should still reach crash reporting.
|
||||
if (reason !== 'killed') {
|
||||
|
|
|
|||
|
|
@ -337,6 +337,7 @@ function openMainWindow(): BrowserWindow {
|
|||
shouldRecordRendererCrash: (details, webContentsId) =>
|
||||
shouldRecordProcessGoneCrash({
|
||||
source: 'renderer',
|
||||
processType: 'renderer',
|
||||
reason: details.reason,
|
||||
exitCode: details.exitCode ?? null,
|
||||
expectedTeardown: getExpectedTeardownScope(webContentsId)
|
||||
|
|
@ -398,7 +399,12 @@ function openMainWindow(): BrowserWindow {
|
|||
},
|
||||
agentAwakeService ?? undefined,
|
||||
crashReports ?? undefined,
|
||||
keybindings
|
||||
keybindings,
|
||||
{
|
||||
onBeforeRelaunch: () => {
|
||||
isQuitting = true
|
||||
}
|
||||
}
|
||||
)
|
||||
automations.setWebContents(window.webContents)
|
||||
automations.start()
|
||||
|
|
@ -524,6 +530,7 @@ function recordProcessGoneCrash(
|
|||
if (
|
||||
!shouldRecordProcessGoneCrash({
|
||||
source,
|
||||
processType,
|
||||
reason,
|
||||
exitCode,
|
||||
expectedTeardown: getExpectedTeardownScope(webContentsId)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,62 @@
|
|||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
const { handlers, appExitMock, appRelaunchMock } = vi.hoisted(() => ({
|
||||
handlers: new Map<string, (_event: unknown, args?: unknown) => unknown>(),
|
||||
appExitMock: vi.fn(),
|
||||
appRelaunchMock: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('electron', () => ({
|
||||
app: {
|
||||
exit: appExitMock,
|
||||
getAppPath: vi.fn(() => '/test/app'),
|
||||
isPackaged: false,
|
||||
relaunch: appRelaunchMock
|
||||
},
|
||||
BrowserWindow: {
|
||||
fromWebContents: vi.fn(() => null)
|
||||
},
|
||||
dialog: {
|
||||
showOpenDialog: vi.fn()
|
||||
},
|
||||
ipcMain: {
|
||||
handle: vi.fn((channel: string, handler: (_event: unknown, args?: unknown) => unknown) => {
|
||||
handlers.set(channel, handler)
|
||||
})
|
||||
}
|
||||
}))
|
||||
|
||||
vi.mock('@electron-toolkit/utils', () => ({
|
||||
is: { dev: true }
|
||||
}))
|
||||
|
||||
import { registerAppHandlers } from './app'
|
||||
|
||||
describe('registerAppHandlers', () => {
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers()
|
||||
handlers.clear()
|
||||
appExitMock.mockReset()
|
||||
appRelaunchMock.mockReset()
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
vi.useRealTimers()
|
||||
})
|
||||
|
||||
it('marks relaunch as expected shutdown before exiting', () => {
|
||||
const onBeforeRelaunch = vi.fn()
|
||||
registerAppHandlers({} as never, { onBeforeRelaunch })
|
||||
|
||||
handlers.get('app:relaunch')?.(null)
|
||||
|
||||
expect(onBeforeRelaunch).toHaveBeenCalledTimes(1)
|
||||
expect(appRelaunchMock).not.toHaveBeenCalled()
|
||||
expect(appExitMock).not.toHaveBeenCalled()
|
||||
|
||||
vi.advanceTimersByTime(150)
|
||||
|
||||
expect(appRelaunchMock).toHaveBeenCalledTimes(1)
|
||||
expect(appExitMock).toHaveBeenCalledWith(0)
|
||||
})
|
||||
})
|
||||
|
|
@ -22,6 +22,10 @@ import { isMarkdownDocumentName, markdownDocumentFromFilePath } from './markdown
|
|||
|
||||
const execFileAsync = promisify(execFile)
|
||||
|
||||
type RegisterAppHandlersOptions = {
|
||||
onBeforeRelaunch?: () => void
|
||||
}
|
||||
|
||||
async function pickFloatingMarkdownDocument(
|
||||
event: IpcMainInvokeEvent
|
||||
): Promise<MarkdownDocument | null> {
|
||||
|
|
@ -96,7 +100,7 @@ function resolveDevFeatureWallAssetDir(): string {
|
|||
return candidates.find((candidate) => existsSync(candidate)) ?? candidates[0]
|
||||
}
|
||||
|
||||
export function registerAppHandlers(store: Store): void {
|
||||
export function registerAppHandlers(store: Store, options: RegisterAppHandlersOptions = {}): void {
|
||||
ipcMain.handle('app:getFeatureWallAssetBaseUrl', (): string => getFeatureWallAssetBaseUrl())
|
||||
|
||||
ipcMain.handle('app:getIdentity', (): AppIdentity => {
|
||||
|
|
@ -166,6 +170,8 @@ export function registerAppHandlers(store: Store): void {
|
|||
// UI state before the window tears down. `app.relaunch()` schedules a
|
||||
// spawn; `app.exit(0)` triggers the actual quit without invoking
|
||||
// before-quit handlers that could block on confirmation dialogs.
|
||||
// Mark shutdown first because app.exit() can bypass the usual quit latch.
|
||||
options.onBeforeRelaunch?.()
|
||||
setTimeout(() => {
|
||||
app.relaunch()
|
||||
app.exit(0)
|
||||
|
|
|
|||
|
|
@ -331,6 +331,7 @@ describe('registerCoreHandlers', () => {
|
|||
const claudeAccounts = { marker: 'claudeAccounts' }
|
||||
const rateLimits = { marker: 'rateLimits' }
|
||||
const agentAwakeService = { marker: 'agentAwakeService' }
|
||||
const onBeforeRelaunch = vi.fn()
|
||||
|
||||
registerCoreHandlers(
|
||||
store as never,
|
||||
|
|
@ -345,13 +346,16 @@ describe('registerCoreHandlers', () => {
|
|||
null,
|
||||
undefined,
|
||||
undefined,
|
||||
agentAwakeService as never
|
||||
agentAwakeService as never,
|
||||
undefined,
|
||||
undefined,
|
||||
{ onBeforeRelaunch }
|
||||
)
|
||||
|
||||
expect(registerClaudeUsageHandlersMock).toHaveBeenCalledWith(claudeUsage)
|
||||
expect(registerCodexUsageHandlersMock).toHaveBeenCalledWith(codexUsage)
|
||||
expect(registerOpenCodeUsageHandlersMock).toHaveBeenCalledWith(openCodeUsage)
|
||||
expect(registerAppHandlersMock).toHaveBeenCalledWith(store)
|
||||
expect(registerAppHandlersMock).toHaveBeenCalledWith(store, { onBeforeRelaunch })
|
||||
expect(registerCodexAccountHandlersMock).toHaveBeenCalledWith(codexAccounts)
|
||||
expect(registerAgentHookHandlersMock).toHaveBeenCalled()
|
||||
expect(registerPetHandlersMock).toHaveBeenCalled()
|
||||
|
|
|
|||
|
|
@ -63,6 +63,10 @@ import type { KeybindingService } from '../keybindings/keybinding-service'
|
|||
|
||||
let registered = false
|
||||
|
||||
type CoreHandlerLifecycleOptions = {
|
||||
onBeforeRelaunch?: () => void
|
||||
}
|
||||
|
||||
export function registerCoreHandlers(
|
||||
store: Store,
|
||||
runtime: OrcaRuntimeService,
|
||||
|
|
@ -78,7 +82,8 @@ export function registerCoreHandlers(
|
|||
commitMessageAgentEnv?: CommitMessageAgentEnvironmentResolvers,
|
||||
agentAwakeService?: AgentAwakeService,
|
||||
crashReports?: CrashReportStore,
|
||||
keybindings?: KeybindingService
|
||||
keybindings?: KeybindingService,
|
||||
lifecycleOptions: CoreHandlerLifecycleOptions = {}
|
||||
): void {
|
||||
// Why: on macOS the app can stay alive after all windows close, then
|
||||
// openMainWindow() is called again on 'activate'. ipcMain.handle() throws
|
||||
|
|
@ -91,7 +96,7 @@ export function registerCoreHandlers(
|
|||
}
|
||||
registered = true
|
||||
|
||||
registerAppHandlers(store)
|
||||
registerAppHandlers(store, { onBeforeRelaunch: lifecycleOptions.onBeforeRelaunch })
|
||||
registerCliHandlers()
|
||||
registerPreflightHandlers()
|
||||
registerClaudeUsageHandlers(claudeUsage)
|
||||
|
|
|
|||
Loading…
Reference in New Issue