fix(terminal): prevent xterm.js crash, websocket disconnect cascade, stale sessions
Three related bugs fixed: 1. Frontend xterm.js crash: TerminalPage rendered ALL sessions with display:none for inactive ones. xterm.js crashes when initialized in a hidden container (Viewport can't read dimensions). Fix: only render the active session's TerminalComponent using conditional rendering. 2. Backend websocket disconnect cascade: When client disconnected (due to #1), the server tried to send 'connected' status on dead socket, caught the WebSocketDisconnect in a generic except block, then tried to close() again causing RuntimeError. Fix: catch WebSocketDisconnect specifically and suppress close() errors. 3. Stale DB sessions: After server restart, DB still had old terminal session rows but no in-memory sessions. list_terminal_sessions returned these ghosts, causing the frontend to render dead tabs. Fix: skip DB-only sessions that have no live in-memory counterpart. Quality gates: pytest (15/15 passed), tsc clean, vitest (7/7 passed)
This commit is contained in:
+1
-1
File diff suppressed because one or more lines are too long
@@ -9,6 +9,7 @@ from contextlib import suppress
|
|||||||
from fastapi import APIRouter, Depends, HTTPException, WebSocket, status
|
from fastapi import APIRouter, Depends, HTTPException, WebSocket, status
|
||||||
from sqlalchemy import select
|
from sqlalchemy import select
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
from starlette.websockets import WebSocketDisconnect
|
||||||
|
|
||||||
from src.auth.dependencies import get_current_user_id, get_db_session
|
from src.auth.dependencies import get_current_user_id, get_db_session
|
||||||
from src.models.terminal_session import TerminalSessionModel
|
from src.models.terminal_session import TerminalSessionModel
|
||||||
@@ -207,6 +208,8 @@ async def _handle_terminal_websocket(
|
|||||||
for task in pending:
|
for task in pending:
|
||||||
task.cancel()
|
task.cancel()
|
||||||
|
|
||||||
|
except WebSocketDisconnect:
|
||||||
|
logger.debug("WebSocket disconnected for instance %s", instance_id)
|
||||||
except Exception as exc:
|
except Exception as exc:
|
||||||
logger.error(
|
logger.error(
|
||||||
"Terminal session error for instance %s: %s",
|
"Terminal session error for instance %s: %s",
|
||||||
@@ -214,7 +217,8 @@ async def _handle_terminal_websocket(
|
|||||||
str(exc),
|
str(exc),
|
||||||
exc_info=True,
|
exc_info=True,
|
||||||
)
|
)
|
||||||
await websocket.close(code=4000, reason=f"Error: {exc}")
|
with suppress(Exception):
|
||||||
|
await websocket.close(code=4000, reason=f"Error: {exc}")
|
||||||
finally:
|
finally:
|
||||||
# Detach WebSocket, don't kill session
|
# Detach WebSocket, don't kill session
|
||||||
with suppress(Exception):
|
with suppress(Exception):
|
||||||
@@ -403,18 +407,20 @@ async def list_terminal_sessions(
|
|||||||
)
|
)
|
||||||
db_rows = result.scalars().all()
|
db_rows = result.scalars().all()
|
||||||
|
|
||||||
# Build response with live has_websockets flag
|
# Build response with live has_websockets flag.
|
||||||
|
# Skip DB-only sessions that have no live in-memory counterpart
|
||||||
|
# (e.g. after server restart).
|
||||||
sessions = []
|
sessions = []
|
||||||
for row in db_rows:
|
for row in db_rows:
|
||||||
live_session = terminal_manager.get_session(str(instance_id), str(row.id))
|
live_session = terminal_manager.get_session(str(instance_id), str(row.id))
|
||||||
|
if not live_session:
|
||||||
|
continue
|
||||||
sessions.append(
|
sessions.append(
|
||||||
{
|
{
|
||||||
"id": str(row.id),
|
"id": str(row.id),
|
||||||
"name": row.name,
|
"name": row.name,
|
||||||
"status": row.status,
|
"status": row.status,
|
||||||
"has_websockets": live_session.has_websockets()
|
"has_websockets": live_session.has_websockets(),
|
||||||
if live_session
|
|
||||||
else False,
|
|
||||||
"created_at": row.created_at.isoformat() if row.created_at else None,
|
"created_at": row.created_at.isoformat() if row.created_at else None,
|
||||||
"last_activity_at": row.last_activity_at.isoformat()
|
"last_activity_at": row.last_activity_at.isoformat()
|
||||||
if row.last_activity_at
|
if row.last_activity_at
|
||||||
|
|||||||
+1
-1
File diff suppressed because one or more lines are too long
@@ -229,23 +229,22 @@ export const TerminalPage: React.FC = () => {
|
|||||||
</div>
|
</div>
|
||||||
<div className="terminal-page-content">
|
<div className="terminal-page-content">
|
||||||
{error && <div className="terminal-error-banner">{error}</div>}
|
{error && <div className="terminal-error-banner">{error}</div>}
|
||||||
{sessions.map((session) => (
|
{sessions
|
||||||
<div
|
.filter((session) => session.id === activeSessionId)
|
||||||
key={session.id}
|
.map((session) => (
|
||||||
className={`terminal-instance ${session.id === activeSessionId ? "active" : ""}`}
|
<div
|
||||||
style={{
|
key={session.id}
|
||||||
display: session.id === activeSessionId ? "flex" : "none",
|
className="terminal-instance active"
|
||||||
}}
|
>
|
||||||
>
|
<TerminalComponent
|
||||||
<TerminalComponent
|
ref={terminalRefs.current[session.id]}
|
||||||
ref={terminalRefs.current[session.id]}
|
instanceId={instanceId}
|
||||||
instanceId={instanceId}
|
sessionId={session.id}
|
||||||
sessionId={session.id}
|
onClose={() => handleClose(session.id)}
|
||||||
onClose={() => handleClose(session.id)}
|
isMobile={true}
|
||||||
isMobile={true}
|
/>
|
||||||
/>
|
</div>
|
||||||
</div>
|
))}
|
||||||
))}
|
|
||||||
{sessions.length === 0 && !loading && (
|
{sessions.length === 0 && !loading && (
|
||||||
<div className="terminal-empty-state">
|
<div className="terminal-empty-state">
|
||||||
<p>No terminal sessions. Press Alt+Shift+N to create one.</p>
|
<p>No terminal sessions. Press Alt+Shift+N to create one.</p>
|
||||||
@@ -289,23 +288,22 @@ export const TerminalPage: React.FC = () => {
|
|||||||
/>
|
/>
|
||||||
<div className="terminal-page-content">
|
<div className="terminal-page-content">
|
||||||
{error && <div className="terminal-error-banner">{error}</div>}
|
{error && <div className="terminal-error-banner">{error}</div>}
|
||||||
{sessions.map((session) => (
|
{sessions
|
||||||
<div
|
.filter((session) => session.id === activeSessionId)
|
||||||
key={session.id}
|
.map((session) => (
|
||||||
className={`terminal-instance ${session.id === activeSessionId ? "active" : ""}`}
|
<div
|
||||||
style={{
|
key={session.id}
|
||||||
display: session.id === activeSessionId ? "flex" : "none",
|
className="terminal-instance active"
|
||||||
}}
|
>
|
||||||
>
|
<TerminalComponent
|
||||||
<TerminalComponent
|
ref={terminalRefs.current[session.id]}
|
||||||
ref={terminalRefs.current[session.id]}
|
instanceId={instanceId}
|
||||||
instanceId={instanceId}
|
sessionId={session.id}
|
||||||
sessionId={session.id}
|
onClose={() => handleClose(session.id)}
|
||||||
onClose={() => handleClose(session.id)}
|
isMobile={false}
|
||||||
isMobile={false}
|
/>
|
||||||
/>
|
</div>
|
||||||
</div>
|
))}
|
||||||
))}
|
|
||||||
{sessions.length === 0 && !loading && (
|
{sessions.length === 0 && !loading && (
|
||||||
<div className="terminal-empty-state">
|
<div className="terminal-empty-state">
|
||||||
<p>No terminal sessions. Press Alt+Shift+N to create one.</p>
|
<p>No terminal sessions. Press Alt+Shift+N to create one.</p>
|
||||||
|
|||||||
Reference in New Issue
Block a user