mirror of
https://github.com/stablyai/orca.git
synced 2026-10-21 08:03:33 +00:00
* fix(runtime): surface desktop RPC startup failures
* fix(runtime): isolate RPC failure telemetry
* fix(runtime): satisfy the changed-code quality gate and kill vacuous dialog tests
The `no-floating-promises` label span covers the whole `app.whenReady().then()`
callback, so adding lines inside it made a long-standing finding overlap changed
code. `void` is the linter's own suppression; no `.catch()` on purpose.
The startup-failure tests were vacuous: mutation runs showed the wait-for-show
deferral, the destroyed-window guard, the `closed` companion event, listener
cleanup, the cause walk, the cycle guard, and the truncation bound could all be
deleted with every test still green. The "not called yet" assertion ran before
any microtask, so it passed either way.
* test(runtime): de-brittle the desktop RPC-failure source assertions
Anchoring the slice on the full destructure and matching the whole dialog
call expression made an innocuous rename break the test with a cryptic
'expected -1'. Match the shape that is actually the contract instead.
* test(runtime): repair the silently-unbounded desktop startup slice
The desktopEnd anchor comment lost a word in 98b00d3a64, so indexOf returned
-1 and slice(start, -1) covered index.ts to EOF. Moving the dialog call to a
path that never runs at startup still passed. Anchor on code instead, and
assert both bounds so a future reword fails loudly.
* test(runtime): bound the attach anchors in the startup ordering slice
Round 3 bounded the desktop pair but left attachStart/attachEnd unguarded in
the same test: deleting the PTY startup barrier from attachMainWindowServices()
and breaking the rateLimits.attach(window) end anchor still left the case green.
* test(startup): bound the last two unguarded slice anchors in this file
Rounds 3 and 4 fixed the desktop and attach pairs; two instances of the same
class survived in the same file, both proven vacuous by mutation:
- it #3 never bounded readyEnd. Renaming the `pairing:` payload key makes it
-1, widening readyPayload from 372B to ~52KB. Moving the reconciliation
status out of the serve-ready payload (its whole point) but leaving it later
in index.ts then kept all 6 cases green.
- it #2 bounded desktopWindowStart against reconciliationStart rather than
serveEnd. An earlier `Promise.resolve(openMainWindow())` steals the anchor,
collapsing desktopStartup to '' while every existing guard still passes, so
its only assertion — a negative — succeeds against an empty string.
Both mutants now fail. `src/main/ipc/pty-startup-barrier-ordering.test.ts:11`
has the same latent shape; left alone as out of scope for this PR.
* fix(runtime): keep walking the cause chain past an unmapped code
getErrorCode returned the first code it found, so an outer wrapper carrying
an unrecognised code masked a nested EACCES/ENOSPC and classified it unknown.
Only a mapped code ends the walk now; every other input classifies as before.
Unreachable today (writeSecureFile rethrows raw fs errors with .code intact),
but the classifier's job is surviving whatever error shape reaches it.
* fix(runtime): tell the user what to fix, not just to restart
The dialog's only advice was "Restart Orca to try again", which is true for
address_in_use and wrong for the rest: permissions, a full or read-only disk,
and a missing data folder all survive a relaunch, so the user restarted, hit
the same failure and had no next step.
Route the error class we already compute into the copy so each cause names the
thing the user has to change. Guidance and telemetry now derive from the same
classifier, so they cannot drift apart.
* fix(runtime): guide users through long RPC paths
* fix(runtime): avoid false window listener warning
* fix(runtime): guard destroyed window before web contents
271 lines
9.5 KiB
TypeScript
271 lines
9.5 KiB
TypeScript
import { EventEmitter } from 'node:events'
|
|
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
|
|
|
const { showMessageBoxMock, trackMock } = vi.hoisted(() => ({
|
|
showMessageBoxMock: vi.fn(),
|
|
trackMock: vi.fn()
|
|
}))
|
|
|
|
vi.mock('electron', () => ({
|
|
dialog: {
|
|
showMessageBox: showMessageBoxMock
|
|
}
|
|
}))
|
|
|
|
vi.mock('../i18n/main-i18n', () => ({
|
|
// Why: substitute every supplied placeholder, not just {{cause}} — a mock that ignores one
|
|
// would leave a literal {{...}} in the detail and hide it from every assertion below.
|
|
translateMain: (
|
|
_key: string,
|
|
fallback: string,
|
|
options?: Readonly<Record<string, string>>
|
|
): string =>
|
|
Object.entries(options ?? {}).reduce(
|
|
(text, [name, value]) => text.replaceAll(`{{${name}}}`, value),
|
|
fallback
|
|
)
|
|
}))
|
|
|
|
vi.mock('../telemetry/client', () => ({
|
|
track: trackMock
|
|
}))
|
|
|
|
import {
|
|
classifyRuntimeRpcStartFailure,
|
|
recordRuntimeRpcStartFailure,
|
|
showRuntimeRpcStartupFailureDialog
|
|
} from './runtime-rpc-startup-failure'
|
|
|
|
type FakeParentWindow = Electron.BrowserWindow & EventEmitter
|
|
|
|
function createParentWindow(
|
|
visible = true,
|
|
destroyed = false,
|
|
webContentsDestroyed = destroyed
|
|
): FakeParentWindow {
|
|
const webContents = Object.assign(new EventEmitter(), {
|
|
isDestroyed: () => webContentsDestroyed
|
|
})
|
|
const parentWindow = Object.assign(new EventEmitter(), {
|
|
isDestroyed: () => destroyed,
|
|
isVisible: () => visible
|
|
}) as unknown as FakeParentWindow
|
|
Object.defineProperty(parentWindow, 'webContents', {
|
|
get: () => {
|
|
if (destroyed) {
|
|
throw new Error('Object has been destroyed')
|
|
}
|
|
return webContents
|
|
}
|
|
})
|
|
return parentWindow
|
|
}
|
|
|
|
// Why: the dialog is deferred behind an await, so a synchronous "not called yet" assertion
|
|
// would pass even if the deferral were deleted; drain the microtask queue first.
|
|
function flushMicrotasks(): Promise<void> {
|
|
return new Promise((resolve) => {
|
|
setImmediate(resolve)
|
|
})
|
|
}
|
|
|
|
describe('runtime RPC startup failure reporting', () => {
|
|
beforeEach(() => {
|
|
showMessageBoxMock.mockReset().mockResolvedValue({ response: 0 })
|
|
trackMock.mockReset()
|
|
})
|
|
|
|
it.each([
|
|
['EACCES', 'permission_denied'],
|
|
['EPERM', 'permission_denied'],
|
|
['EADDRINUSE', 'address_in_use'],
|
|
['ENOSPC', 'storage_unavailable'],
|
|
['EROFS', 'storage_unavailable'],
|
|
['EINVAL', 'invalid_path'],
|
|
['ENOENT', 'invalid_path'],
|
|
['ENAMETOOLONG', 'invalid_path'],
|
|
['unexpected', 'unknown']
|
|
] as const)('classifies %s without exposing the raw error', (code, expected) => {
|
|
const error = Object.assign(new Error('/Users/private/orca-runtime.json'), { code })
|
|
|
|
expect(classifyRuntimeRpcStartFailure(error)).toBe(expected)
|
|
})
|
|
|
|
it('classifies a code carried on a wrapped cause', () => {
|
|
const error = new Error('failed to publish orca-runtime.json', {
|
|
cause: Object.assign(new Error('read-only volume'), { code: 'EROFS' })
|
|
})
|
|
|
|
expect(classifyRuntimeRpcStartFailure(error)).toBe('storage_unavailable')
|
|
})
|
|
|
|
it('walks past an unmapped wrapper code to the mapped cause', () => {
|
|
const error = Object.assign(new Error('failed to publish orca-runtime.json'), {
|
|
code: 'ERR_PUBLISH_FAILED',
|
|
cause: Object.assign(new Error('permission denied'), { code: 'EACCES' })
|
|
})
|
|
|
|
expect(classifyRuntimeRpcStartFailure(error)).toBe('permission_denied')
|
|
})
|
|
|
|
it('survives a self-referential cause chain', () => {
|
|
const error: Error & { cause?: unknown } = new Error('cyclic')
|
|
error.cause = error
|
|
|
|
expect(classifyRuntimeRpcStartFailure(error)).toBe('unknown')
|
|
})
|
|
|
|
it('records a privacy-safe telemetry event', () => {
|
|
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
const error = Object.assign(new Error('/Users/private/orca-runtime.json'), { code: 'EACCES' })
|
|
|
|
recordRuntimeRpcStartFailure(error)
|
|
|
|
expect(trackMock).toHaveBeenCalledWith('runtime_rpc_start_failed', {
|
|
error_class: 'permission_denied'
|
|
})
|
|
expect(JSON.stringify(trackMock.mock.calls)).not.toContain('/Users/private')
|
|
consoleError.mockRestore()
|
|
})
|
|
|
|
it('does not let telemetry failure escape the startup failure handler', () => {
|
|
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
const telemetryError = new Error('telemetry unavailable')
|
|
trackMock.mockImplementationOnce(() => {
|
|
throw telemetryError
|
|
})
|
|
|
|
expect(() => recordRuntimeRpcStartFailure(new Error('RPC failed'))).not.toThrow()
|
|
expect(consoleError).toHaveBeenCalledWith(
|
|
'[runtime] Failed to record RPC startup failure telemetry:',
|
|
telemetryError
|
|
)
|
|
consoleError.mockRestore()
|
|
})
|
|
|
|
it('shows the CLI impact and local cause', async () => {
|
|
const parentWindow = createParentWindow()
|
|
const error = new Error('metadata write failed')
|
|
|
|
await showRuntimeRpcStartupFailureDialog(parentWindow, error)
|
|
|
|
expect(showMessageBoxMock).toHaveBeenCalledWith(
|
|
parentWindow,
|
|
expect.objectContaining({
|
|
type: 'error',
|
|
title: 'Orca CLI unavailable',
|
|
message: "Orca couldn't start its local command transport.",
|
|
detail: expect.stringMatching(
|
|
/orca status.*orca terminal.*orchestration.*Cause: metadata write failed/s
|
|
)
|
|
})
|
|
)
|
|
})
|
|
|
|
// Why: a bare "restart" is only true for address_in_use — the other classes need the user to
|
|
// change something, so each must reach the dialog with its own remediation.
|
|
it.each([
|
|
['EACCES', "Check permissions on Orca's data folder"],
|
|
['EPERM', "Check permissions on Orca's data folder"],
|
|
['ENOSPC', 'Your disk may be full or read-only'],
|
|
['EROFS', 'Your disk may be full or read-only'],
|
|
['EINVAL', 'at a path that is too long'],
|
|
['ENAMETOOLONG', 'at a path that is too long'],
|
|
['ENOENT', "Orca's data folder may be missing"],
|
|
['EADDRINUSE', 'Another process may be holding the port']
|
|
] as const)('guides the user on how to fix %s', async (code, guidance) => {
|
|
const error = Object.assign(new Error('metadata write failed'), { code })
|
|
|
|
await showRuntimeRpcStartupFailureDialog(createParentWindow(), error)
|
|
|
|
const detail = showMessageBoxMock.mock.calls[0]?.[1]?.detail as string
|
|
expect(detail).toContain(guidance)
|
|
expect(detail).not.toContain('{{')
|
|
})
|
|
|
|
it('falls back to a plain restart when the cause is unrecognised', async () => {
|
|
await showRuntimeRpcStartupFailureDialog(createParentWindow(), new Error('mystery'))
|
|
|
|
const detail = showMessageBoxMock.mock.calls[0]?.[1]?.detail as string
|
|
expect(detail).toContain('Restart Orca to try again.')
|
|
expect(detail).not.toContain("Check permissions on Orca's data folder")
|
|
})
|
|
|
|
it('truncates a runaway cause instead of pasting it whole into the dialog', async () => {
|
|
await showRuntimeRpcStartupFailureDialog(createParentWindow(), new Error('x'.repeat(900)))
|
|
|
|
const detail = showMessageBoxMock.mock.calls[0]?.[1]?.detail as string
|
|
const cause = detail.slice(detail.indexOf('Cause: ') + 'Cause: '.length)
|
|
expect(cause).toHaveLength(500)
|
|
expect(cause.endsWith('…')).toBe(true)
|
|
})
|
|
|
|
it('waits until the app window is visible', async () => {
|
|
const parentWindow = createParentWindow(false)
|
|
const reporting = showRuntimeRpcStartupFailureDialog(
|
|
parentWindow,
|
|
new Error('metadata write failed')
|
|
)
|
|
|
|
await flushMicrotasks()
|
|
expect(showMessageBoxMock).not.toHaveBeenCalled()
|
|
parentWindow.emit('show')
|
|
await reporting
|
|
|
|
expect(showMessageBoxMock).toHaveBeenCalledOnce()
|
|
expect(parentWindow.listenerCount('show')).toBe(0)
|
|
expect(parentWindow.webContents.listenerCount('destroyed')).toBe(0)
|
|
})
|
|
|
|
it('never shows a dialog against an already destroyed window', async () => {
|
|
const parentWindow = createParentWindow(false, true)
|
|
|
|
await showRuntimeRpcStartupFailureDialog(parentWindow, new Error('metadata write failed'))
|
|
|
|
expect(showMessageBoxMock).not.toHaveBeenCalled()
|
|
expect(parentWindow.listenerCount('show')).toBe(0)
|
|
})
|
|
|
|
it('never waits on already destroyed web contents', async () => {
|
|
const parentWindow = createParentWindow(false, false, true)
|
|
|
|
await showRuntimeRpcStartupFailureDialog(parentWindow, new Error('metadata write failed'))
|
|
|
|
expect(showMessageBoxMock).not.toHaveBeenCalled()
|
|
expect(parentWindow.listenerCount('show')).toBe(0)
|
|
expect(parentWindow.webContents.listenerCount('destroyed')).toBe(0)
|
|
})
|
|
|
|
it('drops the pending dialog and its listeners when the window closes first', async () => {
|
|
const parentWindow = createParentWindow(false)
|
|
const reporting = showRuntimeRpcStartupFailureDialog(
|
|
parentWindow,
|
|
new Error('metadata write failed')
|
|
)
|
|
|
|
await flushMicrotasks()
|
|
expect(parentWindow.listenerCount('closed')).toBe(0)
|
|
expect(parentWindow.webContents.listenerCount('destroyed')).toBe(1)
|
|
parentWindow.webContents.emit('destroyed')
|
|
await reporting
|
|
|
|
expect(showMessageBoxMock).not.toHaveBeenCalled()
|
|
expect(parentWindow.listenerCount('show')).toBe(0)
|
|
expect(parentWindow.webContents.listenerCount('destroyed')).toBe(0)
|
|
})
|
|
|
|
it('logs instead of rejecting if Electron cannot show the dialog', async () => {
|
|
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => {})
|
|
showMessageBoxMock.mockRejectedValueOnce(new Error('window closed'))
|
|
|
|
await expect(
|
|
showRuntimeRpcStartupFailureDialog(createParentWindow(), new Error('failed'))
|
|
).resolves.toBeUndefined()
|
|
expect(consoleError).toHaveBeenCalledWith(
|
|
'[runtime] Failed to show RPC startup failure dialog:',
|
|
expect.any(Error)
|
|
)
|
|
consoleError.mockRestore()
|
|
})
|
|
})
|