fix(gateway): retry connect when OpenClaw is still starting after cold launch (#1087)
This commit is contained in:
@@ -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;
|
||||
|
||||
@@ -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<void>;
|
||||
};
|
||||
|
||||
async function connectWithStartupRetry(
|
||||
hooks: StartupHooks,
|
||||
port: number,
|
||||
externalToken?: string,
|
||||
): Promise<void> {
|
||||
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<void> {
|
||||
let configRepairAttempted = false;
|
||||
let startAttempts = 0;
|
||||
@@ -45,7 +61,7 @@ export async function runGatewayStartupSequence(hooks: StartupHooks): Promise<vo
|
||||
hooks.assertLifecycle('start/find-existing');
|
||||
if (existing) {
|
||||
logger.debug(`Found existing Gateway on port ${existing.port}`);
|
||||
await hooks.connect(existing.port, existing.externalToken);
|
||||
await connectWithStartupRetry(hooks, existing.port, existing.externalToken);
|
||||
hooks.assertLifecycle('start/connect-existing');
|
||||
hooks.onConnectedToExistingGateway();
|
||||
return;
|
||||
@@ -61,7 +77,7 @@ export async function runGatewayStartupSequence(hooks: StartupHooks): Promise<vo
|
||||
logger.info('Owned Gateway process still alive (likely in-process restart); waiting for it to become ready');
|
||||
await hooks.waitForReady(hooks.port);
|
||||
hooks.assertLifecycle('start/wait-ready-owned');
|
||||
await hooks.connect(hooks.port);
|
||||
await connectWithStartupRetry(hooks, hooks.port);
|
||||
hooks.assertLifecycle('start/connect-owned');
|
||||
hooks.onConnectedToExistingGateway();
|
||||
return;
|
||||
@@ -80,7 +96,7 @@ export async function runGatewayStartupSequence(hooks: StartupHooks): Promise<vo
|
||||
await hooks.waitForReady(hooks.port);
|
||||
hooks.assertLifecycle('start/wait-ready');
|
||||
|
||||
await hooks.connect(hooks.port);
|
||||
await connectWithStartupRetry(hooks, hooks.port);
|
||||
hooks.assertLifecycle('start/connect');
|
||||
|
||||
hooks.onConnectedToManagedGateway();
|
||||
|
||||
@@ -18,10 +18,15 @@ const TRANSIENT_START_ERROR_PATTERNS: RegExp[] = [
|
||||
/Gateway process exited before becoming ready/i,
|
||||
/Timed out waiting for connect\.challenge/i,
|
||||
/Connect handshake timeout/i,
|
||||
// OpenClaw can emit connect.challenge before the connect RPC is accepted.
|
||||
/gateway starting/i,
|
||||
// Port occupied after orphan kill: transient, worth retrying with backoff
|
||||
/Port \d+ still occupied after \d+ms/i,
|
||||
];
|
||||
|
||||
/** Backoff between connect() attempts when the Gateway rejects with "still starting". */
|
||||
export const GATEWAY_CONNECT_STARTUP_RETRY_DELAYS_MS = [500, 1_000, 2_000, 4_000, 8_000, 8_000] as const;
|
||||
|
||||
function normalizeLogLine(value: string): string {
|
||||
return value.trim();
|
||||
}
|
||||
@@ -75,6 +80,53 @@ export function isTransientGatewayStartError(error: unknown): boolean {
|
||||
return TRANSIENT_START_ERROR_PATTERNS.some((pattern) => 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<void>;
|
||||
port: number;
|
||||
externalToken?: string;
|
||||
delay: (ms: number) => Promise<void>;
|
||||
retryDelaysMs?: readonly number[];
|
||||
beforeAttempt?: () => void;
|
||||
logWarn?: (message: string) => void;
|
||||
logInfo?: (message: string) => void;
|
||||
}): Promise<void> {
|
||||
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: {
|
||||
|
||||
@@ -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)));
|
||||
};
|
||||
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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', () => {
|
||||
|
||||
Reference in New Issue
Block a user