mirror of
https://github.com/stablyai/orca
synced 2026-04-21 14:17:16 +00:00
Normalize browser tab close race errors
This commit is contained in:
parent
68193377d5
commit
20a25ee960
2 changed files with 136 additions and 10 deletions
|
|
@ -104,6 +104,9 @@ function failWith(error: string): void {
|
|||
})
|
||||
}
|
||||
|
||||
const CDP_DISCOVERY_FAILURE =
|
||||
'Auto-launch failed: All CDP discovery methods failed: connect ECONNREFUSED 127.0.0.1:9222; WebSocket connect failed'
|
||||
|
||||
describe('AgentBrowserBridge', () => {
|
||||
let bridge: AgentBrowserBridge
|
||||
|
||||
|
|
@ -204,6 +207,57 @@ describe('AgentBrowserBridge', () => {
|
|||
await expect(bridge.click('@e1')).rejects.toThrow('Element not found')
|
||||
})
|
||||
|
||||
it('keeps CDP discovery failures generic while the tab session is still live', async () => {
|
||||
failWith(CDP_DISCOVERY_FAILURE)
|
||||
await expect(bridge.snapshot()).rejects.toMatchObject({
|
||||
code: 'browser_error',
|
||||
message: CDP_DISCOVERY_FAILURE
|
||||
})
|
||||
})
|
||||
|
||||
it('maps in-flight CDP discovery failures to tab not found after the session disappears', async () => {
|
||||
let releaseSnapshot: (() => void) | null = null
|
||||
const activeChild = { kill: vi.fn() }
|
||||
execFileMock.mockImplementation(
|
||||
(_bin: string, args: string[], _opts: unknown, cb: Function) => {
|
||||
if (args.includes('snapshot')) {
|
||||
releaseSnapshot = () => {
|
||||
cb(null, JSON.stringify({ success: false, error: CDP_DISCOVERY_FAILURE }), '')
|
||||
}
|
||||
return activeChild
|
||||
}
|
||||
cb(null, JSON.stringify({ success: true, data: null }), '')
|
||||
return { kill: vi.fn() }
|
||||
}
|
||||
)
|
||||
|
||||
const snapshotPromise = bridge.snapshot()
|
||||
|
||||
await vi.waitFor(() => {
|
||||
expect(releaseSnapshot).not.toBeNull()
|
||||
})
|
||||
// Why: this reproduces the teardown race where the tab close path has
|
||||
// already removed the bridge session before agent-browser reports that
|
||||
// its CDP proxy disappeared.
|
||||
;(bridge as unknown as { sessions: Map<string, unknown> }).sessions.delete('orca-tab-tab-1')
|
||||
releaseSnapshot!()
|
||||
|
||||
await expect(snapshotPromise).rejects.toMatchObject({
|
||||
code: 'browser_tab_not_found',
|
||||
message: 'Browser page tab-1 is no longer available'
|
||||
})
|
||||
})
|
||||
|
||||
it('maps target disappearance during session creation to tab not found', async () => {
|
||||
const wc = mockWebContents(100)
|
||||
webContentsFromIdMock.mockImplementationOnce(() => wc).mockImplementationOnce(() => null)
|
||||
|
||||
await expect(bridge.snapshot(undefined, 'tab-1')).rejects.toMatchObject({
|
||||
code: 'browser_tab_not_found',
|
||||
message: 'Browser page tab-1 is no longer available'
|
||||
})
|
||||
})
|
||||
|
||||
it('handles malformed JSON from agent-browser', async () => {
|
||||
execFileMock.mockImplementation(
|
||||
(_bin: string, _args: string[], _opts: unknown, cb: Function) => {
|
||||
|
|
@ -571,7 +625,10 @@ describe('AgentBrowserBridge', () => {
|
|||
).destroySession('orca-tab-tab-1')
|
||||
|
||||
expect(activeChild.kill).toHaveBeenCalledTimes(1)
|
||||
await expect(runningSnapshot).rejects.toThrow('Session destroyed while command was running')
|
||||
await expect(runningSnapshot).rejects.toMatchObject({
|
||||
code: 'browser_tab_closed',
|
||||
message: 'Tab was closed while command was running'
|
||||
})
|
||||
await destroyPromise
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -169,6 +169,20 @@ function classifyErrorCode(message: string): string {
|
|||
return 'browser_error'
|
||||
}
|
||||
|
||||
function isTabClosedTransportError(message: string): boolean {
|
||||
return /session destroyed while command|session destroyed while commands|connection refused|cdp discovery methods failed|websocket connect failed/i.test(
|
||||
message
|
||||
)
|
||||
}
|
||||
|
||||
function pageUnavailableMessageForSession(sessionName: string): string {
|
||||
const prefix = 'orca-tab-'
|
||||
const browserPageId = sessionName.startsWith(prefix) ? sessionName.slice(prefix.length) : null
|
||||
return browserPageId
|
||||
? `Browser page ${browserPageId} is no longer available`
|
||||
: 'Browser tab is no longer available'
|
||||
}
|
||||
|
||||
function translateResult(
|
||||
stdout: string
|
||||
): { ok: true; result: unknown } | { ok: false; error: { code: string; message: string } } {
|
||||
|
|
@ -1555,7 +1569,7 @@ export class AgentBrowserBridge {
|
|||
|
||||
private async ensureSession(
|
||||
sessionName: string,
|
||||
_browserPageId: string,
|
||||
browserPageId: string,
|
||||
webContentsId: number
|
||||
): Promise<void> {
|
||||
const pendingDestruction = this.pendingSessionDestruction.get(sessionName)
|
||||
|
|
@ -1579,7 +1593,13 @@ export class AgentBrowserBridge {
|
|||
const createSession = async (): Promise<void> => {
|
||||
const wc = this.getWebContents(webContentsId)
|
||||
if (!wc) {
|
||||
throw new BrowserError('browser_no_tab', 'Tab is no longer available')
|
||||
// Why: the renderer can unregister/destroy a webview between target
|
||||
// resolution and session creation. Preserve the explicit page identity
|
||||
// so callers get the same error shape as a settled closed tab.
|
||||
throw new BrowserError(
|
||||
'browser_tab_not_found',
|
||||
`Browser page ${browserPageId} is no longer available`
|
||||
)
|
||||
}
|
||||
|
||||
// Why: agent-browser's daemon persists session state (including the CDP port)
|
||||
|
|
@ -1637,7 +1657,10 @@ export class AgentBrowserBridge {
|
|||
this.commandQueues.delete(sessionName)
|
||||
this.processingQueues.delete(sessionName)
|
||||
if (queue) {
|
||||
const err = new BrowserError('browser_error', 'Session destroyed while commands were queued')
|
||||
const err = new BrowserError(
|
||||
'browser_tab_closed',
|
||||
'Tab was closed while commands were queued'
|
||||
)
|
||||
for (const cmd of queue) {
|
||||
cmd.reject(err)
|
||||
}
|
||||
|
|
@ -1681,14 +1704,17 @@ export class AgentBrowserBridge {
|
|||
): Promise<unknown> {
|
||||
const session = this.sessions.get(sessionName)
|
||||
if (!session) {
|
||||
throw new BrowserError('browser_error', 'Session not found')
|
||||
// Why: queued commands can reach execution after a concurrent tab close
|
||||
// deletes the session. Surface this as a tab lifecycle error, not an
|
||||
// opaque internal bridge failure.
|
||||
throw this.createPageUnavailableError(sessionName)
|
||||
}
|
||||
|
||||
// Why: between enqueue time and execution time (queue delay), the webContents
|
||||
// could be destroyed. Check here to give a clear error instead of letting the
|
||||
// proxy fail with cryptic Electron debugger errors.
|
||||
if (!this.getWebContents(session.webContentsId)) {
|
||||
throw new BrowserError('browser_tab_closed', 'Tab was closed while command was queued')
|
||||
throw this.createPageUnavailableError(sessionName)
|
||||
}
|
||||
|
||||
const args = ['--session', sessionName]
|
||||
|
|
@ -1711,7 +1737,12 @@ export class AgentBrowserBridge {
|
|||
const translated = translateResult(stdout)
|
||||
|
||||
if (!translated.ok) {
|
||||
throw new BrowserError(translated.error.code, translated.error.message)
|
||||
throw this.createCommandError(
|
||||
sessionName,
|
||||
translated.error.message,
|
||||
translated.error.code,
|
||||
session.webContentsId
|
||||
)
|
||||
}
|
||||
|
||||
// Why: only mark initialized after a successful command — if the first --cdp
|
||||
|
|
@ -1748,6 +1779,37 @@ export class AgentBrowserBridge {
|
|||
return translated.result
|
||||
}
|
||||
|
||||
private createPageUnavailableError(sessionName: string): BrowserError {
|
||||
return new BrowserError('browser_tab_not_found', pageUnavailableMessageForSession(sessionName))
|
||||
}
|
||||
|
||||
private createCommandError(
|
||||
sessionName: string,
|
||||
message: string,
|
||||
fallbackCode: string,
|
||||
webContentsId?: number
|
||||
): BrowserError {
|
||||
// Why: CDP "connection refused" can also mean a real proxy failure. Only
|
||||
// convert it to a closed-page error when bridge state confirms the target is gone.
|
||||
if (
|
||||
fallbackCode === 'browser_error' &&
|
||||
isTabClosedTransportError(message) &&
|
||||
this.isSessionTargetClosed(sessionName, webContentsId)
|
||||
) {
|
||||
return this.createPageUnavailableError(sessionName)
|
||||
}
|
||||
return new BrowserError(fallbackCode, message)
|
||||
}
|
||||
|
||||
private isSessionTargetClosed(sessionName: string, webContentsId?: number): boolean {
|
||||
const session = this.sessions.get(sessionName)
|
||||
if (!session) {
|
||||
return true
|
||||
}
|
||||
const targetWebContentsId = webContentsId ?? session.webContentsId
|
||||
return !this.getWebContents(targetWebContentsId)
|
||||
}
|
||||
|
||||
private runAgentBrowserRaw(
|
||||
sessionName: string,
|
||||
args: string[],
|
||||
|
|
@ -1774,7 +1836,9 @@ export class AgentBrowserBridge {
|
|||
}
|
||||
if (child && this.cancelledProcesses.has(child)) {
|
||||
this.cancelledProcesses.delete(child)
|
||||
reject(new BrowserError('browser_error', 'Session destroyed while command was running'))
|
||||
reject(
|
||||
new BrowserError('browser_tab_closed', 'Tab was closed while command was running')
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -1808,14 +1872,19 @@ export class AgentBrowserBridge {
|
|||
try {
|
||||
const parsed = JSON.parse(stdout)
|
||||
if (parsed.error) {
|
||||
reject(new BrowserError(classifyErrorCode(parsed.error), parsed.error))
|
||||
const code = classifyErrorCode(parsed.error)
|
||||
reject(
|
||||
this.createCommandError(sessionName, parsed.error, code, session?.webContentsId)
|
||||
)
|
||||
return
|
||||
}
|
||||
} catch {
|
||||
// stdout not valid JSON — fall through to stderr/error.message
|
||||
}
|
||||
}
|
||||
reject(new BrowserError('browser_error', stderr || error.message))
|
||||
const message = stderr || error.message
|
||||
const code = classifyErrorCode(message)
|
||||
reject(this.createCommandError(sessionName, message, code, session?.webContentsId))
|
||||
return
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue