refactor(desktop): keep main's boot classification and share the ws-URL guard
Follow-up to the renderer-dial retry: - Drop the `!connectionDescriptorResolved` gate so `bootFailureIsRetryable()` still decides every failure main can see (#82679 contract preserved for post-descriptor mint failures); the renderer-owned dial is OR'd in as the one failure main cannot classify. The budget check now short-circuits before the IPC, as on main. - Four booleans → one `stage` variable; the `isGatewayReauthRequired` term is dropped (reauth is raised before the dial stage, so it was unreachable). - `isGatewayWebSocketUrl` moves to `apps/shared/src/json-rpc-gateway.ts` and `JsonRpcGatewayClient.connect()` uses it, so the hook's "valid dial" predicate cannot drift from what `connect()` accepts. - Tests: keep the three that bind behaviour (first dial retries under a stale ready snapshot; invalid URL terminal; post-connect failure terminal), drop the four that pinned the removed gate or duplicated the existing #82679 bound test. 46/46 green; reverting the hook to main makes the retry test fail.
This commit is contained in:
kshitijk4poor
@@ -1,4 +1,3 @@
|
||||
import { GatewayReauthRequiredError } from '@hermes/shared'
|
||||
import { act, cleanup, render } from '@testing-library/react'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
@@ -1666,71 +1665,18 @@ describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () =>
|
||||
expect($desktopBoot.get().error).toBeNull()
|
||||
})
|
||||
|
||||
it('RETRY CONTRACT: a local descriptor never enters the renderer-side remote retry branch', async () => {
|
||||
const desktop = fakeDesktop()
|
||||
desktop.getConnection.mockImplementation(async () => ({ ...remotePrimaryConn, mode: 'local' as const }) as never)
|
||||
desktop.getBootProgress = vi.fn(async () => ({
|
||||
error: 'contradictory main snapshot',
|
||||
fakeMode: false,
|
||||
message: 'Desktop boot failed',
|
||||
phase: 'backend.error',
|
||||
progress: 24,
|
||||
retryable: true,
|
||||
running: false,
|
||||
timestamp: 1
|
||||
}))
|
||||
;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
|
||||
FakeWebSocket.mode = 'fail'
|
||||
|
||||
render(<Harness />)
|
||||
await flushAsync()
|
||||
|
||||
expect($desktopBoot.get().error).toBeTruthy()
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
await advanceBackoff()
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('RETRY CONTRACT: a confirmed remote reauth rejection remains terminal despite contradictory retryable progress', async () => {
|
||||
const desktop = fakeDesktop()
|
||||
desktop.getConnection = vi.fn(async () => ({ ...remotePrimaryConn, authMode: 'oauth' as const }))
|
||||
desktop.getGatewayWsUrl = vi.fn(async () => {
|
||||
throw new GatewayReauthRequiredError('Sign in again')
|
||||
})
|
||||
desktop.getBootProgress = vi.fn(async () => ({
|
||||
error: 'contradictory main snapshot',
|
||||
fakeMode: false,
|
||||
message: 'Desktop boot failed',
|
||||
phase: 'backend.error',
|
||||
progress: 24,
|
||||
retryable: true,
|
||||
running: false,
|
||||
timestamp: 1
|
||||
}))
|
||||
;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
|
||||
|
||||
render(<Harness />)
|
||||
await flushAsync()
|
||||
|
||||
expect($desktopBoot.get().error).toBeTruthy()
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
expect(FakeWebSocket.instances).toHaveLength(0)
|
||||
await advanceBackoff()
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('RETRY CONTRACT: invalid remote WebSocket URLs remain terminal despite contradictory retryable progress', async () => {
|
||||
it('RETRY CONTRACT: an invalid remote WebSocket URL is not a dial failure — it stays terminal under the stale ready snapshot', async () => {
|
||||
const desktop = fakeDesktop()
|
||||
desktop.getConnection = vi.fn(async () => ({ ...remotePrimaryConn, wsUrl: 'not a WebSocket URL' }))
|
||||
desktop.getGatewayWsUrl = vi.fn(async () => 'not a WebSocket URL')
|
||||
desktop.getBootProgress = vi.fn(async () => ({
|
||||
error: 'contradictory main snapshot',
|
||||
error: null,
|
||||
fakeMode: false,
|
||||
message: 'Desktop boot failed',
|
||||
phase: 'backend.error',
|
||||
progress: 24,
|
||||
retryable: true,
|
||||
running: false,
|
||||
message: 'Hermes is ready',
|
||||
phase: 'backend.ready',
|
||||
progress: 100,
|
||||
retryable: false,
|
||||
running: true,
|
||||
timestamp: 1
|
||||
}))
|
||||
;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
|
||||
@@ -1745,43 +1691,17 @@ describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () =>
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('RETRY CONTRACT: missing OAuth ticket capability remains terminal despite contradictory retryable progress', async () => {
|
||||
const desktop = fakeDesktop()
|
||||
desktop.getConnection = vi.fn(async () => ({ ...remotePrimaryConn, authMode: 'oauth' as const }))
|
||||
Reflect.deleteProperty(desktop, 'getGatewayWsUrl')
|
||||
desktop.getBootProgress = vi.fn(async () => ({
|
||||
error: 'contradictory main snapshot',
|
||||
fakeMode: false,
|
||||
message: 'Desktop boot failed',
|
||||
phase: 'backend.error',
|
||||
progress: 24,
|
||||
retryable: true,
|
||||
running: false,
|
||||
timestamp: 1
|
||||
}))
|
||||
;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
|
||||
|
||||
render(<Harness />)
|
||||
await flushAsync()
|
||||
|
||||
expect($desktopBoot.get().error).toMatch(/cannot refresh OAuth WebSocket tickets/i)
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
expect(FakeWebSocket.instances).toHaveLength(0)
|
||||
await advanceBackoff()
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('RETRY CONTRACT: a post-connect failure stays terminal even when its socket closes before boot catches it', async () => {
|
||||
it('RETRY CONTRACT: a post-connect failure stays terminal even when its socket closes before boot catches it — a closed socket after a good dial is not a dial failure', async () => {
|
||||
const desktop = fakeDesktop()
|
||||
desktop.getConnection = vi.fn(async () => remotePrimaryConn)
|
||||
desktop.getBootProgress = vi.fn(async () => ({
|
||||
error: 'contradictory main snapshot',
|
||||
error: null,
|
||||
fakeMode: false,
|
||||
message: 'Desktop boot failed',
|
||||
phase: 'backend.error',
|
||||
progress: 24,
|
||||
retryable: true,
|
||||
running: false,
|
||||
message: 'Hermes is ready',
|
||||
phase: 'backend.ready',
|
||||
progress: 100,
|
||||
retryable: false,
|
||||
running: true,
|
||||
timestamp: 1
|
||||
}))
|
||||
;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
|
||||
@@ -1801,37 +1721,6 @@ describe('useGatewayBoot remote reconnect loop (real hook, fake socket)', () =>
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
|
||||
it('RETRY CONTRACT: a persistent renderer-side remote dial outage consumes the existing five-attempt bound', async () => {
|
||||
const desktop = fakeDesktop()
|
||||
desktop.getConnection = vi.fn(async () => remotePrimaryConn)
|
||||
desktop.getBootProgress = vi.fn(async () => ({
|
||||
error: null,
|
||||
fakeMode: false,
|
||||
message: 'Hermes is ready',
|
||||
phase: 'backend.ready',
|
||||
progress: 100,
|
||||
retryable: false,
|
||||
running: true,
|
||||
timestamp: 1
|
||||
}))
|
||||
;(window as { hermesDesktop?: unknown }).hermesDesktop = desktop
|
||||
FakeWebSocket.mode = 'fail'
|
||||
|
||||
render(<Harness />)
|
||||
await flushAsync()
|
||||
|
||||
for (let i = 0; i < 7; i += 1) {
|
||||
await advanceBackoff()
|
||||
}
|
||||
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(6)
|
||||
expect(FakeWebSocket.instances).toHaveLength(6)
|
||||
expect($desktopBoot.get().error).toBeTruthy()
|
||||
await advanceBackoff()
|
||||
expect(desktop.getConnection).toHaveBeenCalledTimes(6)
|
||||
expect(FakeWebSocket.instances).toHaveLength(6)
|
||||
})
|
||||
|
||||
it('FIX #82679: boot retries are BOUNDED — a persistently dead remote ends in the recovery overlay, not a spinner', async () => {
|
||||
const desktop = fakeDesktop()
|
||||
desktop.getConnection = vi.fn(async () => {
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import { isGatewayReauthRequired, JsonRpcGatewayError, resolveGatewayWsUrl } from '@hermes/shared'
|
||||
import { isGatewayReauthRequired, isGatewayWebSocketUrl, JsonRpcGatewayError, resolveGatewayWsUrl } from '@hermes/shared'
|
||||
import { useEffect, useRef } from 'react'
|
||||
|
||||
import { shouldApplyPostBootProgressError } from '@/components/boot-failure-reauth'
|
||||
@@ -108,13 +108,13 @@ const RECONNECT_ESCALATE_AFTER_MS = 300_000
|
||||
// only a STREAK of unanswered pings rebuilds the transport.
|
||||
const GATEWAY_LIVENESS_PROBE_TIMEOUT_MS = 5_000
|
||||
|
||||
// Bounded self-heal for a failed REMOTE boot (#82679): main classifies faults
|
||||
// before a descriptor exists; the renderer also classifies a valid remote
|
||||
// WebSocket dial that fails before becoming usable. The renderer re-attempts
|
||||
// the whole boot with the same full-jitter backoff the post-boot reconnect loop
|
||||
// uses, up to this many attempts. Retries are bounded and end in the real
|
||||
// recovery affordance (the boot-failure overlay with Retry / Settings), never
|
||||
// an infinite spinner. Local failures, mint/capability failures, and confirmed
|
||||
// Bounded self-heal for a failed REMOTE boot (#82679): main classifies every
|
||||
// fault it can see (via getBootProgress().retryable); the renderer adds the one
|
||||
// it cannot — a valid remote WebSocket dial that fails before becoming usable.
|
||||
// The renderer re-attempts the whole boot with the same full-jitter backoff the
|
||||
// post-boot reconnect loop uses, up to this many attempts. Retries are bounded
|
||||
// and end in the real recovery affordance (the boot-failure overlay with
|
||||
// Retry / Settings), never an infinite spinner. Local failures and confirmed
|
||||
// reauth rejections never enter this loop.
|
||||
const BOOT_RETRY_MAX_ATTEMPTS = 5
|
||||
// Base delay for boot retries. Deliberately slower than the socket reconnect
|
||||
@@ -139,16 +139,6 @@ export function primaryRuntimeConnectionId(connection: Pick<HermesConnection, 'c
|
||||
return connection.mode === 'local' ? 'local' : null
|
||||
}
|
||||
|
||||
function isGatewayWebSocketUrl(value: string): boolean {
|
||||
try {
|
||||
const url = new URL(value)
|
||||
|
||||
return url.protocol === 'ws:' || url.protocol === 'wss:'
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
interface GatewayBootOptions {
|
||||
beforeConnectionSwitch: () => void
|
||||
handleGatewayEvent: (event: RpcEvent) => void
|
||||
@@ -1026,13 +1016,10 @@ export function useGatewayBoot({
|
||||
})
|
||||
|
||||
async function boot() {
|
||||
// These are historical facts for one boot attempt, not a late read of
|
||||
// Where this boot attempt got to — a historical fact, not a late read of
|
||||
// gateway.connectionState. A socket can close after a successful dial;
|
||||
// later initialization errors must not be reclassified as boot dials.
|
||||
let connectionDescriptorResolved = false
|
||||
let resolvedRemoteConnection = false
|
||||
let remoteGatewayConnectAttempted = false
|
||||
let gatewayConnectSucceeded = false
|
||||
let stage: 'resolving' | 'minting' | 'dialing' | 'connected' = 'resolving'
|
||||
|
||||
try {
|
||||
// A profile-pinned helper window (the HUD) dials its target profile's
|
||||
@@ -1052,8 +1039,7 @@ export function useGatewayBoot({
|
||||
return
|
||||
}
|
||||
|
||||
connectionDescriptorResolved = true
|
||||
resolvedRemoteConnection = conn.mode === 'remote'
|
||||
stage = 'minting'
|
||||
|
||||
setDesktopBootStep({
|
||||
phase: 'renderer.gateway.connect',
|
||||
@@ -1079,23 +1065,21 @@ export function useGatewayBoot({
|
||||
// Mint a fresh WS URL right before connecting. For OAuth gateways the
|
||||
// ticket is single-use with a short TTL, so the ticket baked into
|
||||
// conn.wsUrl is stale; resolveGatewayWsUrl() re-mints it rather than
|
||||
// connecting with a dead ticket. Auth rejection asks for sign-in; the
|
||||
// Electron mint path has its own bounded transport retries. This await
|
||||
// is bounded like the reconnect path (#93454) so a wedged mint reaches
|
||||
// the recovery affordance instead of hanging "Starting Hermes…".
|
||||
// connecting with a dead ticket. Auth rejection asks for sign-in. This
|
||||
// await is bounded like the reconnect path (#93454) so a wedged mint
|
||||
// reaches the recovery affordance instead of hanging "Starting Hermes…".
|
||||
const wsUrl = await withTimeout(
|
||||
resolveGatewayWsUrl(desktop, conn),
|
||||
RECONNECT_ATTEMPT_TIMEOUT_MS,
|
||||
'Timed out minting the gateway WebSocket URL'
|
||||
)
|
||||
|
||||
// The complementary retry classification is deliberately narrower
|
||||
// than every remote pre-open error: only a valid WebSocket dial after
|
||||
// a resolved remote descriptor is transient here. URL and capability
|
||||
// failures stay terminal at their own boundaries.
|
||||
remoteGatewayConnectAttempted = resolvedRemoteConnection && isGatewayWebSocketUrl(wsUrl)
|
||||
// Only a valid WebSocket dial against a remote descriptor counts as a
|
||||
// transient renderer-side failure; URL and capability failures stay
|
||||
// terminal at their own boundaries.
|
||||
if (conn.mode === 'remote' && isGatewayWebSocketUrl(wsUrl)) {stage = 'dialing'}
|
||||
await gateway.connect(wsUrl)
|
||||
gatewayConnectSucceeded = true
|
||||
stage = 'connected'
|
||||
|
||||
if (cancelled) {
|
||||
return
|
||||
@@ -1142,25 +1126,16 @@ export function useGatewayBoot({
|
||||
if (!cancelled) {
|
||||
const message = err instanceof Error ? err.message : String(err)
|
||||
|
||||
// Preserve the main-process classification for failures before a
|
||||
// descriptor is available. Once this renderer has a descriptor, the
|
||||
// only complementary retry is a transient, valid remote WebSocket
|
||||
// dial which never became usable. A confirmed reauth rejection,
|
||||
// invalid URL/capability failure, local descriptor, and anything
|
||||
// after a successful dial retain the terminal recovery surface.
|
||||
const retryableBeforeDescriptor = !connectionDescriptorResolved && (await bootFailureIsRetryable())
|
||||
// Main's classification (#82679) still decides every failure it can
|
||||
// see. The one it cannot see is the renderer-owned WebSocket dial:
|
||||
// after a renderer reload main serves its cached descriptor with a
|
||||
// stale `backend.ready / retryable:false` snapshot, so a remote dial
|
||||
// that never became usable is retryable on its own. Anything after a
|
||||
// successful dial keeps the terminal recovery surface.
|
||||
const canRetry = bootRetryAttempt < BOOT_RETRY_MAX_ATTEMPTS
|
||||
const retryable = canRetry && (stage === 'dialing' || (await bootFailureIsRetryable()))
|
||||
|
||||
const retryableRemoteGatewayDial =
|
||||
resolvedRemoteConnection &&
|
||||
remoteGatewayConnectAttempted &&
|
||||
!gatewayConnectSucceeded &&
|
||||
!isGatewayReauthRequired(err)
|
||||
|
||||
if (
|
||||
bootRetryAttempt < BOOT_RETRY_MAX_ATTEMPTS &&
|
||||
(retryableBeforeDescriptor || retryableRemoteGatewayDial) &&
|
||||
!cancelled
|
||||
) {
|
||||
if (retryable && !cancelled) {
|
||||
const delay = reconnectBackoffDelayMs(bootRetryAttempt, { baseDelayMs: BOOT_RETRY_BASE_DELAY_MS })
|
||||
bootRetryAttempt += 1
|
||||
resumeDesktopBootForRetry(translateNow('boot.steps.retryingRemoteBackend'))
|
||||
|
||||
@@ -52,6 +52,7 @@ export {
|
||||
type GatewayEvent,
|
||||
type GatewayEventName,
|
||||
type GatewayRequestId,
|
||||
isGatewayWebSocketUrl,
|
||||
type JsonRpcErrorPayload,
|
||||
type JsonRpcFrame,
|
||||
JsonRpcGatewayClient,
|
||||
|
||||
@@ -100,6 +100,19 @@ const DEFAULT_HEARTBEAT_DEADLINE_MS = 45_000
|
||||
// handshake doesn't land in this window, fail to 'error' so callers can retry.
|
||||
const DEFAULT_CONNECT_TIMEOUT_MS = 15_000
|
||||
|
||||
/** True for a `ws://` / `wss://` URL string — the only thing `JsonRpcGatewayClient.connect()` will dial. */
|
||||
export function isGatewayWebSocketUrl(value: unknown): value is string {
|
||||
if (typeof value !== 'string') {return false}
|
||||
|
||||
try {
|
||||
const protocol = new URL(value).protocol
|
||||
|
||||
return protocol === 'ws:' || protocol === 'wss:'
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
}
|
||||
|
||||
export class JsonRpcGatewayClient {
|
||||
private nextId = 0
|
||||
private pending = new Map<GatewayRequestId, PendingCall>()
|
||||
@@ -162,19 +175,7 @@ export class JsonRpcGatewayClient {
|
||||
return new Error(`gateway connect() requires a ws:// or wss:// URL string, got ${got}`)
|
||||
}
|
||||
|
||||
if (typeof wsUrl !== 'string') {
|
||||
throw invalidUrl()
|
||||
}
|
||||
|
||||
let url: URL
|
||||
|
||||
try {
|
||||
url = new URL(wsUrl)
|
||||
} catch {
|
||||
throw invalidUrl()
|
||||
}
|
||||
|
||||
if (url.protocol !== 'ws:' && url.protocol !== 'wss:') {
|
||||
if (!isGatewayWebSocketUrl(wsUrl)) {
|
||||
throw invalidUrl()
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user