ouyangbo/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 9 Packages Projects Releases Wiki Activity

fix(server): move blocking credential-pool calls off the event loop

Browse Source 
Network off (unplugged) froze the backend 17 minutes: the async
/api/credentials/pool endpoints (GET/POST/DELETE) called load_pool()
synchronously on the event-loop thread -> Copilot token exchange ->
blocking urlopen -> getaddrinfo stuck in C for 1016s, immune to
urlopen(timeout=10). WS dropped (1006), sessions detached, even log
writes stalled.

Fix 1 (web_server.py): move the three endpoint bodies into
asyncio.to_thread, matching the file's existing _run pattern.

Fix 2 (copilot_auth.py): _urlopen_bounded() runs the request in a
daemon thread with a wall-clock hard cap (timeout+5s) so DNS hangs
can no longer block any caller indefinitely.

Measured: simulated DNS hang now raises TimeoutError after 6.0s
instead of freezing the loop.
This commit is contained in:
peetteerr
2026-08-22 07:42:45 +08:00
committed by kshitij
parent 2b4e70ec07
commit ff0afff0e4
2 changed files with 154 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files
hermes_cli/copilot_auth.py
+44 -1
View File
@@ -23,6 +23,7 @@ import logging
import os
import shutil
import subprocess
import threading
import time
from pathlib import Path
from typing import Optional
@@ -520,6 +521,48 @@ def _save_jwt_to_disk(
logger.debug("Failed to persist Copilot JWT: %s", exc)
# Hard wall-clock cap for the token-exchange HTTP call. urllib's ``timeout``
# only bounds socket operations AFTER DNS resolution succeeds; getaddrinfo
# blocks in C and ignores it entirely, so on a networkless Windows host the
# resolver can hang for many minutes (observed: a 17-minute event-loop stall
# on 2026-08-22 that took the whole backend down with it).
_DNS_GRACE_SECONDS = 5.0
def _urlopen_bounded(req, timeout: float):
"""urlopen() with a hard wall-clock cap of timeout + _DNS_GRACE_SECONDS.
Runs the call on a daemon thread and abandons it if the cap fires, so a
DNS/getaddrinfo hang cannot block the caller indefinitely. Raises the
worker's exception, or TimeoutError when the cap fires.
"""
import urllib.request
box: dict = {}
def _worker() -> None:
try:
box["resp"] = urllib.request.urlopen(req, timeout=timeout)
except BaseException as exc: # re-raised on the caller's thread
box["exc"] = exc
t = threading.Thread(
target=_worker, name="copilot-token-exchange", daemon=True
)
t.start()
t.join(timeout + _DNS_GRACE_SECONDS)
if t.is_alive():
raise TimeoutError(
"copilot token exchange exceeded hard cap of "
f"{timeout + _DNS_GRACE_SECONDS:.0f}s (DNS/getaddrinfo hang?)"
)
if "exc" in box:
raise box["exc"]
if "resp" not in box:
raise TimeoutError("copilot token exchange worker died without result")
return box["resp"]
def exchange_copilot_token(raw_token: str, *, timeout: float = 10.0) -> tuple[str, float, Optional[str]]:
"""Exchange a raw GitHub token for a short-lived Copilot API token.
@@ -593,7 +636,7 @@ def exchange_copilot_token(raw_token: str, *, timeout: float = 10.0) -> tuple[st
permanent_failure = False
for attempt in range(_EXCHANGE_MAX_ATTEMPTS):
try:
with urllib.request.urlopen(req, timeout=timeout) as resp:
with _urlopen_bounded(req, timeout) as resp:
data = json.loads(resp.read().decode())
break
except Exception as exc: # noqa: BLE001 — retry all, re-raise below
hermes_cli/web_server.py
+110 -89
View File
@@ -14386,26 +14386,35 @@ async def list_credential_pool():
from agent.credential_pool import load_pool
from hermes_cli.auth import read_credential_pool
providers = []
# read_credential_pool(None) lists every provider that has pooled entries;
# load_pool() then gives us the rich PooledCredential objects per provider.
raw_pool = read_credential_pool()
for provider_id in sorted(raw_pool.keys()):
try:
pool = load_pool(provider_id)
except Exception:
_log.exception("load_pool(%s) failed", provider_id)
continue
entries = pool.entries()
if not entries:
continue
providers.append({
"provider": provider_id,
"entries": [
_pool_entry_summary(e, i) for i, e in enumerate(entries, start=1)
],
})
return {"providers": providers}
# load_pool() may hit the network synchronously (Copilot token exchange
# over raw urllib). urllib's timeout does NOT bound DNS resolution
# (getaddrinfo blocks in C), so on a networkless Windows host this froze
# the uvicorn event loop for 17 minutes (2026-08-22 00:03-00:20 stall).
# Keep every provider load off the loop - same pattern as
# get_memory_status below.
def _run():
providers = []
# read_credential_pool(None) lists every provider that has pooled entries;
# load_pool() then gives us the rich PooledCredential objects per provider.
raw_pool = read_credential_pool()
for provider_id in sorted(raw_pool.keys()):
try:
pool = load_pool(provider_id)
except Exception:
_log.exception("load_pool(%s) failed", provider_id)
continue
entries = pool.entries()
if not entries:
continue
providers.append({
"provider": provider_id,
"entries": [
_pool_entry_summary(e, i) for i, e in enumerate(entries, start=1)
],
})
return {"providers": providers}
return await asyncio.to_thread(_run)
@app.post("/api/credentials/pool")
@@ -14424,40 +14433,46 @@ async def add_credential_pool_entry(body: CredentialPoolAdd):
if not provider or not api_key:
raise HTTPException(status_code=400, detail="provider and api_key are required")
try:
pool = load_pool(provider)
label = (body.label or "").strip() or f"key #{len(pool.entries()) + 1}"
entry = PooledCredential(
provider=provider,
id=_uuid.uuid4().hex[:6],
label=label,
auth_type=AUTH_TYPE_API_KEY,
priority=0,
source=SOURCE_MANUAL,
access_token=api_key,
)
pool.add_entry(entry)
# Re-adding a credential is an explicit re-engagement signal: lift
# every suppression for this provider so a source deleted earlier
# (via DELETE below or `hermes auth remove`) can seed again.
# Mirrors the `hermes auth add` behaviour in auth_commands.py.
if not provider.startswith(CUSTOM_POOL_PREFIX):
try:
from hermes_cli.auth import (
_load_auth_store,
unsuppress_credential_source,
)
suppressed = _load_auth_store().get("suppressed_sources", {})
for src in list(suppressed.get(provider, []) or []):
unsuppress_credential_source(provider, src)
except Exception:
_log.exception("unsuppress after pool add failed (non-fatal)")
except HTTPException:
raise
except Exception as exc:
_log.exception("POST /api/credentials/pool failed")
raise HTTPException(status_code=400, detail=str(exc)) from exc
return {"ok": True, "provider": provider, "count": len(pool.entries())}
# load_pool() may run synchronous OAuth token exchanges (network I/O);
# keep it off the event loop - see list_credential_pool (2026-08-22
# 17-minute stall fix).
def _run():
try:
pool = load_pool(provider)
label = (body.label or "").strip() or f"key #{len(pool.entries()) + 1}"
entry = PooledCredential(
provider=provider,
id=_uuid.uuid4().hex[:6],
label=label,
auth_type=AUTH_TYPE_API_KEY,
priority=0,
source=SOURCE_MANUAL,
access_token=api_key,
)
pool.add_entry(entry)
# Re-adding a credential is an explicit re-engagement signal: lift
# every suppression for this provider so a source deleted earlier
# (via DELETE below or `hermes auth add`) can seed again.
# Mirrors the `hermes auth add` behaviour in auth_commands.py.
if not provider.startswith(CUSTOM_POOL_PREFIX):
try:
from hermes_cli.auth import (
_load_auth_store,
unsuppress_credential_source,
)
suppressed = _load_auth_store().get("suppressed_sources", {})
for src in list(suppressed.get(provider, []) or []):
unsuppress_credential_source(provider, src)
except Exception:
_log.exception("unsuppress after pool add failed (non-fatal)")
return {"ok": True, "provider": provider, "count": len(pool.entries())}
except HTTPException:
raise
except Exception as exc:
_log.exception("POST /api/credentials/pool failed")
raise HTTPException(status_code=400, detail=str(exc)) from exc
return await asyncio.to_thread(_run)
@app.delete("/api/credentials/pool/{provider}/{index}")
@@ -14478,44 +14493,50 @@ async def remove_credential_pool_entry(provider: str, index: int):
from hermes_cli.auth import suppress_credential_source
provider = (provider or "").strip().lower()
try:
pool = load_pool(provider)
removed = pool.remove_index(index)
except Exception as exc:
_log.exception("DELETE /api/credentials/pool failed")
raise HTTPException(status_code=400, detail=str(exc)) from exc
if removed is None:
raise HTTPException(status_code=404, detail="No pool entry at that index")
cleaned: List[str] = []
hints: List[str] = []
step = find_removal_step(provider, removed.source or "")
if step is not None:
# load_pool() may run synchronous token exchanges and the removal steps do
# blocking disk writes - keep them off the event loop (see
# list_credential_pool; 2026-08-22 17-minute stall fix).
def _run():
try:
result = step.remove_fn(provider, removed)
cleaned = list(result.cleaned)
hints = list(result.hints)
if result.suppress:
suppress_credential_source(provider, removed.source)
except Exception:
# Cleanup is best-effort, but suppression is the actual bug fix —
# without it the entry resurrects on the next load_pool(). Apply
# it even when source-specific cleanup blew up.
_log.exception(
"credential source cleanup failed for %s/%s; suppressing anyway",
provider, removed.source,
)
pool = load_pool(provider)
removed = pool.remove_index(index)
except Exception as exc:
_log.exception("DELETE /api/credentials/pool failed")
raise HTTPException(status_code=400, detail=str(exc)) from exc
if removed is None:
raise HTTPException(status_code=404, detail="No pool entry at that index")
cleaned: List[str] = []
hints: List[str] = []
step = find_removal_step(provider, removed.source or "")
if step is not None:
try:
suppress_credential_source(provider, removed.source)
result = step.remove_fn(provider, removed)
cleaned = list(result.cleaned)
hints = list(result.hints)
if result.suppress:
suppress_credential_source(provider, removed.source)
except Exception:
_log.exception("suppress_credential_source failed")
return {
"ok": True,
"provider": provider,
"count": len(pool.entries()),
"cleaned": cleaned,
"hints": hints,
}
# Cleanup is best-effort, but suppression is the actual bug fix -
# without it the entry resurrects on the next load_pool(). Apply
# it even when source-specific cleanup blew up.
_log.exception(
"credential source cleanup failed for %s/%s; suppressing anyway",
provider, removed.source,
)
try:
suppress_credential_source(provider, removed.source)
except Exception:
_log.exception("suppress_credential_source failed")
return {
"ok": True,
"provider": provider,
"count": len(pool.entries()),
"cleaned": cleaned,
"hints": hints,
}
return await asyncio.to_thread(_run)
# ---------------------------------------------------------------------------