Skip to content

Commit fa7963a

Browse files
authored
fix(agent-proxy): survive a client that drops during agent-VM startup (#10880)
agent_ws accepts the client socket before booting the user's agent VM, and _wait_for_vm_healthy polls for up to 120s — far past uvicorn's WebSocket keepalive window. When the phone is dropped mid-wait ("sent 1011 keepalive ping timeout"), the startup path's unguarded send_text/close raised WebSocketDisconnect straight out of the handler: 24 "Exception in ASGI application" tracebacks per day on prod agent-proxy, every one of them at the "Agent VM is not responding" send. The relay loop below already owned this condition (except Exception plus a guarded final close); the startup path did not. Route every startup status, error, and close through one client-liveness seam that treats peer-gone as the terminal, expected end of the connection, so the handler still emits its intended 4002/4003 close code and returns cleanly. Verified: backend/tests/unit/test_agent_proxy_startup_client_gone.py drives the real agent_ws against a socket that raises WebSocketDisconnect on every write — both cases fail with the production exception on the parent commit and pass here; the sibling agent-proxy suite stays green (8 passed). Failure-Class: FC-peer-close-aborts-owed-write
1 parent 3c9e830 commit fa7963a

2 files changed

Lines changed: 146 additions & 10 deletions

File tree

backend/agent-proxy/main.py

Lines changed: 35 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,31 @@ async def _wait_for_vm_healthy(vm_ip: str, auth_token: str, timeout: float = 120
426426
return False
427427

428428

429+
async def _send_startup_event(websocket: WebSocket, uid: str, payload: Dict[str, Any]) -> bool:
430+
"""Push a VM-startup status/error event to the client. False means the client is gone.
431+
432+
VM startup outlives the client's keepalive window (the health wait alone is 120s), so by
433+
the time these events go out uvicorn may already have dropped the phone with a 1011 ping
434+
timeout. A vanished client is the terminal, expected end of this connection — not an ASGI
435+
error — and must not escape agent_ws, which is the only ownership the relay loop below
436+
already has and this startup path did not.
437+
"""
438+
try:
439+
await websocket.send_text(json.dumps(payload))
440+
return True
441+
except Exception as e:
442+
logger.info(f"[agent-proxy] uid={uid} client gone during VM startup: {type(e).__name__}")
443+
return False
444+
445+
446+
async def _close_client(websocket: WebSocket, uid: str, code: int, reason: str) -> None:
447+
"""Close the client socket, tolerating a client that already went away."""
448+
try:
449+
await websocket.close(code=code, reason=reason)
450+
except Exception as e:
451+
logger.debug(f"[agent-proxy] uid={uid} close({code}) on gone client: {type(e).__name__}")
452+
453+
429454
# --------------- encryption helpers ---------------
430455

431456

@@ -624,34 +649,34 @@ async def agent_ws(websocket: WebSocket):
624649
except Exception:
625650
# VM not reachable — check GCE and restart/reset if needed
626651
logger.info(f"[agent-proxy] uid={uid} VM {vm_ip} not reachable, checking GCE...")
627-
await websocket.send_text(json.dumps({"type": "status", "message": "Starting your agent VM..."}))
652+
await _send_startup_event(websocket, uid, {"type": "status", "message": "Starting your agent VM..."})
628653
vm = await _ensure_vm_running(uid, vm, health_failed=True)
629654
if not vm or vm.get("status") != "ready" or not vm.get("ip"):
630-
await websocket.send_text(json.dumps(await run_blocking(db_executor, _vm_unavailable_event, uid)))
631-
await websocket.close(code=4002, reason="VM startup failed")
655+
await _send_startup_event(websocket, uid, await run_blocking(db_executor, _vm_unavailable_event, uid))
656+
await _close_client(websocket, uid, 4002, "VM startup failed")
632657
return
633658
vm_ip = vm["ip"]
634659
vm_token = vm["authToken"]
635660
# Wait for VM to be healthy after restart
636661
healthy = await _wait_for_vm_healthy(vm_ip, vm_token)
637662
if not healthy:
638-
await websocket.send_text(json.dumps({"type": "error", "message": "Agent VM is not responding"}))
639-
await websocket.close(code=4003, reason="VM not healthy")
663+
await _send_startup_event(websocket, uid, {"type": "error", "message": "Agent VM is not responding"})
664+
await _close_client(websocket, uid, 4003, "VM not healthy")
640665
return
641666
else:
642667
# No IP or not ready — must restart
643-
await websocket.send_text(json.dumps({"type": "status", "message": "Starting your agent VM..."}))
668+
await _send_startup_event(websocket, uid, {"type": "status", "message": "Starting your agent VM..."})
644669
vm = await _ensure_vm_running(uid, vm)
645670
if not vm or vm.get("status") != "ready" or not vm.get("ip"):
646-
await websocket.send_text(json.dumps(await run_blocking(db_executor, _vm_unavailable_event, uid)))
647-
await websocket.close(code=4002, reason="VM startup failed")
671+
await _send_startup_event(websocket, uid, await run_blocking(db_executor, _vm_unavailable_event, uid))
672+
await _close_client(websocket, uid, 4002, "VM startup failed")
648673
return
649674
vm_ip = vm["ip"]
650675
vm_token = vm["authToken"]
651676
healthy = await _wait_for_vm_healthy(vm_ip, vm_token)
652677
if not healthy:
653-
await websocket.send_text(json.dumps({"type": "error", "message": "Agent VM is not responding"}))
654-
await websocket.close(code=4003, reason="VM not healthy")
678+
await _send_startup_event(websocket, uid, {"type": "error", "message": "Agent VM is not responding"})
679+
await _close_client(websocket, uid, 4003, "VM not healthy")
655680
return
656681

657682
vm_uri = f"ws://{vm_ip}:8080/ws?token={vm_token}"
Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
"""A client that vanishes during agent-VM startup must end the connection, not crash agent_ws.
2+
3+
``agent_ws`` accepts the client socket and only then boots/waits for the user's VM —
4+
``_wait_for_vm_healthy`` alone polls for 120s, far past uvicorn's WebSocket keepalive
5+
window. When the phone is dropped mid-wait (``sent 1011 keepalive ping timeout``), the
6+
startup path's unguarded ``send_text``/``close`` raised ``WebSocketDisconnect`` straight
7+
out of the handler: 24 "Exception in ASGI application" tracebacks per day on prod
8+
agent-proxy, all at the "Agent VM is not responding" send. The relay loop below already
9+
owned this (``except Exception`` + guarded close); the startup path did not.
10+
"""
11+
12+
import importlib.util
13+
import sys
14+
import types
15+
from pathlib import Path
16+
from types import ModuleType
17+
from unittest.mock import MagicMock
18+
19+
import firebase_admin
20+
import pytest
21+
from fastapi import WebSocketDisconnect
22+
from firebase_admin import firestore
23+
24+
BACKEND_DIR = Path(__file__).resolve().parents[2]
25+
AGENT_PROXY_DIR = BACKEND_DIR / "agent-proxy"
26+
if str(BACKEND_DIR) not in sys.path:
27+
sys.path.insert(0, str(BACKEND_DIR))
28+
# `agent-proxy/main.py` imports its siblings by bare name (`from resilience import ...`),
29+
# so loading it by file path also needs its own directory importable.
30+
if str(AGENT_PROXY_DIR) not in sys.path:
31+
sys.path.insert(0, str(AGENT_PROXY_DIR))
32+
33+
34+
@pytest.fixture
35+
def agent_proxy(monkeypatch) -> ModuleType:
36+
monkeypatch.delenv("GOOGLE_APPLICATION_CREDENTIALS", raising=False)
37+
monkeypatch.setattr(firebase_admin, "initialize_app", MagicMock(return_value=object()))
38+
monkeypatch.setattr(firestore, "client", MagicMock(return_value=object()))
39+
40+
spec = importlib.util.spec_from_file_location("agent_proxy_startup_client_gone_test", AGENT_PROXY_DIR / "main.py")
41+
assert spec is not None and spec.loader is not None
42+
module = importlib.util.module_from_spec(spec)
43+
spec.loader.exec_module(module)
44+
return module
45+
46+
47+
class _GoneClientWebSocket:
48+
"""A client socket that uvicorn already dropped: every write reports the disconnect."""
49+
50+
def __init__(self) -> None:
51+
self.headers = {"authorization": "Bearer test-token"}
52+
self.accepted = False
53+
self.send_attempts: list[str] = []
54+
self.close_attempts: list[int] = []
55+
56+
async def accept(self) -> None:
57+
self.accepted = True
58+
59+
async def send_text(self, text: str) -> None:
60+
self.send_attempts.append(text)
61+
raise WebSocketDisconnect(code=1006)
62+
63+
async def close(self, code: int = 1000, reason: str = "") -> None:
64+
self.close_attempts.append(code)
65+
raise WebSocketDisconnect(code=1006)
66+
67+
68+
def _stub_startup(agent_proxy: ModuleType, monkeypatch, *, ensure_result, healthy: bool) -> None:
69+
"""Drive agent_ws down the restart path with an authenticated uid and no live VM."""
70+
71+
async def direct_run_blocking(_executor, func, *args, **kwargs):
72+
return func(*args, **kwargs)
73+
74+
async def ensure_vm_running(_uid, _vm, health_failed=False):
75+
return ensure_result
76+
77+
async def wait_for_vm_healthy(_ip, _token, timeout=120):
78+
return healthy
79+
80+
monkeypatch.setattr(agent_proxy, "run_blocking", direct_run_blocking)
81+
monkeypatch.setattr(agent_proxy, "_verify_id_token", lambda _token: {"uid": "uid-gone"})
82+
monkeypatch.setattr(
83+
agent_proxy,
84+
"_get_user_context",
85+
lambda _uid: ({"vmName": "omi-agent-gone", "zone": "us-central1-a", "status": "stopped"}, "standard"),
86+
)
87+
monkeypatch.setattr(agent_proxy, "_ensure_vm_running", ensure_vm_running)
88+
monkeypatch.setattr(agent_proxy, "_wait_for_vm_healthy", wait_for_vm_healthy)
89+
monkeypatch.setattr(agent_proxy, "_vm_unavailable_event", lambda _uid: {"type": "error", "code": "unavailable"})
90+
monkeypatch.setattr(agent_proxy, "httpx", types.SimpleNamespace(AsyncClient=MagicMock()))
91+
92+
93+
class TestAgentWsStartupSurvivesAGoneClient:
94+
async def test_unavailable_vm_does_not_raise_when_the_client_is_gone(self, agent_proxy, monkeypatch):
95+
_stub_startup(agent_proxy, monkeypatch, ensure_result=None, healthy=False)
96+
websocket = _GoneClientWebSocket()
97+
98+
await agent_proxy.agent_ws(websocket)
99+
100+
assert websocket.accepted
101+
assert websocket.close_attempts == [4002], "the connection must still be closed with its startup-failure code"
102+
103+
async def test_unhealthy_vm_does_not_raise_when_the_client_is_gone(self, agent_proxy, monkeypatch):
104+
ready_vm = {"vmName": "omi-agent-gone", "status": "ready", "ip": "34.9.9.9", "authToken": "vm-token"}
105+
_stub_startup(agent_proxy, monkeypatch, ensure_result=ready_vm, healthy=False)
106+
websocket = _GoneClientWebSocket()
107+
108+
await agent_proxy.agent_ws(websocket)
109+
110+
assert [s for s in websocket.send_attempts if "not responding" in s], "the error event must still be attempted"
111+
assert websocket.close_attempts == [4003], "the connection must still be closed with its not-healthy code"

0 commit comments

Comments
 (0)