fix(browser): stop an over-limit screencast frame from killing the paired runtime socket (#12680)
Opening any webpage in the remote browser dropped the paired runtime connection, and the client then retried forever without recovering. Causal chain: the screencast travels host->client, a direction that admits up to 8 MiB. The host's encrypted channel rejects anything larger with close code 1013 "Outbound reply buffer overflow" — killing every subscription on that connection. The producer treats a false return as backpressure and retries the identical frame, which for an over-limit frame can never succeed. A permanent condition was being treated as transient. Two changes: 1. A paired-runtime admission wrapper: an over-limit frame is dropped rather than handed to the transport, and reported as handled so the producer advances instead of retrying something doomed. The generic Chromium producer is untouched, so local browser behavior is unchanged. 2. The actual source of over-limit frames. Live frames are hard-bounded by maxWidth/maxHeight, but the navigation snapshot path ignored those bounds entirely, feeding capturePage device pixels straight into the encoder — capturePage's rect is CSS pixels while the bitmap is device pixels, so at deviceScaleFactor 2 a snapshot could be 4x the pixel area the live path is allowed to send. That path fires on page load, which is literally the reported trigger. Applying the caller's own clamp there makes the drop a backstop rather than the mitigation. Dropping a frame is safe here because frames are complete standalone images, not deltas — each replaces the client image wholesale, so the next frame fully repaints. Disclosed in the PR: mobile web-view mode sends no viewport and takes the unclipped screenshot branch, where the drop guard remains the only protection; still strictly better than a 1013 that kills every subscription. Verified by reverting in place: neutralizing the admission guard fails 3 oracles, with the integration test emitting the real [1013, "Outbound reply buffer overflow"] from an actual E2EEChannel — the production symptom, not a mock. Neutralizing the snapshot clamp fails its own oracle, re-proven after the test was relocated. The second half of the report — never recovering without an app restart — is only partly addressed here and is now tracked as STA-3483: the browser stream restart arms a single 500ms retry and never reschedules, so any connection loss can strand the pane. Fixes STA-2970.
This commit is contained in:
parent
0f9caf52b1
commit
9accd97bd9
|
|
@ -0,0 +1,94 @@
|
|||
import { Buffer } from 'node:buffer'
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { startBrowserScreencast } from './browser-screencast-stream'
|
||||
|
||||
function createMockWebContents(capturePage: () => Promise<unknown>) {
|
||||
let attached = false
|
||||
const dbg = new EventEmitter() as EventEmitter & {
|
||||
isAttached: ReturnType<typeof vi.fn>
|
||||
attach: ReturnType<typeof vi.fn>
|
||||
detach: ReturnType<typeof vi.fn>
|
||||
sendCommand: ReturnType<typeof vi.fn>
|
||||
}
|
||||
dbg.isAttached = vi.fn(() => attached)
|
||||
dbg.attach = vi.fn(() => {
|
||||
attached = true
|
||||
})
|
||||
dbg.detach = vi.fn(() => {
|
||||
attached = false
|
||||
})
|
||||
dbg.sendCommand = vi.fn(async () => ({}))
|
||||
return { isDestroyed: vi.fn(() => false), debugger: dbg, capturePage: vi.fn(capturePage) }
|
||||
}
|
||||
|
||||
function createCapturedImage(width: number, height: number) {
|
||||
const resized = {
|
||||
getSize: vi.fn(() => ({ width, height })),
|
||||
resize: vi.fn(),
|
||||
toJPEG: vi.fn(() => Buffer.from('scaled-frame')),
|
||||
toPNG: vi.fn(() => Buffer.from('scaled-frame'))
|
||||
}
|
||||
const image = {
|
||||
getSize: vi.fn(() => ({ width, height })),
|
||||
resize: vi.fn(() => resized),
|
||||
toJPEG: vi.fn(() => Buffer.from('captured-frame')),
|
||||
toPNG: vi.fn(() => Buffer.from('captured-frame'))
|
||||
}
|
||||
return { image, resized }
|
||||
}
|
||||
|
||||
describe('browser screencast snapshot scaling', () => {
|
||||
it('scales a hi-DPI capture down to the requested frame bounds', async () => {
|
||||
const { image, resized } = createCapturedImage(4000, 3000)
|
||||
const webContents = createMockWebContents(async () => image)
|
||||
const onFrame = vi.fn()
|
||||
|
||||
const session = await startBrowserScreencast(webContents as never, {
|
||||
format: 'jpeg',
|
||||
quality: 70,
|
||||
maxWidth: 1440,
|
||||
maxHeight: 1200,
|
||||
viewportWidth: 2000,
|
||||
viewportHeight: 1500,
|
||||
deviceScaleFactor: 2,
|
||||
everyNthFrame: 2,
|
||||
minFrameIntervalMs: 0,
|
||||
onFrame
|
||||
})
|
||||
|
||||
await vi.waitFor(() => expect(onFrame).toHaveBeenCalledTimes(1))
|
||||
expect(image.resize).toHaveBeenCalledWith({ width: 1440, height: 1080 })
|
||||
expect(resized.toJPEG).toHaveBeenCalledWith(70)
|
||||
expect(image.toJPEG).not.toHaveBeenCalled()
|
||||
|
||||
session.stop()
|
||||
await session.done
|
||||
})
|
||||
|
||||
it('leaves a capture already within the frame bounds unscaled', async () => {
|
||||
const { image } = createCapturedImage(1200, 800)
|
||||
const webContents = createMockWebContents(async () => image)
|
||||
const onFrame = vi.fn()
|
||||
|
||||
const session = await startBrowserScreencast(webContents as never, {
|
||||
format: 'jpeg',
|
||||
quality: 70,
|
||||
maxWidth: 1440,
|
||||
maxHeight: 1200,
|
||||
viewportWidth: 1200,
|
||||
viewportHeight: 800,
|
||||
everyNthFrame: 2,
|
||||
minFrameIntervalMs: 0,
|
||||
onFrame
|
||||
})
|
||||
|
||||
await vi.waitFor(() => expect(onFrame).toHaveBeenCalledTimes(1))
|
||||
expect(image.resize).not.toHaveBeenCalled()
|
||||
expect(image.toJPEG).toHaveBeenCalledWith(70)
|
||||
|
||||
session.stop()
|
||||
await session.done
|
||||
})
|
||||
})
|
||||
|
|
@ -1,6 +1,6 @@
|
|||
/* eslint-disable max-lines -- Why: screencast setup, CDP lifecycle, metadata normalization, and stream teardown stay together so frame behavior cannot drift across files. */
|
||||
import { Buffer } from 'node:buffer'
|
||||
import type { WebContents } from 'electron'
|
||||
import type { NativeImage, WebContents } from 'electron'
|
||||
import {
|
||||
BrowserScreencastOpcode,
|
||||
encodeBrowserScreencastFrame,
|
||||
|
|
@ -86,6 +86,20 @@ function scaleToFit(
|
|||
}
|
||||
}
|
||||
|
||||
// Why: capturePage returns device pixels, so a hi-DPI viewport yields a bitmap several times
|
||||
// larger than the live path is allowed to send. Apply the caller's cap here too.
|
||||
function scaleSnapshotToFit(image: NativeImage, options: BrowserScreencastOptions): NativeImage {
|
||||
const size = image.getSize()
|
||||
if (!size.width || !size.height) {
|
||||
return image
|
||||
}
|
||||
const fitted = scaleToFit(size.width, size.height, options.maxWidth, options.maxHeight)
|
||||
if (fitted.width === size.width && fitted.height === size.height) {
|
||||
return image
|
||||
}
|
||||
return image.resize(fitted)
|
||||
}
|
||||
|
||||
function isNearSize(
|
||||
actual: { width: number; height: number },
|
||||
expected: { width: number; height: number }
|
||||
|
|
@ -518,8 +532,9 @@ export async function startBrowserScreencast(
|
|||
width: viewportWidth,
|
||||
height: viewportHeight
|
||||
})
|
||||
const capture = scaleSnapshotToFit(nativeImage, options)
|
||||
const buffer =
|
||||
options.format === 'png' ? nativeImage.toPNG() : nativeImage.toJPEG(options.quality)
|
||||
options.format === 'png' ? capture.toPNG() : capture.toJPEG(options.quality)
|
||||
if (buffer.byteLength > 0) {
|
||||
image = new Uint8Array(buffer)
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { AgentBrowserBridge } from '../browser/agent-browser-bridge'
|
||||
import { REMOTE_RUNTIME_MAX_OUTBOUND_BINARY_FRAME_BYTES } from '../../shared/remote-runtime-memory-limits'
|
||||
import type { RuntimeBrowserCommandHost } from './orca-runtime-browser'
|
||||
|
||||
const {
|
||||
|
|
@ -579,6 +580,32 @@ describe('RuntimeBrowserCommands browser screencast', () => {
|
|||
await second.session.done
|
||||
expect(secondStop).toHaveBeenCalledTimes(1)
|
||||
}, 10_000)
|
||||
|
||||
it('admits screencast frames through the paired-runtime size guard', async () => {
|
||||
const { RuntimeBrowserCommands } = await import('./orca-runtime-browser')
|
||||
webContentsFromIdMock.mockReturnValue({ isDestroyed: () => false })
|
||||
const done = deferred<void>()
|
||||
startBrowserScreencastMock.mockResolvedValue({
|
||||
stop: vi.fn(() => done.resolve()),
|
||||
done: done.promise
|
||||
})
|
||||
const sendBinary = vi.fn(() => true)
|
||||
|
||||
const commands = new RuntimeBrowserCommands(createHost())
|
||||
const started = await commands.browserScreencast(
|
||||
{ worktree: 'id:wt-1', page: 'page-1', format: 'jpeg' },
|
||||
{ sendBinary }
|
||||
)
|
||||
const { onFrame } = startBrowserScreencastMock.mock.calls[0][1]
|
||||
|
||||
expect(onFrame(new Uint8Array(REMOTE_RUNTIME_MAX_OUTBOUND_BINARY_FRAME_BYTES + 1))).toBe(true)
|
||||
expect(sendBinary).not.toHaveBeenCalled()
|
||||
expect(onFrame(new Uint8Array(64))).toBe(true)
|
||||
expect(sendBinary).toHaveBeenCalledTimes(1)
|
||||
|
||||
started.session.stop()
|
||||
await started.session.done
|
||||
})
|
||||
})
|
||||
|
||||
describe('RuntimeBrowserCommands headless offscreen routing', () => {
|
||||
|
|
|
|||
|
|
@ -68,6 +68,7 @@ import {
|
|||
selectBrowserProfile
|
||||
} from '../browser/browser-cookie-import'
|
||||
import { waitForTabRegistration, waitForWorktreeTabRegistration } from '../ipc/browser'
|
||||
import { sendRemoteBrowserScreencastFrame } from './remote-browser-screencast-frame-admission'
|
||||
|
||||
export type BrowserCommandTargetParams = {
|
||||
worktree?: string
|
||||
|
|
@ -507,7 +508,7 @@ export class RuntimeBrowserCommands {
|
|||
mobile: params.mobile === true,
|
||||
everyNthFrame: clampInteger(params.everyNthFrame, 1, 10, 2),
|
||||
minFrameIntervalMs: clampInteger(params.minFrameIntervalMs, 0, 1000, 0),
|
||||
onFrame: stream.sendBinary,
|
||||
onFrame: (bytes) => sendRemoteBrowserScreencastFrame(stream.sendBinary, bytes),
|
||||
onEvent: stream.emit,
|
||||
onError: (message) => stream.emit?.({ type: 'error', message })
|
||||
})
|
||||
|
|
|
|||
|
|
@ -0,0 +1,149 @@
|
|||
import { Buffer } from 'node:buffer'
|
||||
import { EventEmitter } from 'node:events'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { deriveSharedKey, encrypt, generateKeyPair } from '../../shared/e2ee-crypto'
|
||||
import { REMOTE_RUNTIME_MAX_OUTBOUND_BINARY_FRAME_BYTES } from '../../shared/remote-runtime-memory-limits'
|
||||
import { startBrowserScreencast } from '../browser/browser-screencast-stream'
|
||||
import { E2EEChannel } from './rpc/e2ee-channel'
|
||||
import { sendRemoteBrowserScreencastFrame } from './remote-browser-screencast-frame-admission'
|
||||
|
||||
function createMockWebContents() {
|
||||
let attached = false
|
||||
const dbg = new EventEmitter() as EventEmitter & {
|
||||
isAttached: ReturnType<typeof vi.fn>
|
||||
attach: ReturnType<typeof vi.fn>
|
||||
detach: ReturnType<typeof vi.fn>
|
||||
sendCommand: ReturnType<typeof vi.fn>
|
||||
}
|
||||
dbg.isAttached = vi.fn(() => attached)
|
||||
dbg.attach = vi.fn(() => {
|
||||
attached = true
|
||||
})
|
||||
dbg.detach = vi.fn(() => {
|
||||
attached = false
|
||||
})
|
||||
dbg.sendCommand = vi.fn(async () => ({}))
|
||||
return { isDestroyed: vi.fn(() => false), debugger: dbg }
|
||||
}
|
||||
|
||||
// Why: exercises the real E2EE channel so the oracle fails if an over-limit frame ever reaches
|
||||
// the transport that answers it with close code 1013.
|
||||
function createRuntimeBinarySender() {
|
||||
const serverKeys = generateKeyPair()
|
||||
const clientKeys = generateKeyPair()
|
||||
const ws = {
|
||||
OPEN: 1 as const,
|
||||
readyState: 1,
|
||||
send: vi.fn(),
|
||||
close: vi.fn()
|
||||
}
|
||||
const onTransportError = vi.fn((code: number, reason: string) => ws.close(code, reason))
|
||||
const channel = new E2EEChannel(ws as never, {
|
||||
serverSecretKey: serverKeys.secretKey,
|
||||
resolveAuthenticatedDevice: (token) =>
|
||||
token === 'valid-token'
|
||||
? { deviceId: 'device-1', deviceToken: token, scope: 'runtime' }
|
||||
: null,
|
||||
onReady: vi.fn(),
|
||||
onError: onTransportError
|
||||
})
|
||||
const sharedKey = deriveSharedKey(clientKeys.secretKey, serverKeys.publicKey)
|
||||
channel.handleRawMessage(
|
||||
JSON.stringify({
|
||||
type: 'e2ee_hello',
|
||||
publicKeyB64: Buffer.from(clientKeys.publicKey).toString('base64')
|
||||
})
|
||||
)
|
||||
channel.handleRawMessage(
|
||||
encrypt(JSON.stringify({ type: 'e2ee_auth', deviceToken: 'valid-token' }), sharedKey)
|
||||
)
|
||||
let sendBinary: ((bytes: Uint8Array<ArrayBufferLike>) => boolean | void) | undefined
|
||||
channel.onMessage((_plaintext, _sendText, sendBinaryReply) => {
|
||||
sendBinary = sendBinaryReply
|
||||
})
|
||||
channel.handleRawMessage(encrypt('start-screencast', sharedKey))
|
||||
if (!sendBinary) {
|
||||
throw new Error('Runtime binary sender was not established')
|
||||
}
|
||||
return { channel, ws, onTransportError, sendBinary }
|
||||
}
|
||||
|
||||
describe('sendRemoteBrowserScreencastFrame', () => {
|
||||
it('reports an over-limit frame as handled so the producer does not retry it', () => {
|
||||
const sendBinary = vi.fn(() => true)
|
||||
const oversized = new Uint8Array(REMOTE_RUNTIME_MAX_OUTBOUND_BINARY_FRAME_BYTES + 1)
|
||||
|
||||
expect(sendRemoteBrowserScreencastFrame(sendBinary, oversized)).toBe(true)
|
||||
expect(sendBinary).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still forwards backpressure for a frame the transport can accept', () => {
|
||||
const withinLimit = new Uint8Array(1024)
|
||||
|
||||
expect(
|
||||
sendRemoteBrowserScreencastFrame(
|
||||
vi.fn(() => false),
|
||||
withinLimit
|
||||
)
|
||||
).toBe(false)
|
||||
expect(
|
||||
sendRemoteBrowserScreencastFrame(
|
||||
vi.fn(() => true),
|
||||
withinLimit
|
||||
)
|
||||
).toBe(true)
|
||||
expect(sendRemoteBrowserScreencastFrame(vi.fn(), withinLimit)).toBe(true)
|
||||
})
|
||||
|
||||
it('drops an oversized encoded frame without closing or stalling the paired stream', async () => {
|
||||
vi.useFakeTimers()
|
||||
const webContents = createMockWebContents()
|
||||
const transport = createRuntimeBinarySender()
|
||||
const session = await startBrowserScreencast(webContents as never, {
|
||||
format: 'jpeg',
|
||||
quality: 70,
|
||||
maxWidth: 3840,
|
||||
maxHeight: 2160,
|
||||
everyNthFrame: 1,
|
||||
minFrameIntervalMs: 0,
|
||||
onFrame: (bytes) => sendRemoteBrowserScreencastFrame(transport.sendBinary, bytes)
|
||||
})
|
||||
|
||||
try {
|
||||
webContents.debugger.emit('message', {}, 'Page.screencastFrame', {
|
||||
data: Buffer.alloc(REMOTE_RUNTIME_MAX_OUTBOUND_BINARY_FRAME_BYTES + 1).toString('base64'),
|
||||
sessionId: 42,
|
||||
metadata: {}
|
||||
})
|
||||
await Promise.resolve()
|
||||
|
||||
expect(transport.onTransportError).not.toHaveBeenCalled()
|
||||
expect(transport.ws.close).not.toHaveBeenCalled()
|
||||
expect(webContents.debugger.sendCommand).toHaveBeenCalledWith('Page.screencastFrameAck', {
|
||||
sessionId: 42
|
||||
})
|
||||
const sendsAfterDrop = transport.ws.send.mock.calls.length
|
||||
await vi.advanceTimersByTimeAsync(500)
|
||||
expect(transport.ws.send).toHaveBeenCalledTimes(sendsAfterDrop)
|
||||
|
||||
webContents.debugger.emit('message', {}, 'Page.screencastFrame', {
|
||||
data: Buffer.from('next-frame').toString('base64'),
|
||||
sessionId: 43,
|
||||
metadata: {}
|
||||
})
|
||||
await Promise.resolve()
|
||||
|
||||
expect(transport.onTransportError).not.toHaveBeenCalled()
|
||||
expect(webContents.debugger.sendCommand).toHaveBeenCalledWith('Page.screencastFrameAck', {
|
||||
sessionId: 43
|
||||
})
|
||||
expect(transport.ws.send.mock.calls.length).toBeGreaterThan(sendsAfterDrop)
|
||||
} finally {
|
||||
session.stop()
|
||||
await session.done
|
||||
transport.channel.destroy()
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
@ -0,0 +1,14 @@
|
|||
import { isRemoteRuntimeBinaryFrameWithinLimit } from '../../shared/remote-runtime-memory-limits'
|
||||
|
||||
// Why: the E2EE channel closes the socket (1013) on an over-limit binary frame, and the
|
||||
// screencast producer reads `false` as backpressure and retries the identical frame. Reporting
|
||||
// an over-limit frame as handled drops it so the stream advances instead of retrying forever.
|
||||
export function sendRemoteBrowserScreencastFrame(
|
||||
sendBinary: (bytes: Uint8Array<ArrayBufferLike>) => boolean | void,
|
||||
bytes: Uint8Array<ArrayBufferLike>
|
||||
): boolean {
|
||||
if (!isRemoteRuntimeBinaryFrameWithinLimit(bytes)) {
|
||||
return true
|
||||
}
|
||||
return sendBinary(bytes) !== false
|
||||
}
|
||||
Loading…
Reference in New Issue