mirror of
https://github.com/bdnugget/rs-sdk.git
synced 2026-11-07 09:07:30 +00:00
fix(sdk): bound the whole connect handshake, settle on early close
connect()'s only timer was cleared on socket open, so a gateway that accepted the socket but never answered sdk_connect (dead session, half-open TCP during an outage) parked connect() forever while every liveness signal read healthy - observed as multi-minute babysitter stalls that survived server recovery (bug report mscll1sc). A socket that closed mid-handshake leaked the pending promise the same way. One connectTimeout deadline (default 30s, above the gateway's 15s auth bound) now covers socket open plus the sdk_connected/sdk_error answer, and onclose rejects a still-pending handshake. Both guards are scoped to pre-handshake, so post-connect close/reconnect semantics are unchanged. A mock-gateway test pins all four handshake outcomes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
0fccd0cc77
commit
fa3f4bd8e5
3 files changed
+117
-6
No files matched your search
+25
-6
@@ -215,6 +215,7 @@ export class BotSDK {
|
||||
browserLaunchUrl: config.browserLaunchUrl || '',
|
||||
browserLaunchTimeout: config.browserLaunchTimeout || 10000,
|
||||
readyTimeout: config.readyTimeout ?? 15000,
|
||||
connectTimeout: config.connectTimeout ?? 30000,
|
||||
actionTimeout: config.actionTimeout || 60000,
|
||||
autoReconnect: config.autoReconnect ?? true,
|
||||
reconnectMaxRetries: config.reconnectMaxRetries ?? Infinity,
|
||||
@@ -265,13 +266,21 @@ export class BotSDK {
|
||||
const url = this.config.gatewayUrl || `ws://${this.config.host}:${this.config.port}`;
|
||||
this.ws = new WebSocket(url);
|
||||
|
||||
const timeout = setTimeout(() => {
|
||||
reject(new Error('Connection timeout'));
|
||||
// One deadline for the whole handshake - socket open AND the
|
||||
// gateway's sdk_connected/sdk_error answer. The old timer was
|
||||
// cleared on socket open, so a gateway that accepted the socket
|
||||
// but never answered (dead session, half-open TCP during an
|
||||
// outage) parked connect() forever with every liveness signal
|
||||
// reading healthy (bug report mscll1sc). Sized above the
|
||||
// gateway's own 15s auth bound so a slow-but-alive login server
|
||||
// doesn't false-negative.
|
||||
let handshakeDone = false;
|
||||
const deadline = setTimeout(() => {
|
||||
reject(new Error(`Gateway handshake timed out after ${this.config.connectTimeout}ms`));
|
||||
this.ws?.close();
|
||||
}, 10000);
|
||||
}, this.config.connectTimeout);
|
||||
|
||||
this.ws.onopen = () => {
|
||||
clearTimeout(timeout);
|
||||
this.send({
|
||||
type: 'sdk_connect',
|
||||
username: this.config.botUsername,
|
||||
@@ -287,6 +296,13 @@ export class BotSDK {
|
||||
|
||||
this.ws.onclose = () => {
|
||||
console.warn(`[LOGOUT DEBUG] SDK WebSocket closed - autoReconnect=${this.config.autoReconnect}, intentionalDisconnect=${this.intentionalDisconnect}`);
|
||||
// A close before the handshake completed must settle the
|
||||
// pending connect() - otherwise its awaiters hang forever
|
||||
// while the auto-reconnect below builds a fresh promise.
|
||||
if (!handshakeDone) {
|
||||
clearTimeout(deadline);
|
||||
reject(new Error('Connection closed before the gateway handshake completed'));
|
||||
}
|
||||
this.connectPromise = null;
|
||||
this.ws = null;
|
||||
this.authenticated = false;
|
||||
@@ -307,7 +323,7 @@ export class BotSDK {
|
||||
|
||||
this.ws.onerror = (error) => {
|
||||
console.warn('[LOGOUT DEBUG] SDK WebSocket error event');
|
||||
clearTimeout(timeout);
|
||||
clearTimeout(deadline);
|
||||
reject(new Error('WebSocket error'));
|
||||
};
|
||||
|
||||
@@ -315,6 +331,8 @@ export class BotSDK {
|
||||
try {
|
||||
const msg = JSON.parse(event.data);
|
||||
if (msg.type === 'sdk_connected') {
|
||||
handshakeDone = true;
|
||||
clearTimeout(deadline);
|
||||
this.ws?.removeEventListener('message', checkConnected);
|
||||
this.reconnectAttempt = 0;
|
||||
this.authenticated = true;
|
||||
@@ -345,7 +363,8 @@ export class BotSDK {
|
||||
});
|
||||
} else if (msg.type === 'sdk_error') {
|
||||
// Handle authentication errors during connection
|
||||
clearTimeout(timeout);
|
||||
handshakeDone = true;
|
||||
clearTimeout(deadline);
|
||||
this.ws?.removeEventListener('message', checkConnected);
|
||||
const errorMessage = msg.error || 'Authentication failed';
|
||||
console.error(`[BotSDK] Connection error: ${errorMessage}`);
|
||||
|
||||
@@ -0,0 +1,84 @@
|
||||
// connect() must settle for every gateway behavior - the handshake deadline
|
||||
// covers socket open AND the sdk_connected/sdk_error answer. The old
|
||||
// implementation cleared its only timer on socket open, so a gateway that
|
||||
// accepted the socket but never answered (dead session, half-open TCP during
|
||||
// an outage) parked connect() forever while every liveness signal read
|
||||
// healthy (bug report mscll1sc).
|
||||
|
||||
import { afterEach, describe, expect, test } from 'bun:test';
|
||||
import type { Server } from 'bun';
|
||||
|
||||
import { BotSDK } from '../index';
|
||||
|
||||
type GatewayScript = (ws: { send(data: string): void; close(): void }) => void;
|
||||
|
||||
let server: Server | null = null;
|
||||
|
||||
/** A one-shot fake gateway whose sdk_connect response is scripted per test. */
|
||||
function fakeGateway(onConnect: GatewayScript): string {
|
||||
server = Bun.serve({
|
||||
port: 0,
|
||||
fetch(req, srv) {
|
||||
if (srv.upgrade(req)) return;
|
||||
return new Response('not a websocket', { status: 400 });
|
||||
},
|
||||
websocket: {
|
||||
message(ws, raw) {
|
||||
const msg = JSON.parse(String(raw));
|
||||
if (msg.type === 'sdk_connect') {
|
||||
onConnect(ws);
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
return `ws://localhost:${server.port}`;
|
||||
}
|
||||
|
||||
function makeSDK(gatewayUrl: string, connectTimeout: number): BotSDK {
|
||||
return new BotSDK({
|
||||
botUsername: 'testbot',
|
||||
password: 'x',
|
||||
gatewayUrl,
|
||||
connectTimeout,
|
||||
readyTimeout: 0, // settle at the handshake; no game state in these tests
|
||||
autoReconnect: false,
|
||||
autoLaunchBrowser: false
|
||||
});
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
server?.stop(true);
|
||||
server = null;
|
||||
});
|
||||
|
||||
describe('connect() handshake', () => {
|
||||
test('resolves when the gateway answers sdk_connected', async () => {
|
||||
const url = fakeGateway(ws => ws.send(JSON.stringify({ type: 'sdk_connected', success: true })));
|
||||
const sdk = makeSDK(url, 2000);
|
||||
await sdk.connect();
|
||||
expect(sdk.isConnected()).toBe(true);
|
||||
await sdk.disconnect();
|
||||
});
|
||||
|
||||
test('rejects when the gateway answers sdk_error', async () => {
|
||||
const url = fakeGateway(ws => ws.send(JSON.stringify({ type: 'sdk_error', error: 'Authentication failed: bad password' })));
|
||||
const sdk = makeSDK(url, 2000);
|
||||
await expect(sdk.connect()).rejects.toThrow(/Authentication failed/);
|
||||
});
|
||||
|
||||
test('rejects at the deadline when the gateway accepts the socket but never answers', async () => {
|
||||
const url = fakeGateway(() => { /* accept sdk_connect, say nothing */ });
|
||||
const sdk = makeSDK(url, 300);
|
||||
const start = Date.now();
|
||||
await expect(sdk.connect()).rejects.toThrow(/handshake timed out/);
|
||||
expect(Date.now() - start).toBeLessThan(2000); // settled by the deadline, not a socket timeout
|
||||
});
|
||||
|
||||
test('rejects promptly when the socket closes before the handshake completes', async () => {
|
||||
const url = fakeGateway(ws => ws.close());
|
||||
const sdk = makeSDK(url, 5000);
|
||||
const start = Date.now();
|
||||
await expect(sdk.connect()).rejects.toThrow(/closed before the gateway handshake/);
|
||||
expect(Date.now() - start).toBeLessThan(1000); // the close settles it, not the deadline
|
||||
});
|
||||
});
|
||||
@@ -658,6 +658,14 @@ export interface SDKConfig {
|
||||
* "authenticated fine, but no game client is logged in".
|
||||
*/
|
||||
readyTimeout?: number;
|
||||
/**
|
||||
* Deadline for the whole connect handshake - socket open plus the
|
||||
* gateway's sdk_connected/sdk_error answer (default: 30000ms). Kept above
|
||||
* the gateway's own 15s auth bound so a slow-but-alive login server
|
||||
* doesn't get misread as a dead gateway. On expiry connect() rejects
|
||||
* instead of hanging, so babysitters can recycle the process.
|
||||
*/
|
||||
connectTimeout?: number;
|
||||
actionTimeout?: number;
|
||||
autoReconnect?: boolean;
|
||||
reconnectMaxRetries?: number;
|
||||
|
||||
Reference in new issue
Block a user