From e91b6774d4dca5c663f8db287a09a4cdc847cf6f Mon Sep 17 00:00:00 2001 From: paisley <8197966+su8su@users.noreply.github.com> Date: Tue, 2 Jun 2026 15:17:18 +0800 Subject: [PATCH] fix(gateway): retry connect when OpenClaw is still starting after cold launch (#1087) --- electron/gateway/manager.ts | 4 + electron/gateway/startup-orchestrator.ts | 24 +++++- electron/gateway/startup-recovery.ts | 52 ++++++++++++ electron/gateway/ws-client.ts | 7 ++ .../unit/gateway-startup-orchestrator.test.ts | 31 +++++++- tests/unit/gateway-startup-recovery.test.ts | 79 ++++++++++++++++++- tests/unit/gateway-ws-client.test.ts | 50 ++++++++++++ 7 files changed, 240 insertions(+), 7 deletions(-) diff --git a/electron/gateway/manager.ts b/electron/gateway/manager.ts index dedc770..26d208e 100644 --- a/electron/gateway/manager.ts +++ b/electron/gateway/manager.ts @@ -435,6 +435,10 @@ export class GatewayManager extends EventEmitter { error ); this.setStatus({ state: 'error', error: String(error) }); + if (this.shouldReconnect) { + logger.warn('Gateway start failed; scheduling auto-reconnect recovery'); + this.scheduleReconnect(); + } throw error; } finally { this.startLock = false; diff --git a/electron/gateway/startup-orchestrator.ts b/electron/gateway/startup-orchestrator.ts index 7e4bc04..0e86c09 100644 --- a/electron/gateway/startup-orchestrator.ts +++ b/electron/gateway/startup-orchestrator.ts @@ -1,6 +1,6 @@ import { logger } from '../utils/logger'; import { LifecycleSupersededError } from './lifecycle-controller'; -import { getGatewayStartupRecoveryAction } from './startup-recovery'; +import { connectGatewayWithStartupRetry, getGatewayStartupRecoveryAction } from './startup-recovery'; export interface ExistingGatewayInfo { port: number; @@ -29,6 +29,22 @@ type StartupHooks = { delay: (ms: number) => Promise; }; +async function connectWithStartupRetry( + hooks: StartupHooks, + port: number, + externalToken?: string, +): Promise { + await connectGatewayWithStartupRetry({ + connect: hooks.connect, + port, + externalToken, + delay: hooks.delay, + beforeAttempt: () => hooks.assertLifecycle('start/connect-retry'), + logWarn: (message) => logger.warn(message), + logInfo: (message) => logger.info(message), + }); +} + export async function runGatewayStartupSequence(hooks: StartupHooks): Promise { let configRepairAttempted = false; let startAttempts = 0; @@ -45,7 +61,7 @@ export async function runGatewayStartupSequence(hooks: StartupHooks): Promise pattern.test(errorText)); } +export function isGatewayStillStartingError(error: unknown): boolean { + const errorText = error instanceof Error + ? error.message + : String(error ?? ''); + return /gateway starting/i.test(errorText); +} + +export async function connectGatewayWithStartupRetry(options: { + connect: (port: number, externalToken?: string) => Promise; + port: number; + externalToken?: string; + delay: (ms: number) => Promise; + retryDelaysMs?: readonly number[]; + beforeAttempt?: () => void; + logWarn?: (message: string) => void; + logInfo?: (message: string) => void; +}): Promise { + const retryDelaysMs = options.retryDelaysMs ?? GATEWAY_CONNECT_STARTUP_RETRY_DELAYS_MS; + const logWarn = options.logWarn ?? (() => {}); + const logInfo = options.logInfo ?? (() => {}); + let lastError: unknown; + + for (let attempt = 0; attempt <= retryDelaysMs.length; attempt += 1) { + options.beforeAttempt?.(); + try { + await options.connect(options.port, options.externalToken); + if (attempt > 0) { + logInfo(`Gateway connect succeeded after ${attempt + 1} attempt(s)`); + } + return; + } catch (error) { + lastError = error; + if (!isGatewayStillStartingError(error) || attempt >= retryDelaysMs.length) { + throw error; + } + const delayMs = retryDelaysMs[attempt] ?? retryDelaysMs[retryDelaysMs.length - 1]!; + logWarn( + `Gateway connect rejected while still starting (${String(error)}); ` + + `retrying in ${delayMs}ms (${attempt + 1}/${retryDelaysMs.length})`, + ); + await options.delay(delayMs); + } + } + + throw lastError instanceof Error ? lastError : new Error(String(lastError ?? 'Gateway connect failed')); +} + export type GatewayStartupRecoveryAction = 'repair' | 'retry' | 'fail'; export function getGatewayStartupRecoveryAction(options: { diff --git a/electron/gateway/ws-client.ts b/electron/gateway/ws-client.ts index 28df689..900221e 100644 --- a/electron/gateway/ws-client.ts +++ b/electron/gateway/ws-client.ts @@ -223,6 +223,13 @@ export async function connectGatewaySocket(options: { if (settled) return; settled = true; cleanupHandshakeRequest(); + if (!handshakeComplete) { + try { + ws.terminate(); + } catch { + // ignore cleanup errors during failed startup handshakes + } + } reject(error instanceof Error ? error : new Error(String(error))); }; diff --git a/tests/unit/gateway-startup-orchestrator.test.ts b/tests/unit/gateway-startup-orchestrator.test.ts index ef336c5..f109243 100644 --- a/tests/unit/gateway-startup-orchestrator.test.ts +++ b/tests/unit/gateway-startup-orchestrator.test.ts @@ -75,7 +75,7 @@ describe('runGatewayStartupSequence', () => { expect(hooks.findExistingGateway).toHaveBeenCalledWith(18789); expect(hooks.hasOwnedProcess).toHaveBeenCalled(); expect(hooks.waitForReady).toHaveBeenCalledWith(18789); - expect(hooks.connect).toHaveBeenCalledWith(18789); + expect(hooks.connect).toHaveBeenCalledWith(18789, undefined); expect(hooks.onConnectedToExistingGateway).toHaveBeenCalledTimes(1); // Must NOT start a new process or wait for port free @@ -102,7 +102,7 @@ describe('runGatewayStartupSequence', () => { expect(hooks.waitForPortFree).toHaveBeenCalledWith(18789); expect(hooks.startProcess).toHaveBeenCalledTimes(1); expect(hooks.waitForReady).toHaveBeenCalledWith(18789); - expect(hooks.connect).toHaveBeenCalledWith(18789); + expect(hooks.connect).toHaveBeenCalledWith(18789, undefined); expect(hooks.onConnectedToManagedGateway).toHaveBeenCalledTimes(1); expect(hooks.onConnectedToExistingGateway).not.toHaveBeenCalled(); @@ -256,4 +256,31 @@ describe('runGatewayStartupSequence', () => { // resetStartupStderrLines should be called once per attempt expect(hooks.resetStartupStderrLines).toHaveBeenCalledTimes(2); }); + + it('retries connect in-place when gateway is still starting', async () => { + let callCount = 0; + const hooks = createMockHooks({ + findExistingGateway: vi.fn().mockResolvedValue(null), + hasOwnedProcess: vi.fn().mockReturnValue(false), + connect: vi.fn().mockImplementation(async () => { + callCount++; + if (callCount === 1) { + throw new Error('gateway starting; retry shortly'); + } + }), + delay: vi.fn().mockImplementation(async (ms: number) => { + // Speed up inner connect retry delays in the test harness. + if (ms >= 500) { + await Promise.resolve(); + } + }), + maxStartAttempts: 3, + }); + + await runGatewayStartupSequence(hooks); + + expect(hooks.connect).toHaveBeenCalledTimes(2); + expect(hooks.startProcess).toHaveBeenCalledTimes(1); + expect(hooks.onConnectedToManagedGateway).toHaveBeenCalledTimes(1); + }); }); diff --git a/tests/unit/gateway-startup-recovery.test.ts b/tests/unit/gateway-startup-recovery.test.ts index 9ff766a..6a44f6c 100644 --- a/tests/unit/gateway-startup-recovery.test.ts +++ b/tests/unit/gateway-startup-recovery.test.ts @@ -1,8 +1,11 @@ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { + connectGatewayWithStartupRetry, getGatewayStartupRecoveryAction, hasInvalidConfigFailureSignal, + isGatewayStillStartingError, isInvalidConfigSignal, + isTransientGatewayStartError, shouldAttemptConfigAutoRepair, } from '@electron/gateway/startup-recovery'; @@ -109,4 +112,78 @@ describe('getGatewayStartupRecoveryAction', () => { }); expect(action).toBe('fail'); }); + + it('returns retry for gateway still starting handshake rejection', () => { + const action = getGatewayStartupRecoveryAction({ + startupError: new Error('gateway starting; retry shortly'), + startupStderrLines: [], + configRepairAttempted: false, + attempt: 1, + maxAttempts: 3, + }); + expect(action).toBe('retry'); + expect(isTransientGatewayStartError(new Error('gateway starting; retry shortly'))).toBe(true); + expect(isGatewayStillStartingError(new Error('gateway starting; retry shortly'))).toBe(true); + }); +}); + +describe('connectGatewayWithStartupRetry', () => { + it('retries connect when gateway is still starting', async () => { + const connect = vi.fn() + .mockRejectedValueOnce(new Error('gateway starting; retry shortly')) + .mockRejectedValueOnce(new Error('gateway starting; retry shortly')) + .mockResolvedValueOnce(undefined); + const delay = vi.fn().mockResolvedValue(undefined); + + await connectGatewayWithStartupRetry({ + connect, + port: 18789, + delay, + retryDelaysMs: [10, 20], + }); + + expect(connect).toHaveBeenCalledTimes(3); + expect(delay).toHaveBeenCalledTimes(2); + expect(delay).toHaveBeenNthCalledWith(1, 10); + expect(delay).toHaveBeenNthCalledWith(2, 20); + }); + + it('throws immediately for non-starting errors', async () => { + const connect = vi.fn().mockRejectedValue(new Error('token mismatch')); + const delay = vi.fn().mockResolvedValue(undefined); + + await expect(connectGatewayWithStartupRetry({ + connect, + port: 18789, + delay, + retryDelaysMs: [10], + })).rejects.toThrow('token mismatch'); + + expect(connect).toHaveBeenCalledTimes(1); + expect(delay).not.toHaveBeenCalled(); + }); + + it('checks lifecycle before each retry attempt', async () => { + const connect = vi.fn() + .mockRejectedValueOnce(new Error('gateway starting; retry shortly')) + .mockResolvedValueOnce(undefined); + const delay = vi.fn().mockResolvedValue(undefined); + const beforeAttempt = vi.fn() + .mockImplementationOnce(() => {}) + .mockImplementationOnce(() => { + throw new Error('Gateway start superseded'); + }); + + await expect(connectGatewayWithStartupRetry({ + connect, + port: 18789, + delay, + beforeAttempt, + retryDelaysMs: [10], + })).rejects.toThrow('Gateway start superseded'); + + expect(beforeAttempt).toHaveBeenCalledTimes(2); + expect(connect).toHaveBeenCalledTimes(1); + expect(delay).toHaveBeenCalledWith(10); + }); }); diff --git a/tests/unit/gateway-ws-client.test.ts b/tests/unit/gateway-ws-client.test.ts index e1d2f07..e167352 100644 --- a/tests/unit/gateway-ws-client.test.ts +++ b/tests/unit/gateway-ws-client.test.ts @@ -189,6 +189,56 @@ describe('connectGatewaySocket', () => { expect(socket.close).toHaveBeenCalledTimes(1); expect(pendingRequests.size).toBe(0); }); + + it('terminates the pre-handshake socket when connect is rejected', async () => { + const pendingRequests = new Map(); + + const connectionPromise = connectGatewaySocket({ + port: 18789, + deviceIdentity: null, + platform: 'win32', + pendingRequests, + getToken: vi.fn().mockResolvedValue('token-123'), + onHandshakeComplete: vi.fn(), + onMessage: (message) => { + if (typeof message !== 'object' || message === null) return; + const msg = message as { type?: string; id?: string; ok?: boolean; error?: { message?: string } }; + if (msg.type !== 'res' || typeof msg.id !== 'string') return; + const pending = pendingRequests.get(msg.id); + if (!pending) return; + pending.reject(new Error(msg.error?.message ?? 'Gateway request failed')); + }, + onCloseAfterHandshake: vi.fn(), + }); + const connectionErrorPromise = connectionPromise.then( + () => null, + (error) => error, + ); + + const socket = getLatestSocket(); + socket.emitOpen(); + socket.emitJsonMessage({ + type: 'event', + event: 'connect.challenge', + payload: { nonce: 'nonce-123' }, + }); + + await flushMicrotasks(); + + const connectFrame = JSON.parse(socket.sentFrames[0]) as { id: string }; + socket.emitJsonMessage({ + type: 'res', + id: connectFrame.id, + ok: false, + error: { message: 'gateway starting; retry shortly' }, + }); + + const connectionError = await connectionErrorPromise; + expect(connectionError).toBeInstanceOf(Error); + expect((connectionError as Error).message).toBe('gateway starting; retry shortly'); + expect(socket.terminate).toHaveBeenCalledTimes(1); + expect(pendingRequests.size).toBe(0); + }); }); describe('probeGatewayReady', () => {