diff --git a/src/renderer/src/components/terminal-pane/TerminalPane.tsx b/src/renderer/src/components/terminal-pane/TerminalPane.tsx index ee5ec73798e..7dfe737e34a 100644 --- a/src/renderer/src/components/terminal-pane/TerminalPane.tsx +++ b/src/renderer/src/components/terminal-pane/TerminalPane.tsx @@ -204,6 +204,7 @@ import { type TerminalPasteSource, type TerminalPasteTextOptions } from './terminal-paste-coordinator' +import { appendTerminalErrorMessage } from './terminal-error-accumulation' import { formatTerminalPasteExecutionError } from './terminal-paste-errors' import { resolveTerminalPasteRuntime } from './terminal-paste-runtime' import { getTerminalPasteSshRemotePlatform } from './terminal-paste-ssh-platform' @@ -468,8 +469,15 @@ function TerminalPane( setSessionStateSaveFailureOpen(true) return } - setTerminalError((prev) => (prev ? `${prev}\n${message}` : message)) + setTerminalError((prev) => appendTerminalErrorMessage(prev, message)) }) + /** Dismissal is the only signal that the user has seen the surface, so it must also release the transports' repeat-suppression memory. */ + const dismissTerminalError = useCallback(() => { + setTerminalError(null) + for (const transport of paneTransportsRef.current.values()) { + transport.notifyErrorSurfaceDismissed?.() + } + }, []) const onPtyRecoveryStateRef = useRef( (paneId: number, state: PtyTransportRecoveryState | null) => { setPtyRecoveryStatesByPaneId((previous) => @@ -2893,7 +2901,7 @@ function TerminalPane( {terminalError && isActive && !showSshReconnectOverlay ? ( setTerminalError(null)} + onDismiss={dismissTerminalError} onRestartDaemon={() => daemonActions.setPending('restart')} /> ) : null} diff --git a/src/renderer/src/components/terminal-pane/pty-transport-types.ts b/src/renderer/src/components/terminal-pane/pty-transport-types.ts index b0dd959416d..c64b9393490 100644 --- a/src/renderer/src/components/terminal-pane/pty-transport-types.ts +++ b/src/renderer/src/components/terminal-pane/pty-transport-types.ts @@ -161,6 +161,8 @@ export type PtyTransport = { getRecoveryState?: () => PtyTransportRecoveryState /** Starts a fresh connection epoch while preserving the authoritative remote PTY identity. */ retryRecovery?: () => boolean + /** The user dismissed the error surface; the next occurrence of the same message must surface again. */ + notifyErrorSurfaceDismissed?: () => void getPtyId: () => string | null getConnectionId?: () => string | null | undefined /** The runtime captured by this transport; legacy remote PTY ids do not diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-error-surface-dismissal.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-error-surface-dismissal.test.ts new file mode 100644 index 00000000000..1df9a355a05 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-error-surface-dismissal.test.ts @@ -0,0 +1,78 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +// Why: the transport suppresses a repeat of the message it last surfaced so one outage cannot spam the +// pane. Dismissal is the only evidence the user consumed that surface, so it must re-arm the memory — +// otherwise an identical fatal error recurring during the same outage leaves a dead pane looking fine. +describe('remote runtime error surface dismissal', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const subscriptionSendBinary = vi.fn() + const FATAL_ERROR = 'Remote terminal rejected the write: permission denied.' + + beforeEach(() => { + vi.resetModules() + vi.clearAllMocks() + subscriptionSendBinary.mockReset() + runtimeCall.mockImplementation(async () => ({ ok: true, result: {} })) + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: { onResponse: (response: unknown) => void }) => { + queueMicrotask(() => callbacks.onResponse({ ok: true, result: { type: 'ready' } })) + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall, + subscribe: runtimeSubscribe + } + } + }) + }) + + async function attachTransportWithFatalSends(onError: (message: string) => void) { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { worktreeId: 'wt-1' }) + transport.attach({ existingPtyId: 'remote:terminal-1', callbacks: { onError } }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'terminal.send') { + throw new Error(FATAL_ERROR) + } + return { ok: true, result: {} } + }) + return transport + } + + it('re-surfaces an identical fatal error after the user dismisses the error surface', async () => { + const onError = vi.fn() + const transport = await attachTransportWithFatalSends(onError) + + expect(await transport.sendInputAccepted?.('a')).toBe(false) + expect(await transport.sendInputAccepted?.('b')).toBe(false) + // Contract preserved: one continuous outage still surfaces the repeated error once. + expect(onError.mock.calls).toEqual([[FATAL_ERROR]]) + + transport.notifyErrorSurfaceDismissed?.() + + expect(await transport.sendInputAccepted?.('c')).toBe(false) + expect(onError.mock.calls).toEqual([[FATAL_ERROR], [FATAL_ERROR]]) + + // The memory re-arms: repeats after the re-surfaced error are suppressed again until the next dismissal. + expect(await transport.sendInputAccepted?.('d')).toBe(false) + expect(onError).toHaveBeenCalledTimes(2) + transport.destroy?.() + }) + + it('leaves an undismissed surface deduped', async () => { + const onError = vi.fn() + const transport = await attachTransportWithFatalSends(onError) + + expect(await transport.sendInputAccepted?.('a')).toBe(false) + expect(await transport.sendInputAccepted?.('b')).toBe(false) + expect(await transport.sendInputAccepted?.('c')).toBe(false) + + expect(onError).toHaveBeenCalledTimes(1) + transport.destroy?.() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-outage-toast-flood-and-stuck-reconnect.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-outage-toast-flood-and-stuck-reconnect.test.ts new file mode 100644 index 00000000000..8cd8600dac2 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-outage-toast-flood-and-stuck-reconnect.test.ts @@ -0,0 +1,438 @@ +/** + * Deterministic reproduction for issue3-error-flood-stuck-terminal + * (remote-runtime outage floods the UI with repeated raw error toasts; + * terminal panes stay stuck after connectivity is restored). + * + * Three tests assert the adjudicated fix; the STA-3002 case covers reconnect + * activation, which PR #11542 landed separately. + * + * Fault-injection point: window.api.runtimeEnvironments.call — the exact + * Electron IPC boundary the remote PTY transport uses. Rejections are shaped + * like real Electron IPC rejections: a plain Error whose message carries the + * "Error invoking remote method 'runtimeEnvironments:call': : " + * prefix and NO `code` property (Electron structured-clone keeps only + * name/message/stack), which forces the renderer's message-fragment + * classification path. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + encodeTerminalStreamFrame, + encodeTerminalStreamJson, + encodeTerminalStreamText +} from '../../../../shared/terminal-stream-protocol' +import { RuntimeRpcCallQueueOverloadError } from '../../../../shared/runtime-rpc-call-queue' +import { withRemoteRuntimeTailscaleHint } from '../../../../shared/remote-runtime-tailscale-hint' +import type { PtyTransportRecoveryState } from './pty-transport-types' + +const ELECTRON_IPC_PREFIX = "Error invoking remote method 'runtimeEnvironments:call': " + +/** A rejection exactly as the renderer sees it after Electron IPC strips custom props. */ +function electronIpcShapedRejection(errorName: string, message: string): Error { + return new Error(`${ELECTRON_IPC_PREFIX}${errorName}: ${message}`) +} + +const QUEUE_OVERLOAD_RAW = new RuntimeRpcCallQueueOverloadError('selector').message + +// The exact user-reported toast text family (timeout + Tailscale funnel hint). +const TIMEOUT_WITH_TAILSCALE_HINT = withRemoteRuntimeTailscaleHint( + 'Timed out waiting for the remote Orca runtime to respond.', + 'https://orca-server.tail1234.ts.net' +) + +describe('remote runtime outage: toast flood and stuck reconnect (issue3)', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const refreshSessionTabsSnapshot = vi.fn(async () => {}) + const subscriptionSendBinary = vi.fn() + let subscriptionCallbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { code: string; message: string }) => void + onClose?: () => void + } | null = null + + function emitMultiplexReady(): void { + subscriptionCallbacks?.onResponse({ ok: true, result: { type: 'ready' } }) + } + + function latestSubscribePayload(): { streamId: number; terminal: string } { + const frames = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .filter((frame) => frame?.opcode === TerminalStreamOpcode.Subscribe) + const frame = frames.at(-1) + if (!frame) { + throw new Error('missing terminal subscribe frame') + } + const payload = decodeTerminalStreamJson<{ streamId: number; terminal: string }>(frame.payload) + if (!payload) { + throw new Error('invalid terminal subscribe payload') + } + return payload + } + + function subscribedTerminalHandles(): string[] { + return subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .flatMap((frame) => { + if (frame?.opcode !== TerminalStreamOpcode.Subscribe) { + return [] + } + const payload = decodeTerminalStreamJson<{ terminal: string }>(frame.payload) + return payload ? [payload.terminal] : [] + }) + } + + function emitSnapshot(streamId: number, data: string): void { + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotStart, + streamId, + seq: 1, + payload: encodeTerminalStreamJson({ kind: 'scrollback' }) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotChunk, + streamId, + seq: 2, + payload: encodeTerminalStreamText(data) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotEnd, + streamId, + seq: 3, + payload: new Uint8Array() + }) + ) + } + + function installHealthyRuntimeCallMock(): void { + runtimeCall.mockImplementation(async (request: { method: string; params?: unknown }) => { + if (request.method === 'session.tabs.activate') { + const params = request.params as { tabId: string; leafId?: string } + const resolvedLeafId = params.leafId ?? 'pane:1' + return { + ok: true, + result: { + worktree: 'id:wt-1', + publicationEpoch: 'epoch-1', + snapshotVersion: 1, + activeGroupId: 'group-1', + activeTabId: `${params.tabId}::${resolvedLeafId}`, + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: `${params.tabId}::${resolvedLeafId}`, + parentTabId: params.tabId, + leafId: resolvedLeafId, + title: 'Terminal', + isActive: true, + status: 'ready', + terminal: 'terminal-1' + } + ] + } + } + } + if (request.method === 'terminal.resolvePane') { + const params = request.params as { paneKey: string; worktreeId: string } + const separator = params.paneKey.indexOf(':') + return { + ok: true, + result: { + terminal: { + handle: 'terminal-1', + tabId: params.paneKey.slice(0, separator), + leafId: params.paneKey.slice(separator + 1), + worktreeId: params.worktreeId + } + } + } + } + return { ok: true, result: { terminal: { handle: 'terminal-1' } } } + }) + } + + beforeEach(() => { + vi.resetModules() + vi.doUnmock('../../runtime/remote-runtime-terminal-multiplexer') + vi.doMock('@/runtime/web-runtime-session', () => ({ + refreshWebRuntimeSessionTabsSnapshot: refreshSessionTabsSnapshot + })) + vi.clearAllMocks() + subscriptionCallbacks = null + subscriptionSendBinary.mockReset() + installHealthyRuntimeCallMock() + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + subscriptionCallbacks = callbacks + queueMicrotask(emitMultiplexReady) + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall, + subscribe: runtimeSubscribe + } + } + }) + }) + + it('sanity: the exact reported timeout+funnel toast text classifies as recoverable, so flood text must come from unclassified surfaces', async () => { + const { isRecoverableRemoteRuntimeConnectionError, toRemoteRuntimeClientErrorLike } = + await import('../../../../shared/remote-runtime-client-error-classification') + const rendererSide = toRemoteRuntimeClientErrorLike( + electronIpcShapedRejection('RemoteRuntimeClientError', TIMEOUT_WITH_TAILSCALE_HINT) + ) + // Electron IPC stripped the code; the fragment list still catches this one. + expect(rendererSide.code).toBeUndefined() + expect(isRecoverableRemoteRuntimeConnectionError(rendererSide)).toBe(true) + // …but the queue-overload rejection produced by the same outage (main's + // per-selector RPC queue saturated by 15s-timeout calls) is classified + // fatal even though its own code says "retry later". + const overload = toRemoteRuntimeClientErrorLike( + electronIpcShapedRejection('RuntimeRpcCallQueueOverloadError', QUEUE_OVERLOAD_RAW) + ) + expect(overload.code).toBeUndefined() + // DESIRED: transient capacity pressure during an outage is recoverable, + // not a fatal red-toast error. RED on main: not in codes or fragments. + expect(isRecoverableRemoteRuntimeConnectionError(overload)).toBe(true) + }) + + it('FLOOD: repeated identical outage-shaped send failures surface at most one red toast per pane', async () => { + // Outage onset: the multiplexed stream subscription round-trip hangs + // (socket black-holed), so keystrokes fall back to one-shot terminal.send + // RPCs; main's saturated per-selector queue rejects each one instantly. + runtimeSubscribe.mockImplementation(async () => new Promise(() => {})) + let sendRejections = 0 + installHealthyRuntimeCallMock() + const healthyImpl = runtimeCall.getMockImplementation()! + runtimeCall.mockImplementation(async (request: { method: string; params?: unknown }) => { + if (request.method === 'terminal.send') { + sendRejections += 1 + throw electronIpcShapedRejection('RuntimeRpcCallQueueOverloadError', QUEUE_OVERLOAD_RAW) + } + return healthyImpl(request) + }) + + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const onError = vi.fn() + const recoveryPhases: string[] = [] + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: { + onError, + onRecoveryStateChange: (state: PtyTransportRecoveryState) => + recoveryPhases.push(state.phase) + } + }) + await vi.waitFor(() => expect(transport.getPtyId()).toBe('remote:env-1@@terminal-1')) + + // Three keystroke bursts during the same outage. + expect(transport.sendInputImmediate?.('k1')).toBe(true) + await vi.waitFor(() => expect(sendRejections).toBe(1)) + expect(transport.sendInputImmediate?.('k2')).toBe(true) + await vi.waitFor(() => expect(sendRejections).toBe(2)) + expect(transport.sendInputImmediate?.('k3')).toBe(true) + await vi.waitFor(() => expect(sendRejections).toBe(3)) + await new Promise((resolve) => setTimeout(resolve, 0)) + + expect(onError).not.toHaveBeenCalled() + expect(recoveryPhases).toContain('backoff') + transport.destroy?.() + }) + + it('STUCK (cancel dead-end): a fatal resubscribe error leaves no recovery path after connectivity returns', async () => { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { retryAllRemoteRuntimePtyRecoveriesNow } = + await import('./remote-runtime-pty-recovery-state') + const { updateTerminalRemoteRuntimeRecoveryUiState } = + await import('./terminal-remote-runtime-recovery-ui-state') + const onError = vi.fn() + let bannerUiState: Parameters[0] = {} + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'tab-1', + leafId: 'pane:1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: { + onError, + onRecoveryStateChange: (state: PtyTransportRecoveryState) => { + bannerUiState = updateTerminalRemoteRuntimeRecoveryUiState(bannerUiState, 1, state) + } + } + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + emitSnapshot(latestSubscribePayload().streamId, 'live before outage') + expect(transport.isConnected()).toBe(true) + + // The dedicated stream dies, then a fatal retry response must leave the + // terminal disconnected but manually revivable. + const fatalMessage = 'Remote runtime pairing credentials expired.' + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'terminal.resolvePane') { + throw Object.assign(new Error(fatalMessage), { code: 'unauthorized' }) + } + throw electronIpcShapedRejection('RemoteRuntimeClientError', TIMEOUT_WITH_TAILSCALE_HINT) + }) + subscriptionCallbacks?.onClose?.() + await vi.waitFor(() => expect(onError).toHaveBeenCalled()) + expect(onError).toHaveBeenCalledTimes(1) + expect(onError).toHaveBeenCalledWith(fatalMessage) + + // Connectivity fully restored. + installHealthyRuntimeCallMock() + const subscribeCallsBeforeTriggers = runtimeSubscribe.mock.calls.length + // Fire every built-in revival trigger the app has: + const revivedByOnlineOrResume = retryAllRemoteRuntimePtyRecoveriesNow() + const manualRetryAccepted = transport.retryRecovery?.() ?? false + const reconnectBannerVisible = 1 in bannerUiState + + // DESIRED INVARIANT (RED on main): after the fault clears, at least one + // recovery affordance must exist — the online/resume trigger revives a + // parked retry, or the Reconnect banner is visible and its retry is + // accepted. On main: cancel() latched phase 'idle', pendingRetry is gone, + // the banner is unmounted, and retryRecovery() returns false — keystrokes + // silently vanish and no output ever renders again. + expect( + { + revivedByOnlineOrResume, + manualRetryAccepted, + reconnectBannerVisible + }, + 'pane must remain revivable after a fatal resubscribe error' + ).not.toEqual({ + revivedByOnlineOrResume: 0, + manualRetryAccepted: false, + reconnectBannerVisible: false + }) + + // Full recovery: a fresh subscribe attempt must reach the runtime. + await vi.waitFor(() => + expect(runtimeSubscribe.mock.calls.length).toBeGreaterThan(subscribeCallsBeforeTriggers) + ) + transport.destroy?.() + }) + + // PR #11542 owns reconnect activation for STA-3002; it has landed, so this must stay green. + it('STUCK (STA-3002 shape): reconnect never re-materializes a host surface demoted to pending-handle, even via online trigger and Reconnect', async () => { + vi.useFakeTimers() + try { + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const { retryAllRemoteRuntimePtyRecoveriesNow } = + await import('./remote-runtime-pty-recovery-state') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: 'wt-1', + tabId: 'web-terminal-host-tab-1', + leafId: 'leaf-1' + }) + transport.attach({ + existingPtyId: 'remote:env-1@@terminal-1', + cols: 80, + rows: 24, + callbacks: {} + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + emitSnapshot(latestSubscribePayload().streamId, 'live before host restart') + expect(transport.isConnected()).toBe(true) + + // Host restarted during the outage: it republishes this pane as + // status 'pending-handle' with no terminal. A real host only mints the + // PTY handle when someone calls session.tabs.activate. + let hostActivated = false + let activateCallsAfterOutage = 0 + let listCallsAfterOutage = 0 + const hostSnapshot = () => ({ + ok: true, + result: { + worktree: 'wt-1', + publicationEpoch: 'epoch-2', + snapshotVersion: 2 + listCallsAfterOutage, + activeGroupId: null, + activeTabId: 'host-tab-1::leaf-1', + activeTabType: 'terminal', + tabs: [ + { + type: 'terminal', + id: 'host-tab-1::leaf-1', + parentTabId: 'host-tab-1', + leafId: 'leaf-1', + title: 'Terminal', + isActive: true, + ...(hostActivated + ? { status: 'ready', terminal: 'terminal-2' } + : { status: 'pending-handle', terminal: null }) + } + ] + } + }) + runtimeCall.mockImplementation(async (request: { method: string }) => { + if (request.method === 'session.tabs.list') { + listCallsAfterOutage += 1 + return hostSnapshot() + } + if (request.method === 'session.tabs.activate') { + activateCallsAfterOutage += 1 + hostActivated = true + return hostSnapshot() + } + return { ok: true, result: {} } + }) + + // Stream lost → reconnect. The resubscribe path looks for a status:'ready' + // handle and finds only the pending surface. + subscriptionCallbacks?.onClose?.() + await vi.advanceTimersByTimeAsync(16_000) + + // Auto-recovery deadline latches the pane 'disconnected'. + await vi.advanceTimersByTimeAsync(60_000) + expect(transport.getRecoveryState?.().phase).toBe('disconnected') + + // Connectivity restored; 'online'/system-resume trigger fires. + retryAllRemoteRuntimePtyRecoveriesNow() + await vi.advanceTimersByTimeAsync(16_000) + + // Latch again, then the user clicks the Reconnect banner. + await vi.advanceTimersByTimeAsync(60_000) + transport.retryRecovery?.() + await vi.advanceTimersByTimeAsync(16_000) + + // A reconnect against a host that publishes this pane as pending-handle + // must call session.tabs.activate (the only RPC that materializes the PTY) + // and attach to the minted handle. Before #11542 only the initial-connect + // path activated, so every reconnect polled session.tabs.list forever and + // the pane stayed stuck until the user resumed the session in a new one. + expect( + activateCallsAfterOutage, + `reconnect ran ${listCallsAfterOutage} list-only inventory polls across online trigger + Reconnect click without ever activating the pending surface` + ).toBeGreaterThan(0) + await vi.waitFor(() => expect(subscribedTerminalHandles()).toContain('terminal-2')) + emitSnapshot(latestSubscribePayload().streamId, 'rematerialized') + expect(transport.isConnected()).toBe(true) + expect(transport.getPtyId()).toBe('remote:env-1@@terminal-2') + transport.destroy?.() + } finally { + vi.useRealTimers() + } + }) +}) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts index 6126c039e30..198c2481836 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.test.ts @@ -535,7 +535,8 @@ describe('createRemoteRuntimePtyTransport', () => { expect( runtimeCall.mock.calls.filter(([args]) => args.method === 'terminal.create') ).toHaveLength(1) - expect(onError).toHaveBeenCalledTimes(1) + expect(onError).not.toHaveBeenCalled() + expect(transport.getRecoveryState?.().phase).toBe('disconnected') transport.destroy?.() }) diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts index 42f7df89467..a74fed2876d 100644 --- a/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts +++ b/src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts @@ -2,6 +2,7 @@ import type { RuntimeRpcResponse } from '../../../../shared/runtime-rpc-envelope' import { isRecoverableRemoteRuntimeConnectionError, + isRuntimeRpcQueueOverloadError, toRemoteRuntimeClientErrorLike } from '../../../../shared/remote-runtime-client-error-classification' import type { @@ -181,6 +182,7 @@ export function createRemoteRuntimePtyTransport( let desiredOutputPaused = false let desiredViewport: { cols: number; rows: number } | null = null let storedCallbacks: Parameters[0]['callbacks'] = {} + let lastSurfacedErrorMessage: string | null = null let resubscribeEpoch: number | null = null let resubscribeRequestedHandle: string | null = null let resubscribeRequestedReplacementPolicy: HostHandleReplacementPolicy = 'reuse' @@ -435,6 +437,19 @@ export function createRemoteRuntimePtyTransport( storedCallbacks.onRecoveryStateChange?.(state) } + function surfaceErrorMessage(message: string): void { + if (message === lastSurfacedErrorMessage) { + return + } + lastSurfacedErrorMessage = message + storedCallbacks.onError?.(message) + } + + function markRecoveryHealthy(): void { + lastSurfacedErrorMessage = null + recovery.markHealthy() + } + function hostSnapshotOwnsLaunch( result: RemoteAgentSessionLaunchResult, environmentId: string @@ -761,12 +776,12 @@ export function createRemoteRuntimePtyTransport( return undefined } if (hostHandle === null) { - storedCallbacks.onError?.('Remote terminal was closed.') + surfaceErrorMessage('Remote terminal was closed.') return undefined } if (!hostHandle || !isCurrent()) { if (isCurrent()) { - storedCallbacks.onError?.('Remote terminal was closed.') + surfaceErrorMessage('Remote terminal was closed.') } return undefined } @@ -1144,7 +1159,7 @@ export function createRemoteRuntimePtyTransport( }) .catch((error) => { if (!destroyed && handle === expiredHandle) { - storedCallbacks.onError?.(runtimeTerminalErrorMessage(error)) + surfaceErrorMessage(runtimeTerminalErrorMessage(error)) } }) .finally(() => { @@ -1438,14 +1453,19 @@ export function createRemoteRuntimePtyTransport( recoverExpiredHostPane() return } - if (isRecoverableRemoteRuntimeConnectionError(toRemoteRuntimeClientErrorLike(error))) { + const clientError = toRemoteRuntimeClientErrorLike(error) + if (isRuntimeRpcQueueOverloadError(clientError)) { + scheduleCapacityPressureRetry() + return + } + if (isRecoverableRemoteRuntimeConnectionError(clientError)) { // Why: a partition is attachment state, not a terminal failure; keep the red error surface for actionable fatal errors. scheduleResubscribeAfterTransportClose() return } connecting = false emitRecoveryState() - storedCallbacks.onError?.(message) + surfaceErrorMessage(message) } function recoverAfterSubscribeFailure( @@ -1627,7 +1647,9 @@ export function createRemoteRuntimePtyTransport( scheduleResubscribeAfterTransportClose(currentReplacementPolicy, nextEpoch) }) } else { - recovery.cancel() + recovery.markDisconnected() + // Why: stale/gone/SSH-expired handling lives in handleRemoteTerminalError; its + // fallthrough surfaces the message, so routing here keeps those recoveries alive. handleRemoteTerminalError(error) } } @@ -1743,7 +1765,7 @@ export function createRemoteRuntimePtyTransport( setAttachmentReady(true) connecting = false resetRecoveryReplacementPolicy() - recovery.markHealthy() + markRecoveryHealthy() emitRecoveryState() storedCallbacks.onConnect?.() storedCallbacks.onStatus?.('shell') @@ -1844,7 +1866,7 @@ export function createRemoteRuntimePtyTransport( setAttachmentReady(subscriptionAttached) if (subscriptionAttached) { resetRecoveryReplacementPolicy() - recovery.markHealthy() + markRecoveryHealthy() } // Why: a viewport change during the subscribe round-trip hit the no-op one-shot fallback; replay the latest viewport so the PTY isn't stuck at subscribe-time size. if (pendingViewportClaim && desiredViewport) { @@ -1875,6 +1897,7 @@ export function createRemoteRuntimePtyTransport( const createEnvironmentId = currentRuntimeEnvironmentId lastConnectOptions = options lastAttachOptions = null + lastSurfacedErrorMessage = null storedCallbacks = options.callbacks resetRecoveryReplacementPolicy() resetSameHandleEndReuse() @@ -2082,13 +2105,18 @@ export function createRemoteRuntimePtyTransport( } catch (error) { if (!destroyed && lifecycleEpoch === connectLifecycleEpoch) { connecting = false - recovery.cancel() const message = runtimeTerminalErrorMessage(error) if (isRemoteTerminalGoneMessage(message)) { + recovery.cancel() handleRemoteTerminalError(error) + } else if ( + isRecoverableRemoteRuntimeConnectionError(toRemoteRuntimeClientErrorLike(error)) + ) { + recovery.markDisconnected() } else { + recovery.cancel() emitRecoveryState() - storedCallbacks.onError?.(message) + surfaceErrorMessage(message) } } return undefined @@ -2104,6 +2132,7 @@ export function createRemoteRuntimePtyTransport( resetSameHandleEndReuse() clearPublishedHandleWait() lastAttachOptions = options + lastSurfacedErrorMessage = null storedCallbacks = options.callbacks terminalEnded = false connecting = true @@ -2128,7 +2157,7 @@ export function createRemoteRuntimePtyTransport( handle = null connecting = false emitRecoveryState() - storedCallbacks.onError?.('Remote runtime terminal id is invalid.') + surfaceErrorMessage('Remote runtime terminal id is invalid.') return } const persistedHandle = nextHandle @@ -2176,7 +2205,7 @@ export function createRemoteRuntimePtyTransport( return } if (!resolved) { - storedCallbacks.onError?.('Remote terminal was closed.') + surfaceErrorMessage('Remote terminal was closed.') return } await adoptResolvedHostPane(resolved, options, false, generation) @@ -2352,6 +2381,11 @@ export function createRemoteRuntimePtyTransport( getRecoveryState, + // Why: dedup exists to stop one outage spamming the surface; once the user dismisses it, the next occurrence is new information again. + notifyErrorSurfaceDismissed() { + lastSurfacedErrorMessage = null + }, + retryRecovery() { if ( !destroyed && diff --git a/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts b/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts new file mode 100644 index 00000000000..7353e491ef1 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/remote-runtime-resubscribe-failure-recovery-routing.test.ts @@ -0,0 +1,354 @@ +/** + * Pins the routing of a NON-RECOVERABLE resubscribe failure in + * remote-runtime-pty-transport. + * + * `scheduleResubscribeAfterTransportClose` catches the rejection of + * `resubscribeAfterTransportClose`. When the error is not a recoverable + * connection error it must hand the error to `handleRemoteTerminalError`. + * Surfacing the message directly instead looks harmless — the pane turns red + * either way — but it skips four lifecycle routes, because none of these + * messages appear in RECOVERABLE_MESSAGE_FRAGMENTS and so all of them land in + * exactly this branch: + * + * terminal_handle_stale -> require-replacement resubscribe (or retire) + * terminal_gone / terminal_exited -> retire the pane + * SSH_SESSION_EXPIRED -> terminal.recoverPane on the hub + * snapshot-too-large -> informational, no red error at all + * + * Fault-injection point: window.api.runtimeEnvironments.call, the Electron IPC + * boundary the transport really uses. Each pane is first driven to a genuinely + * connected state, then the multiplexed stream is closed so recovery starts + * with a live epoch, and only the resubscribe attempt fails. + */ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { + TerminalStreamOpcode, + decodeTerminalStreamFrame, + decodeTerminalStreamJson, + encodeTerminalStreamFrame, + encodeTerminalStreamJson, + encodeTerminalStreamText +} from '../../../../shared/terminal-stream-protocol' +import type { PtyTransport } from './pty-transport-types' + +type ResolvePaneOutcome = { handle: string } | { error: Error } + +const PANE_TAB_ID = 'tab-1' +const PANE_LEAF_ID = 'pane:1' +const PANE_WORKTREE_ID = 'wt-1' +const FIRST_HANDLE = 'terminal-1' +const FIRST_PTY_ID = 'remote:env-1@@terminal-1' + +describe('remote runtime resubscribe failure: recovery routing', () => { + const runtimeCall = vi.fn() + const runtimeSubscribe = vi.fn() + const refreshSessionTabsSnapshot = vi.fn(async () => {}) + const subscriptionSendBinary = vi.fn() + let subscriptionCallbacks: { + onResponse: (response: unknown) => void + onBinary?: (bytes: Uint8Array) => void + onError?: (error: { code: string; message: string }) => void + onClose?: () => void + } | null = null + /** Consumed in order by terminal.resolvePane; the last entry repeats. */ + let resolvePaneOutcomes: ResolvePaneOutcome[] = [{ handle: FIRST_HANDLE }] + let recoverPaneOutcome: ResolvePaneOutcome = { handle: FIRST_HANDLE } + let methodLog: string[] = [] + let subscribeOutcomes: (Error | null)[] = [] + + function emitMultiplexReady(): void { + subscriptionCallbacks?.onResponse({ ok: true, result: { type: 'ready' } }) + } + + function latestSubscribePayload(): { streamId: number; terminal: string } { + const frame = subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .findLast((candidate) => candidate?.opcode === TerminalStreamOpcode.Subscribe) + if (!frame) { + throw new Error('missing terminal subscribe frame') + } + const payload = decodeTerminalStreamJson<{ streamId: number; terminal: string }>(frame.payload) + if (!payload) { + throw new Error('invalid terminal subscribe payload') + } + return payload + } + + function subscribedTerminalHandles(): string[] { + return subscriptionSendBinary.mock.calls + .map((call) => decodeTerminalStreamFrame(call[0])) + .flatMap((frame) => { + if (frame?.opcode !== TerminalStreamOpcode.Subscribe) { + return [] + } + const payload = decodeTerminalStreamJson<{ terminal: string }>(frame.payload) + return payload ? [payload.terminal] : [] + }) + } + + function emitSnapshot(streamId: number, data: string): void { + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotStart, + streamId, + seq: 1, + payload: encodeTerminalStreamJson({ kind: 'scrollback' }) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotChunk, + streamId, + seq: 2, + payload: encodeTerminalStreamText(data) + }) + ) + subscriptionCallbacks?.onBinary?.( + encodeTerminalStreamFrame({ + opcode: TerminalStreamOpcode.SnapshotEnd, + streamId, + seq: 3, + payload: new Uint8Array() + }) + ) + } + + function nextOutcome(outcomes: ResolvePaneOutcome[]): ResolvePaneOutcome { + return outcomes.length > 1 ? (outcomes.shift() as ResolvePaneOutcome) : outcomes[0] + } + + function paneResult(handle: string, paneKey: string, worktreeId: string): unknown { + const separator = paneKey.indexOf(':') + return { + ok: true, + result: { + terminal: { + handle, + tabId: paneKey.slice(0, separator), + leafId: paneKey.slice(separator + 1), + worktreeId + } + } + } + } + + /** Fails the pane's next resubscribe with `error`, then serves `thenHandle`. */ + function failNextResubscribeWith(error: Error, thenHandle = FIRST_HANDLE): void { + resolvePaneOutcomes = [{ error }, { handle: thenHandle }] + methodLog = [] + } + + async function attachLivePane( + overrides: Partial<{ tabId: string; leafId: string }> & { + onError?: (message: string) => void + onPtyExit?: (ptyId: string) => void + onPtyRebind?: (nextPtyId: string, previousPtyId: string) => void + } + ): Promise { + const { onError, onPtyExit, onPtyRebind, ...ids } = overrides + const { createRemoteRuntimePtyTransport } = await import('./remote-runtime-pty-transport') + const transport = createRemoteRuntimePtyTransport('env-1', { + worktreeId: PANE_WORKTREE_ID, + ...('tabId' in ids ? { tabId: ids.tabId } : { tabId: PANE_TAB_ID }), + ...('leafId' in ids ? { leafId: ids.leafId } : { leafId: PANE_LEAF_ID }), + onPtyExit, + onPtyRebind + }) + transport.attach({ + existingPtyId: FIRST_PTY_ID, + cols: 80, + rows: 24, + callbacks: { onError } + }) + await vi.waitFor(() => expect(subscriptionSendBinary).toHaveBeenCalled()) + emitSnapshot(latestSubscribePayload().streamId, 'live before the fault') + expect(transport.isConnected()).toBe(true) + return transport + } + + /** Kills the multiplexed stream so the transport enters recovery with a live epoch. */ + function dropMultiplexedStream(): void { + subscriptionCallbacks?.onClose?.() + } + + beforeEach(() => { + vi.resetModules() + vi.doUnmock('../../runtime/remote-runtime-terminal-multiplexer') + vi.doMock('@/runtime/web-runtime-session', () => ({ + refreshWebRuntimeSessionTabsSnapshot: refreshSessionTabsSnapshot + })) + vi.clearAllMocks() + subscriptionCallbacks = null + subscriptionSendBinary.mockReset() + resolvePaneOutcomes = [{ handle: FIRST_HANDLE }] + recoverPaneOutcome = { handle: FIRST_HANDLE } + subscribeOutcomes = [] + methodLog = [] + runtimeCall.mockImplementation(async (request: { method: string; params?: unknown }) => { + methodLog.push(request.method) + if (request.method === 'terminal.resolvePane') { + const params = request.params as { paneKey: string; worktreeId: string } + const outcome = nextOutcome(resolvePaneOutcomes) + if ('error' in outcome) { + throw outcome.error + } + return paneResult(outcome.handle, params.paneKey, params.worktreeId) + } + if (request.method === 'terminal.recoverPane') { + const params = request.params as { paneKey: string; worktreeId: string } + if ('error' in recoverPaneOutcome) { + throw recoverPaneOutcome.error + } + return paneResult(recoverPaneOutcome.handle, params.paneKey, params.worktreeId) + } + return { ok: true, result: { terminal: { handle: FIRST_HANDLE } } } + }) + runtimeSubscribe.mockImplementation( + async (_args: unknown, callbacks: typeof subscriptionCallbacks) => { + const outcome = subscribeOutcomes.shift() + if (outcome) { + throw outcome + } + subscriptionCallbacks = callbacks + queueMicrotask(emitMultiplexReady) + return { unsubscribe: vi.fn(), sendBinary: subscriptionSendBinary } + } + ) + vi.stubGlobal('window', { + api: { + runtimeEnvironments: { + call: runtimeCall, + subscribe: runtimeSubscribe + } + } + }) + }) + + it('re-resolves and adopts the replacement handle when a resubscribe fails stale', async () => { + const onError = vi.fn() + const onPtyRebind = vi.fn() + const transport = await attachLivePane({ onError, onPtyRebind }) + + // The host fenced this handle during the outage and has already minted its + // successor; only a require-replacement retry can reach it. + failNextResubscribeWith(new Error('terminal_handle_stale'), 'terminal-2') + dropMultiplexedStream() + + await vi.waitFor(() => expect(subscribedTerminalHandles()).toContain('terminal-2')) + // Two resolvePane round-trips: the stale one, then the require-replacement retry. + expect(methodLog.filter((method) => method === 'terminal.resolvePane')).toHaveLength(2) + expect(onPtyRebind).toHaveBeenCalledWith('remote:env-1@@terminal-2', FIRST_PTY_ID) + expect(transport.getPtyId()).toBe('remote:env-1@@terminal-2') + + emitSnapshot(latestSubscribePayload().streamId, 'after replacement') + expect(transport.isConnected()).toBe(true) + expect(transport.getRecoveryState?.().phase).toBe('connected') + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('retires the pane when the stale resubscribe finds only the fenced handle', async () => { + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ onError, onPtyExit }) + + // Host still advertises the fenced handle; require-replacement forbids + // reattaching to it, so the pane must retire rather than mirror a dead PTY. + failNextResubscribeWith(new Error('terminal_handle_stale'), FIRST_HANDLE) + dropMultiplexedStream() + + await vi.waitFor(() => expect(onPtyExit).toHaveBeenCalledWith(FIRST_PTY_ID)) + expect(methodLog.filter((method) => method === 'terminal.resolvePane')).toHaveLength(2) + expect(subscribedTerminalHandles().filter((handle) => handle === FIRST_HANDLE)).toHaveLength(1) + expect(transport.getRecoveryState?.().phase).toBe('ended') + expect(transport.getPtyId()).toBeNull() + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('retires a pane whose stale resubscribe has no tab/leaf ids to re-resolve', async () => { + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ + tabId: undefined, + leafId: undefined, + onError, + onPtyExit + }) + + // Without tab/leaf/worktree coordinates the resubscribe goes straight back + // to the multiplexer, and a stale rejection there has nothing to re-resolve. + subscribeOutcomes = [new Error('terminal_handle_stale')] + dropMultiplexedStream() + + await vi.waitFor(() => expect(onPtyExit).toHaveBeenCalledWith(FIRST_PTY_ID)) + expect(transport.getRecoveryState?.().phase).toBe('ended') + expect(transport.getPtyId()).toBeNull() + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('retires the pane when a resubscribe fails with terminal-gone', async () => { + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ onError, onPtyExit }) + + failNextResubscribeWith(new Error('terminal_gone')) + dropMultiplexedStream() + + await vi.waitFor(() => expect(onPtyExit).toHaveBeenCalledWith(FIRST_PTY_ID)) + // Lifecycle evidence, not a replaceable handle: no second re-resolve. + expect(methodLog.filter((method) => method === 'terminal.resolvePane')).toHaveLength(1) + expect(transport.getRecoveryState?.().phase).toBe('ended') + expect(transport.getPtyId()).toBeNull() + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('recovers the host pane when a resubscribe fails with an expired SSH session', async () => { + const onError = vi.fn() + const onPtyRebind = vi.fn() + const transport = await attachLivePane({ onError, onPtyRebind }) + + recoverPaneOutcome = { handle: 'terminal-3' } + failNextResubscribeWith(new Error('SSH_SESSION_EXPIRED')) + dropMultiplexedStream() + + await vi.waitFor(() => expect(methodLog).toContain('terminal.recoverPane')) + expect(runtimeCall).toHaveBeenCalledWith( + expect.objectContaining({ + method: 'terminal.recoverPane', + params: { + paneKey: `${PANE_TAB_ID}:${PANE_LEAF_ID}`, + worktreeId: PANE_WORKTREE_ID, + expectedTerminal: FIRST_HANDLE + } + }) + ) + await vi.waitFor(() => expect(subscribedTerminalHandles()).toContain('terminal-3')) + expect(onPtyRebind).toHaveBeenCalledWith('remote:env-1@@terminal-3', FIRST_PTY_ID) + + emitSnapshot(latestSubscribePayload().streamId, 'after ssh pane recovery') + expect(transport.isConnected()).toBe(true) + expect(onError).not.toHaveBeenCalled() + transport.destroy?.() + }) + + it('keeps an oversized-snapshot resubscribe failure informational', async () => { + const { REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE } = + await import('../../runtime/remote-runtime-terminal-multiplexer') + const onError = vi.fn() + const onPtyExit = vi.fn() + const transport = await attachLivePane({ onError, onPtyExit }) + + failNextResubscribeWith(new Error(REMOTE_TERMINAL_SNAPSHOT_TOO_LARGE)) + dropMultiplexedStream() + + await vi.waitFor(() => expect(transport.getRecoveryState?.().phase).toBe('disconnected')) + // The snapshot was skipped, not the terminal: no red banner, no retirement. + expect(onError).not.toHaveBeenCalled() + expect(onPtyExit).not.toHaveBeenCalled() + expect(transport.getPtyId()).toBe(FIRST_PTY_ID) + transport.destroy?.() + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-error-accumulation.test.ts b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.test.ts new file mode 100644 index 00000000000..3de52452c7b --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.test.ts @@ -0,0 +1,70 @@ +import { describe, expect, it } from 'vitest' +import { appendTerminalErrorMessage } from './terminal-error-accumulation' +import { stripSshReconnectOwnedErrorLines } from './TerminalErrorToast' + +const MULTILINE_ERROR = 'Remote terminal write failed.\nThe remote runtime rejected the request.' + +describe('appendTerminalErrorMessage', () => { + it('starts the surface with the first message', () => { + expect(appendTerminalErrorMessage(null, 'Paste failed.')).toBe('Paste failed.') + }) + + it('appends distinct messages as newline-joined entries', () => { + const accumulated = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, 'Paste failed.'), + 'Remote terminal was closed.' + ) + expect(accumulated).toBe('Paste failed.\nRemote terminal was closed.') + }) + + it('keeps the first occurrence of a repeated single-line message', () => { + const accumulated = appendTerminalErrorMessage(null, 'Paste failed.') + expect(appendTerminalErrorMessage(accumulated, 'Paste failed.')).toBe(accumulated) + }) + + it('does not re-append a repeated multi-line message', () => { + let accumulated = appendTerminalErrorMessage(null, MULTILINE_ERROR) + accumulated = appendTerminalErrorMessage(accumulated, MULTILINE_ERROR) + accumulated = appendTerminalErrorMessage(accumulated, MULTILINE_ERROR) + expect(accumulated).toBe(MULTILINE_ERROR) + }) + + it('detects a repeated multi-line message in any position of the surface', () => { + const leading = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, MULTILINE_ERROR), + 'Paste failed.' + ) + expect(appendTerminalErrorMessage(leading, MULTILINE_ERROR)).toBe(leading) + + const trailing = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, 'Paste failed.'), + MULTILINE_ERROR + ) + expect(appendTerminalErrorMessage(trailing, MULTILINE_ERROR)).toBe(trailing) + + const middle = appendTerminalErrorMessage(trailing, 'Remote terminal was closed.') + expect(appendTerminalErrorMessage(middle, MULTILINE_ERROR)).toBe(middle) + }) + + it('keeps per-line dedup for a single-line message already present as a line', () => { + const accumulated = appendTerminalErrorMessage(null, MULTILINE_ERROR) + expect(appendTerminalErrorMessage(accumulated, 'Remote terminal write failed.')).toBe( + accumulated + ) + }) + + it('appends a message that is only a substring of an existing line', () => { + const accumulated = appendTerminalErrorMessage(null, MULTILINE_ERROR) + expect(appendTerminalErrorMessage(accumulated, 'terminal write failed.')).toBe( + `${MULTILINE_ERROR}\nterminal write failed.` + ) + }) + + it('stays a newline-joined string the toast can still filter per line', () => { + const accumulated = appendTerminalErrorMessage( + appendTerminalErrorMessage(null, 'SSH connection failed: host unreachable'), + MULTILINE_ERROR + ) + expect(stripSshReconnectOwnedErrorLines(accumulated)).toBe(MULTILINE_ERROR) + }) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts new file mode 100644 index 00000000000..719d022507e --- /dev/null +++ b/src/renderer/src/components/terminal-pane/terminal-error-accumulation.ts @@ -0,0 +1,22 @@ +// Why: the error surface aggregates every pane error into ONE newline-joined +// string so TerminalErrorToast's per-line filters (isSshReconnectOwnedTerminalError, +// stripSshReconnectOwnedErrorLines) keep working. That join makes line-based +// dedup wrong for messages that themselves contain newlines: a multi-line +// message is never one line of the accumulated value, so it would re-append on +// every recurrence and grow without bound. +function containsWholeLineRun(accumulated: string, message: string): boolean { + return ( + accumulated === message || + accumulated.startsWith(`${message}\n`) || + accumulated.endsWith(`\n${message}`) || + accumulated.includes(`\n${message}\n`) + ) +} + +/** Appends an error to the aggregated surface, keeping the first occurrence of an already-present message. */ +export function appendTerminalErrorMessage(accumulated: string | null, message: string): string { + if (!accumulated) { + return message + } + return containsWholeLineRun(accumulated, message) ? accumulated : `${accumulated}\n${message}` +} diff --git a/src/shared/remote-runtime-client-error-classification.test.ts b/src/shared/remote-runtime-client-error-classification.test.ts index b819f6ba44a..786b3287a04 100644 --- a/src/shared/remote-runtime-client-error-classification.test.ts +++ b/src/shared/remote-runtime-client-error-classification.test.ts @@ -5,14 +5,17 @@ import { } from './remote-runtime-client-error-classification' describe('remote runtime client error classification', () => { - it.each(['remote_runtime_unavailable', 'runtime_timeout', 'runtime_unavailable', 'reconnecting'])( - 'treats %s as recoverable', - (code) => { - expect(isRecoverableRemoteRuntimeConnectionError({ code, message: 'transport failed' })).toBe( - true - ) - } - ) + it.each([ + 'remote_runtime_unavailable', + 'runtime_rpc_queue_overloaded', + 'runtime_timeout', + 'runtime_unavailable', + 'reconnecting' + ])('treats %s as recoverable', (code) => { + expect(isRecoverableRemoteRuntimeConnectionError({ code, message: 'transport failed' })).toBe( + true + ) + }) it('does not retry authentication or protocol failures', () => { expect( @@ -31,6 +34,7 @@ describe('remote runtime client error classification', () => { 'Remote Orca runtime closed the connection.', 'Remote Orca runtime connection closed.', 'Remote Orca runtime is not connected.', + "Error invoking remote method 'runtimeEnvironments:call': RuntimeRpcCallQueueOverloadError: Remote runtime call queue is full; retry after current calls finish.", 'Remote runtime subscription closed before it started.' ])('normalizes unstructured connection failure: %s', (message) => { const error = toRemoteRuntimeClientErrorLike(new Error(message)) diff --git a/src/shared/remote-runtime-client-error-classification.ts b/src/shared/remote-runtime-client-error-classification.ts index af5a531d3e5..1732a454f6f 100644 --- a/src/shared/remote-runtime-client-error-classification.ts +++ b/src/shared/remote-runtime-client-error-classification.ts @@ -1,7 +1,11 @@ export type RemoteRuntimeClientErrorLike = { code?: string; message: string } +export const RUNTIME_RPC_QUEUE_OVERLOAD_CODE = 'runtime_rpc_queue_overloaded' +export const RUNTIME_RPC_QUEUE_OVERLOAD_MESSAGE_FRAGMENT = 'remote runtime call queue is full' + const RECOVERABLE_CODES = new Set([ 'remote_runtime_unavailable', + RUNTIME_RPC_QUEUE_OVERLOAD_CODE, 'runtime_timeout', 'runtime_unavailable', 'reconnecting', @@ -13,12 +17,20 @@ const RECOVERABLE_MESSAGE_FRAGMENTS = [ 'remote orca runtime closed the connection', 'remote orca runtime connection closed', 'remote orca runtime is not connected', + RUNTIME_RPC_QUEUE_OVERLOAD_MESSAGE_FRAGMENT, 'remote runtime connection closed', 'remote runtime subscription closed before it started', 'remote terminal stream is not connected', 'timed out waiting for the remote orca runtime' ] +export function isRuntimeRpcQueueOverloadError(error: RemoteRuntimeClientErrorLike): boolean { + return ( + error.code === RUNTIME_RPC_QUEUE_OVERLOAD_CODE || + error.message.toLowerCase().includes(RUNTIME_RPC_QUEUE_OVERLOAD_MESSAGE_FRAGMENT) + ) +} + export function isRecoverableRemoteRuntimeConnectionError( error: RemoteRuntimeClientErrorLike ): boolean {