fix: WebSocket reconnect awaits /connect POST before creating WS
The /connect POST that respawns ttyd was fire-and-forget — fetch fired and the next line immediately created a new WebSocket. The WS raced ttyd's startup and lost every time, staying stuck on 'Reconnecting...'. Fix: extract WS creation into _connectWebSocket(). When _reconnectAttempts >= 2, chain via .then() after the /connect fetch + 800ms settle delay for ttyd to bind its port, then return early to prevent fallthrough. When < 2, call _connectWebSocket() directly (normal reconnect — ttyd is still alive).
This commit is contained in:
@@ -53,27 +53,18 @@ function connectWebSocket(name, sourceUrl) {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
function connect() {
|
// _connectWebSocket — creates the WebSocket instance and registers all event handlers.
|
||||||
// 'tty' subprotocol is REQUIRED — without it ttyd never starts the PTY.
|
// Called directly for normal reconnects (ttyd still alive), or after a brief delay
|
||||||
// Confirmed via raw Python WebSocket tests: ttyd accepts the TCP upgrade but
|
// following the /connect POST (ttyd was dead and needed respawning).
|
||||||
// sits completely silent (no child process spawned) when subprotocol is omitted.
|
|
||||||
//
|
//
|
||||||
// Local const `ws` captures this specific instance so each handler can check
|
// Local const `ws` captures this specific instance so each handler can check
|
||||||
// `if (ws !== _ws) return;` (stale guard). Without it, rapid reconnects or
|
// `if (ws !== _ws) return;` (stale guard). Without it, rapid reconnects or
|
||||||
// session switches cause old handlers to fire on the new _ws while it is still
|
// session switches cause old handlers to fire on the new _ws while it is still
|
||||||
// CONNECTING → send error → close → reconnect → infinite loop (Bug 2).
|
// CONNECTING → send error → close → reconnect → infinite loop (Bug 2).
|
||||||
|
function _connectWebSocket() {
|
||||||
// After 2 failed WS attempts, ttyd is likely dead (e.g. after service restart).
|
// 'tty' subprotocol is REQUIRED — without it ttyd never starts the PTY.
|
||||||
// Fire-and-forget POST to /api/sessions/{name}/connect to respawn ttyd before
|
// Confirmed via raw Python WebSocket tests: ttyd accepts the TCP upgrade but
|
||||||
// attempting the next WebSocket connection. fetch() includes cookies automatically
|
// sits completely silent (no child process spawned) when subprotocol is omitted.
|
||||||
// for same-origin requests so auth is handled transparently.
|
|
||||||
if (_reconnectAttempts >= 2 && _currentSession) {
|
|
||||||
fetch('/api/sessions/' + encodeURIComponent(_currentSession) + '/connect', {
|
|
||||||
method: 'POST',
|
|
||||||
headers: { 'Content-Type': 'application/json' },
|
|
||||||
}).catch(function() {}); // ignore errors — the WS retry will catch the result
|
|
||||||
}
|
|
||||||
|
|
||||||
const ws = new WebSocket(url, ['tty']);
|
const ws = new WebSocket(url, ['tty']);
|
||||||
_ws = ws;
|
_ws = ws;
|
||||||
ws.binaryType = 'arraybuffer';
|
ws.binaryType = 'arraybuffer';
|
||||||
@@ -126,6 +117,30 @@ function connectWebSocket(name, sourceUrl) {
|
|||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function connect() {
|
||||||
|
// After 2 failed WS attempts, ttyd is likely dead (e.g. after service restart).
|
||||||
|
// AWAIT the /connect POST before opening the WebSocket — ttyd must be alive first.
|
||||||
|
// fetch() includes cookies automatically for same-origin requests so auth is transparent.
|
||||||
|
//
|
||||||
|
// Critical: this path uses .then() so _connectWebSocket() runs only AFTER the POST
|
||||||
|
// response (plus an 800ms settle delay for ttyd to bind its port). The early return
|
||||||
|
// prevents falling through to the direct _connectWebSocket() call below.
|
||||||
|
if (_reconnectAttempts >= 2 && _currentSession) {
|
||||||
|
fetch('/api/sessions/' + encodeURIComponent(_currentSession) + '/connect', {
|
||||||
|
method: 'POST',
|
||||||
|
headers: { 'Content-Type': 'application/json' },
|
||||||
|
})
|
||||||
|
.catch(function() { return null; })
|
||||||
|
.then(function() {
|
||||||
|
// Brief delay for ttyd to bind its port after /connect spawns it
|
||||||
|
setTimeout(_connectWebSocket, 800);
|
||||||
|
});
|
||||||
|
return; // Don't fall through — .then() handles the WebSocket creation
|
||||||
|
}
|
||||||
|
|
||||||
|
_connectWebSocket();
|
||||||
|
}
|
||||||
|
|
||||||
connect();
|
connect();
|
||||||
}
|
}
|
||||||
function initVisualViewport() {
|
function initVisualViewport() {
|
||||||
|
|||||||
@@ -816,6 +816,21 @@ test('terminal.js WebSocket reconnect calls /connect after 2 failed attempts', (
|
|||||||
assert.ok(source.includes('Math.pow'), 'must use exponential backoff');
|
assert.ok(source.includes('Math.pow'), 'must use exponential backoff');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('terminal.js WebSocket reconnect awaits /connect before creating WS', () => {
|
||||||
|
const source = fs.readFileSync(new URL('../terminal.js', import.meta.url), 'utf8');
|
||||||
|
assert.ok(source.includes('_reconnectAttempts'), 'must track reconnect attempts');
|
||||||
|
// WS creation must be extracted into a separate helper — not inlined in connect()
|
||||||
|
assert.ok(source.includes('_connectWebSocket'), 'must extract WS creation into _connectWebSocket helper');
|
||||||
|
// The /connect fetch must use .then() to chain WS creation — not fire-and-forget
|
||||||
|
assert.ok(source.includes('.then('), '/connect fetch must chain via .then() before WS creation');
|
||||||
|
// connect() must return after scheduling the fetch chain, to prevent falling through to immediate WS creation
|
||||||
|
const connectFn = source.substring(
|
||||||
|
source.indexOf('function connect()'),
|
||||||
|
source.indexOf('function _connectWebSocket'),
|
||||||
|
);
|
||||||
|
assert.ok(connectFn.includes('return;'), 'connect() must return after fetch to prevent falling through to immediate WS creation');
|
||||||
|
});
|
||||||
|
|
||||||
// --- Issue 4: setTerminalFontSize ---
|
// --- Issue 4: setTerminalFontSize ---
|
||||||
|
|
||||||
test('terminal.js exposes window._setTerminalFontSize function', () => {
|
test('terminal.js exposes window._setTerminalFontSize function', () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user