fix(FN-958): fix terminal idle disconnect with heartbeat tolerance
- Add heartbeat tolerance window to prevent false idle disconnects on terminal connections - Increase client heartbeat interval from 30s to 45s for better idle tolerance - Add session staleness detection on reconnect with idle state logging - Add tests for heartbeat tolerance and client interval configuration - Add changeset for patch release
This commit is contained in:
5
.changeset/fix-terminal-idle-disconnect.md
Normal file
5
.changeset/fix-terminal-idle-disconnect.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"@gsxdsm/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix terminal WebSocket disconnect after tab idle. Server now tolerates 2 consecutive missed pongs (~90s) before terminating, and client heartbeat interval increased to 45s for better resilience against browser throttling.
|
||||||
@@ -505,4 +505,51 @@ describe("useTerminal", () => {
|
|||||||
expect(onConnect).toHaveBeenLastCalledWith({ shell: "/bin/zsh", cwd: "/new" });
|
expect(onConnect).toHaveBeenLastCalledWith({ shell: "/bin/zsh", cwd: "/new" });
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("heartbeat interval", () => {
|
||||||
|
it("sends client ping at 45-second interval (not 30s)", () => {
|
||||||
|
vi.useFakeTimers();
|
||||||
|
renderHook(() => useTerminal("heartbeat-test"));
|
||||||
|
const ws = MockWebSocket.instances[0];
|
||||||
|
|
||||||
|
act(() => {
|
||||||
|
ws.emitOpen();
|
||||||
|
});
|
||||||
|
|
||||||
|
// Clear any messages sent during open
|
||||||
|
ws.sent.length = 0;
|
||||||
|
|
||||||
|
// At 30 seconds, no ping should be sent yet (old interval was 30s)
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
});
|
||||||
|
|
||||||
|
const pingsAt30s = ws.sent.filter((m) => {
|
||||||
|
try { return JSON.parse(m).type === "ping"; } catch { return false; }
|
||||||
|
});
|
||||||
|
expect(pingsAt30s).toHaveLength(0);
|
||||||
|
|
||||||
|
// At 45 seconds, the first ping should be sent
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(15000);
|
||||||
|
});
|
||||||
|
|
||||||
|
const pingsAt45s = ws.sent.filter((m) => {
|
||||||
|
try { return JSON.parse(m).type === "ping"; } catch { return false; }
|
||||||
|
});
|
||||||
|
expect(pingsAt45s).toHaveLength(1);
|
||||||
|
|
||||||
|
// At 90 seconds, second ping should be sent
|
||||||
|
act(() => {
|
||||||
|
vi.advanceTimersByTime(45000);
|
||||||
|
});
|
||||||
|
|
||||||
|
const pingsAt90s = ws.sent.filter((m) => {
|
||||||
|
try { return JSON.parse(m).type === "ping"; } catch { return false; }
|
||||||
|
});
|
||||||
|
expect(pingsAt90s).toHaveLength(2);
|
||||||
|
|
||||||
|
vi.useRealTimers();
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -41,7 +41,7 @@ interface BufferedMessages {
|
|||||||
|
|
||||||
const MAX_RECONNECT_ATTEMPTS = 5;
|
const MAX_RECONNECT_ATTEMPTS = 5;
|
||||||
const INITIAL_RECONNECT_DELAY = 1000;
|
const INITIAL_RECONNECT_DELAY = 1000;
|
||||||
const HEARTBEAT_INTERVAL = 30000;
|
const HEARTBEAT_INTERVAL = 45000; // 45 seconds — slightly longer than server's 30s interval
|
||||||
|
|
||||||
function createEmptyBuffer(): BufferedMessages {
|
function createEmptyBuffer(): BufferedMessages {
|
||||||
return { scrollback: null, connected: null, data: [] };
|
return { scrollback: null, connected: null, data: [] };
|
||||||
|
|||||||
@@ -1,10 +1,34 @@
|
|||||||
// @vitest-environment node
|
// @vitest-environment node
|
||||||
|
|
||||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||||
import { createServer } from "./server.js";
|
import http from "node:http";
|
||||||
|
import express from "express";
|
||||||
|
import { createServer, setupTerminalWebSocket } from "./server.js";
|
||||||
import type { TaskStore } from "@fusion/core";
|
import type { TaskStore } from "@fusion/core";
|
||||||
import { get as performGet, request as performRequest } from "./test-request.js";
|
import { get as performGet, request as performRequest } from "./test-request.js";
|
||||||
|
|
||||||
|
// Mock terminal-service before any imports that use it
|
||||||
|
vi.mock("./terminal-service.js", () => {
|
||||||
|
const mockTerminalService = {
|
||||||
|
getSession: vi.fn(),
|
||||||
|
getScrollbackAndClearPending: vi.fn().mockReturnValue(null),
|
||||||
|
onData: vi.fn().mockReturnValue(() => {}),
|
||||||
|
onExit: vi.fn().mockReturnValue(() => {}),
|
||||||
|
write: vi.fn(),
|
||||||
|
resize: vi.fn(),
|
||||||
|
evictStaleSessions: vi.fn().mockReturnValue(0),
|
||||||
|
};
|
||||||
|
|
||||||
|
return {
|
||||||
|
getTerminalService: vi.fn(() => mockTerminalService),
|
||||||
|
STALE_SESSION_THRESHOLD_MS: 300_000,
|
||||||
|
__mockTerminalService: mockTerminalService,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
// Access the mock terminal service
|
||||||
|
const { __mockTerminalService: mockTerminalService } = await import("./terminal-service.js") as any;
|
||||||
|
|
||||||
function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||||
return {
|
return {
|
||||||
getTask: vi.fn(),
|
getTask: vi.fn(),
|
||||||
@@ -248,3 +272,193 @@ describe("API Error Handling Middleware", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("Terminal WebSocket heartbeat", () => {
|
||||||
|
let app: ReturnType<typeof express>;
|
||||||
|
let server: http.Server;
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
app = express();
|
||||||
|
server = http.createServer(app);
|
||||||
|
vi.useFakeTimers();
|
||||||
|
vi.spyOn(console, "warn").mockImplementation(() => {});
|
||||||
|
vi.spyOn(console, "info").mockImplementation(() => {});
|
||||||
|
vi.spyOn(console, "log").mockImplementation(() => {});
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
vi.useRealTimers();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
server.close();
|
||||||
|
});
|
||||||
|
|
||||||
|
/** Create a mock WebSocket that simulates the ws library's WebSocket */
|
||||||
|
function createMockWs(): any {
|
||||||
|
const listeners: Record<string, Function[]> = {};
|
||||||
|
return {
|
||||||
|
readyState: 1, // OPEN
|
||||||
|
_listeners: listeners,
|
||||||
|
on(event: string, handler: Function) {
|
||||||
|
if (!listeners[event]) listeners[event] = [];
|
||||||
|
listeners[event].push(handler);
|
||||||
|
},
|
||||||
|
emit(event: string, ...args: any[]) {
|
||||||
|
(listeners[event] || []).forEach((h) => h(...args));
|
||||||
|
},
|
||||||
|
send: vi.fn(),
|
||||||
|
close: vi.fn(),
|
||||||
|
terminate: vi.fn(),
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Create a mock HTTP request with sessionId */
|
||||||
|
function createMockReq(sessionId: string): any {
|
||||||
|
return {
|
||||||
|
url: `/api/terminal/ws?sessionId=${sessionId}`,
|
||||||
|
headers: { host: "localhost:3000" },
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Setup terminal WebSocket and trigger a connection */
|
||||||
|
function setupAndConnect(ws: any, req: any): void {
|
||||||
|
const wss = setupTerminalWebSocket(app, server);
|
||||||
|
|
||||||
|
// The function sets up wss on the server's upgrade event.
|
||||||
|
// We need to access the WebSocketServer directly to emit a connection.
|
||||||
|
// setupTerminalWebSocket stores wss on the app
|
||||||
|
const storedWss = (app as any).terminalWsServer;
|
||||||
|
if (storedWss) {
|
||||||
|
storedWss.emit("connection", ws, req);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
it("does NOT terminate connection after 1 missed pong", () => {
|
||||||
|
const ws = createMockWs();
|
||||||
|
const req = createMockReq("session-1");
|
||||||
|
|
||||||
|
// Setup a mock session
|
||||||
|
mockTerminalService.getSession.mockReturnValue({
|
||||||
|
id: "session-1",
|
||||||
|
shell: "/bin/bash",
|
||||||
|
cwd: "/project",
|
||||||
|
lastActivityAt: new Date(),
|
||||||
|
});
|
||||||
|
|
||||||
|
setupAndConnect(ws, req);
|
||||||
|
|
||||||
|
// First ping interval: mark as not alive
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
// The server sends a ping, ws is marked as not alive
|
||||||
|
expect(ws.send).toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Don't send a pong response — simulate missed pong
|
||||||
|
// Second ping interval: first missed pong — should NOT terminate
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
|
||||||
|
// Connection should still be alive after 1 missed pong
|
||||||
|
expect(ws.terminate).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("terminates connection after 2 consecutive missed pongs", () => {
|
||||||
|
const ws = createMockWs();
|
||||||
|
const req = createMockReq("session-2");
|
||||||
|
|
||||||
|
mockTerminalService.getSession.mockReturnValue({
|
||||||
|
id: "session-2",
|
||||||
|
shell: "/bin/bash",
|
||||||
|
cwd: "/project",
|
||||||
|
lastActivityAt: new Date(),
|
||||||
|
});
|
||||||
|
|
||||||
|
setupAndConnect(ws, req);
|
||||||
|
|
||||||
|
// First ping interval: mark as not alive (isAlive = false)
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
expect(ws.send).toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Don't send pong — missed pong #1
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
expect(ws.terminate).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Don't send pong — missed pong #2: should terminate
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
expect(ws.terminate).toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("resets missed pong counter on successful pong", () => {
|
||||||
|
const ws = createMockWs();
|
||||||
|
const req = createMockReq("session-3");
|
||||||
|
|
||||||
|
mockTerminalService.getSession.mockReturnValue({
|
||||||
|
id: "session-3",
|
||||||
|
shell: "/bin/bash",
|
||||||
|
cwd: "/project",
|
||||||
|
lastActivityAt: new Date(),
|
||||||
|
});
|
||||||
|
|
||||||
|
setupAndConnect(ws, req);
|
||||||
|
|
||||||
|
// First interval: mark as not alive
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
|
||||||
|
// Miss first pong — interval 2: missedPongs = 1
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
expect(ws.terminate).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Now respond with pong (application-level "pong" message)
|
||||||
|
const msgHandler = ws._listeners["message"]?.[0];
|
||||||
|
expect(msgHandler).toBeDefined();
|
||||||
|
msgHandler!(Buffer.from(JSON.stringify({ type: "pong" })));
|
||||||
|
|
||||||
|
// Interval 3: isAlive is true again, missedPongs is 0
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
// Still alive — missed pong counter was reset
|
||||||
|
expect(ws.terminate).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Miss 2 more pongs — should still be alive after just 1 more miss
|
||||||
|
vi.advanceTimersByTime(30000);
|
||||||
|
expect(ws.terminate).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("logs warning for stale session reconnect", () => {
|
||||||
|
const ws = createMockWs();
|
||||||
|
const req = createMockReq("stale-session");
|
||||||
|
|
||||||
|
// Session last active 10 minutes ago (past the 5-minute threshold)
|
||||||
|
const tenMinutesAgo = new Date(Date.now() - 600_000);
|
||||||
|
mockTerminalService.getSession.mockReturnValue({
|
||||||
|
id: "stale-session",
|
||||||
|
shell: "/bin/bash",
|
||||||
|
cwd: "/project",
|
||||||
|
lastActivityAt: tenMinutesAgo,
|
||||||
|
});
|
||||||
|
|
||||||
|
setupAndConnect(ws, req);
|
||||||
|
|
||||||
|
expect(console.warn).toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining("stale-session"),
|
||||||
|
);
|
||||||
|
expect(console.warn).toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining("PTY may be stale"),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not warn for fresh session reconnect", () => {
|
||||||
|
const ws = createMockWs();
|
||||||
|
const req = createMockReq("fresh-session");
|
||||||
|
|
||||||
|
// Session last active 1 minute ago (under the 5-minute threshold)
|
||||||
|
mockTerminalService.getSession.mockReturnValue({
|
||||||
|
id: "fresh-session",
|
||||||
|
shell: "/bin/bash",
|
||||||
|
cwd: "/project",
|
||||||
|
lastActivityAt: new Date(Date.now() - 60_000),
|
||||||
|
});
|
||||||
|
|
||||||
|
setupAndConnect(ws, req);
|
||||||
|
|
||||||
|
expect(console.warn).not.toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining("PTY may be stale"),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import { createApiRoutes } from "./routes.js";
|
|||||||
import { createSSE } from "./sse.js";
|
import { createSSE } from "./sse.js";
|
||||||
import { rateLimit, RATE_LIMITS } from "./rate-limit.js";
|
import { rateLimit, RATE_LIMITS } from "./rate-limit.js";
|
||||||
import { getOrCreateProjectStore, evictAllProjectStores } from "./project-store-resolver.js";
|
import { getOrCreateProjectStore, evictAllProjectStores } from "./project-store-resolver.js";
|
||||||
import { getTerminalService, type TerminalSession } from "./terminal-service.js";
|
import { getTerminalService, type TerminalSession, STALE_SESSION_THRESHOLD_MS } from "./terminal-service.js";
|
||||||
import { WebSocketServer, type WebSocket } from "ws";
|
import { WebSocketServer, type WebSocket } from "ws";
|
||||||
import { terminalSessionManager } from "./terminal.js";
|
import { terminalSessionManager } from "./terminal.js";
|
||||||
import { getCurrentGitHubRepo, parseBadgeUrl } from "./github.js";
|
import { getCurrentGitHubRepo, parseBadgeUrl } from "./github.js";
|
||||||
@@ -379,11 +379,22 @@ export function setupTerminalWebSocket(
|
|||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const MAX_MISSED_PONGS = 2; // Allow 2 missed pongs (~90s) before terminating
|
||||||
|
|
||||||
// Track if connection is alive
|
// Track if connection is alive
|
||||||
let isAlive = true;
|
let isAlive = true;
|
||||||
|
let missedPongs = 0; // Track consecutive missed pongs
|
||||||
let dataUnsub: (() => void) | null = null;
|
let dataUnsub: (() => void) | null = null;
|
||||||
let exitUnsub: (() => void) | null = null;
|
let exitUnsub: (() => void) | null = null;
|
||||||
|
|
||||||
|
// Detect potentially stale sessions on reconnect
|
||||||
|
const idleMs = Date.now() - session.lastActivityAt.getTime();
|
||||||
|
if (idleMs > STALE_SESSION_THRESHOLD_MS) {
|
||||||
|
console.warn(
|
||||||
|
`[terminal] Session ${sessionId} reconnect after ${Math.round(idleMs / 1000)}s idle — PTY may be stale`
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// Send scrollback buffer first
|
// Send scrollback buffer first
|
||||||
const scrollback = terminalService.getScrollbackAndClearPending(sessionId);
|
const scrollback = terminalService.getScrollbackAndClearPending(sessionId);
|
||||||
if (scrollback) {
|
if (scrollback) {
|
||||||
@@ -413,6 +424,8 @@ export function setupTerminalWebSocket(
|
|||||||
if (id === sessionId && isAlive) {
|
if (id === sessionId && isAlive) {
|
||||||
try {
|
try {
|
||||||
ws.send(JSON.stringify({ type: "exit", exitCode }));
|
ws.send(JSON.stringify({ type: "exit", exitCode }));
|
||||||
|
const idleSec = id ? Math.round((Date.now() - (terminalService.getSession(id)?.lastActivityAt?.getTime() ?? Date.now())) / 1000) : 0;
|
||||||
|
console.info(`[terminal] Session ${id} exited with code ${exitCode} (was ${idleSec}s idle)`);
|
||||||
} catch {
|
} catch {
|
||||||
// WebSocket might be closing
|
// WebSocket might be closing
|
||||||
}
|
}
|
||||||
@@ -422,9 +435,15 @@ export function setupTerminalWebSocket(
|
|||||||
// Heartbeat ping/pong
|
// Heartbeat ping/pong
|
||||||
const pingInterval = setInterval(() => {
|
const pingInterval = setInterval(() => {
|
||||||
if (!isAlive) {
|
if (!isAlive) {
|
||||||
|
missedPongs++;
|
||||||
|
if (missedPongs >= MAX_MISSED_PONGS) {
|
||||||
|
console.warn(`[terminal] Connection dead after ${missedPongs} missed pongs, terminating`);
|
||||||
ws.terminate();
|
ws.terminate();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
console.info(`[terminal] Missed pong #${missedPongs}, waiting for response...`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
isAlive = false;
|
isAlive = false;
|
||||||
try {
|
try {
|
||||||
ws.send(JSON.stringify({ type: "ping" }));
|
ws.send(JSON.stringify({ type: "ping" }));
|
||||||
@@ -435,6 +454,7 @@ export function setupTerminalWebSocket(
|
|||||||
|
|
||||||
ws.on("pong", () => {
|
ws.on("pong", () => {
|
||||||
isAlive = true;
|
isAlive = true;
|
||||||
|
missedPongs = 0; // Reset on successful pong
|
||||||
});
|
});
|
||||||
|
|
||||||
ws.on("message", (message: Buffer) => {
|
ws.on("message", (message: Buffer) => {
|
||||||
@@ -457,6 +477,7 @@ export function setupTerminalWebSocket(
|
|||||||
break;
|
break;
|
||||||
case "pong":
|
case "pong":
|
||||||
isAlive = true;
|
isAlive = true;
|
||||||
|
missedPongs = 0; // Reset on successful pong
|
||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
} catch {
|
} catch {
|
||||||
|
|||||||
Reference in New Issue
Block a user