fix: harden persistent WebUI health checks
This commit is contained in:
140
api/routes.py
140
api/routes.py
@@ -12,6 +12,7 @@ import queue
|
||||
import re
|
||||
import platform
|
||||
import shutil
|
||||
import sqlite3
|
||||
import subprocess
|
||||
import sys
|
||||
import threading
|
||||
@@ -19,6 +20,7 @@ import time
|
||||
import uuid
|
||||
import re
|
||||
from pathlib import Path
|
||||
from contextlib import closing
|
||||
from urllib.parse import parse_qs
|
||||
from api.agent_sessions import MESSAGING_SOURCES
|
||||
|
||||
@@ -1222,6 +1224,7 @@ from api.models import (
|
||||
title_from,
|
||||
_write_session_index,
|
||||
SESSION_INDEX_FILE,
|
||||
_active_state_db_path,
|
||||
load_projects,
|
||||
save_projects,
|
||||
import_cli_session,
|
||||
@@ -1658,6 +1661,131 @@ def _handle_insights(handler, parsed) -> bool:
|
||||
# ── GET routes ────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def _accept_loop_health(handler) -> dict:
|
||||
server = getattr(handler, "server", None)
|
||||
return {
|
||||
"requests_total": int(getattr(server, "accept_loop_requests_total", 0) or 0),
|
||||
"last_request_at": round(float(getattr(server, "accept_loop_last_request_at", 0.0) or 0.0), 3),
|
||||
}
|
||||
|
||||
|
||||
def _streams_lock_health(timeout_seconds: float = 0.5) -> dict:
|
||||
t0 = time.time()
|
||||
acquired = STREAMS_LOCK.acquire(timeout=timeout_seconds)
|
||||
elapsed_ms = round((time.time() - t0) * 1000, 1)
|
||||
if not acquired:
|
||||
return {
|
||||
"status": "blocked",
|
||||
"timeout_seconds": timeout_seconds,
|
||||
"ms": elapsed_ms,
|
||||
}
|
||||
try:
|
||||
return {
|
||||
"status": "ok",
|
||||
"active_streams": len(STREAMS),
|
||||
"ms": elapsed_ms,
|
||||
}
|
||||
finally:
|
||||
STREAMS_LOCK.release()
|
||||
|
||||
|
||||
def _deep_health_checks() -> tuple[dict, bool]:
|
||||
"""Run cheap probes that exercise the state paths used by the UI shell.
|
||||
|
||||
Plain /health intentionally stays tiny. /health?deep=1 is for supervisors
|
||||
and watchdogs that need to know whether the process can still touch the
|
||||
shared stream map, sidebar/session path, project state, and Hermes state.db
|
||||
without hitting the RST-before-write failure mode from #1458.
|
||||
"""
|
||||
checks: dict[str, dict] = {}
|
||||
|
||||
checks["streams_lock"] = _streams_lock_health()
|
||||
if checks["streams_lock"].get("status") != "ok":
|
||||
return checks, False
|
||||
|
||||
t0 = time.time()
|
||||
try:
|
||||
sessions = all_sessions()
|
||||
checks["sessions"] = {
|
||||
"status": "ok",
|
||||
"count": len(sessions),
|
||||
"ms": round((time.time() - t0) * 1000, 1),
|
||||
}
|
||||
except Exception as exc:
|
||||
checks["sessions"] = {
|
||||
"status": "error",
|
||||
"error": type(exc).__name__,
|
||||
"ms": round((time.time() - t0) * 1000, 1),
|
||||
}
|
||||
|
||||
t0 = time.time()
|
||||
try:
|
||||
projects = load_projects(_migrate=False)
|
||||
checks["projects"] = {
|
||||
"status": "ok",
|
||||
"count": len(projects),
|
||||
"ms": round((time.time() - t0) * 1000, 1),
|
||||
}
|
||||
except Exception as exc:
|
||||
checks["projects"] = {
|
||||
"status": "error",
|
||||
"error": type(exc).__name__,
|
||||
"ms": round((time.time() - t0) * 1000, 1),
|
||||
}
|
||||
|
||||
t0 = time.time()
|
||||
try:
|
||||
db_path = _active_state_db_path()
|
||||
if not db_path.exists():
|
||||
checks["state_db"] = {
|
||||
"status": "missing",
|
||||
"ms": round((time.time() - t0) * 1000, 1),
|
||||
}
|
||||
else:
|
||||
with closing(sqlite3.connect(str(db_path))) as conn:
|
||||
conn.execute("PRAGMA schema_version").fetchone()
|
||||
checks["state_db"] = {
|
||||
"status": "ok",
|
||||
"ms": round((time.time() - t0) * 1000, 1),
|
||||
}
|
||||
except Exception as exc:
|
||||
checks["state_db"] = {
|
||||
"status": "error",
|
||||
"error": type(exc).__name__,
|
||||
"ms": round((time.time() - t0) * 1000, 1),
|
||||
}
|
||||
|
||||
healthy = all(
|
||||
check.get("status") in {"ok", "missing"}
|
||||
for check in checks.values()
|
||||
)
|
||||
return checks, healthy
|
||||
|
||||
|
||||
def _handle_health(handler, parsed):
|
||||
deep = parse_qs(parsed.query or "").get("deep", [""])[0].lower() in {"1", "true", "yes", "on"}
|
||||
stream_check = _streams_lock_health()
|
||||
payload = {
|
||||
"status": "ok" if stream_check.get("status") == "ok" else "degraded",
|
||||
"sessions": len(SESSIONS),
|
||||
"active_streams": int(stream_check.get("active_streams") or 0),
|
||||
"uptime_seconds": round(time.time() - SERVER_START_TIME, 1),
|
||||
"accept_loop": _accept_loop_health(handler),
|
||||
}
|
||||
if deep:
|
||||
if stream_check.get("status") != "ok":
|
||||
payload["checks"] = {"streams_lock": stream_check}
|
||||
return j(handler, payload, status=503)
|
||||
checks, healthy = _deep_health_checks()
|
||||
payload["checks"] = checks
|
||||
if not healthy:
|
||||
payload["status"] = "degraded"
|
||||
return j(handler, payload, status=503)
|
||||
if payload["status"] != "ok":
|
||||
return j(handler, payload, status=503)
|
||||
return j(handler, payload)
|
||||
|
||||
|
||||
def handle_get(handler, parsed) -> bool:
|
||||
"""Handle all GET routes. Returns True if handled, False for 404."""
|
||||
|
||||
@@ -1776,17 +1904,7 @@ def handle_get(handler, parsed) -> bool:
|
||||
return _handle_insights(handler, parsed)
|
||||
|
||||
if parsed.path == "/health":
|
||||
with STREAMS_LOCK:
|
||||
n_streams = len(STREAMS)
|
||||
return j(
|
||||
handler,
|
||||
{
|
||||
"status": "ok",
|
||||
"sessions": len(SESSIONS),
|
||||
"active_streams": n_streams,
|
||||
"uptime_seconds": round(time.time() - SERVER_START_TIME, 1),
|
||||
},
|
||||
)
|
||||
return _handle_health(handler, parsed)
|
||||
|
||||
if parsed.path == "/api/models":
|
||||
return j(handler, get_available_models())
|
||||
|
||||
@@ -235,3 +235,44 @@ PID PPID CMD
|
||||
If PPID is ``1`` (init) when it should be the supervisor, the orphan-server
|
||||
loop is happening — re-check that ``--foreground`` (or one of the env vars)
|
||||
is reaching the process.
|
||||
|
||||
## HTTP watchdog / deep health
|
||||
|
||||
``KeepAlive`` / ``Restart=always`` only recover a process that exits. If the
|
||||
process is still listening on the port but request handling is wedged, pair your
|
||||
supervisor with an HTTP probe and force a restart when the probe fails.
|
||||
|
||||
Hermes Web UI exposes two health levels:
|
||||
|
||||
- ``/health`` — cheap liveness probe with ``active_streams``, uptime, and an
|
||||
``accept_loop`` heartbeat counter.
|
||||
- ``/health?deep=1`` — readiness probe that briefly acquires the stream lock,
|
||||
reads the sidebar/session path, reads projects state, and touches Hermes
|
||||
``state.db`` if it exists. Use this for watchdogs.
|
||||
|
||||
At startup the server also tries to raise its file-descriptor soft limit to
|
||||
4096 on platforms that support ``RLIMIT_NOFILE``. That is defense in depth for
|
||||
persistent hosts: leaks should still be fixed, but a higher soft limit gives
|
||||
you more diagnostic headroom before request handling falls over.
|
||||
|
||||
Minimal macOS launchd watchdog script:
|
||||
|
||||
```bash
|
||||
#!/usr/bin/env bash
|
||||
set -euo pipefail
|
||||
LABEL="com.example.hermes-webui"
|
||||
BASE="http://127.0.0.1:8787"
|
||||
|
||||
if ! curl -fsS --max-time 10 "$BASE/health?deep=1" >/dev/null; then
|
||||
launchctl kickstart -k "gui/$(id -u)/$LABEL"
|
||||
fi
|
||||
```
|
||||
|
||||
Run it every few minutes from a separate ``StartInterval`` LaunchAgent. For
|
||||
systemd, prefer a timer/service pair that runs the same curl probe and
|
||||
``systemctl --user restart hermes-webui.service`` on failure.
|
||||
|
||||
The ``accept_loop.requests_total`` value should increase when probes arrive. If
|
||||
it stays flat while the process is still alive, the server accept loop is not
|
||||
making progress; capture logs/thread samples before restarting if you are
|
||||
collecting diagnostics for a bug report.
|
||||
|
||||
60
server.py
60
server.py
@@ -9,6 +9,11 @@ import sys
|
||||
import time
|
||||
import traceback
|
||||
from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer
|
||||
|
||||
try:
|
||||
import resource
|
||||
except ImportError: # pragma: no cover - resource is Unix-only
|
||||
resource = None
|
||||
from urllib.parse import urlparse
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -26,6 +31,22 @@ class QuietHTTPServer(ThreadingHTTPServer):
|
||||
"""Custom HTTP server that silently handles common network errors."""
|
||||
daemon_threads = True
|
||||
request_queue_size = 64
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
super().__init__(*args, **kwargs)
|
||||
self.accept_loop_requests_total = 0
|
||||
self.accept_loop_last_request_at = 0.0
|
||||
|
||||
def _handle_request_noblock(self):
|
||||
"""Record accept-loop progress before dispatching a request handler.
|
||||
|
||||
A process can be alive and still stop accepting/dispatching requests.
|
||||
Exposing this heartbeat on /health gives supervisors and watchdogs a
|
||||
cheap signal that the accept loop is still moving.
|
||||
"""
|
||||
self.accept_loop_requests_total += 1
|
||||
self.accept_loop_last_request_at = time.time()
|
||||
return super()._handle_request_noblock()
|
||||
|
||||
def handle_error(self, request, client_address):
|
||||
"""Override to suppress logging for common client disconnect errors."""
|
||||
@@ -129,11 +150,50 @@ class Handler(BaseHTTPRequestHandler):
|
||||
clear_request_profile()
|
||||
|
||||
|
||||
def _raise_fd_soft_limit(target: int = 4096) -> dict:
|
||||
"""Best-effort raise of RLIMIT_NOFILE for persistent WebUI hosts.
|
||||
|
||||
macOS launchd jobs often start with a 256 soft limit. If a future FD leak
|
||||
regresses, that low ceiling turns a leak into a hard HTTP wedge quickly.
|
||||
Raising the soft limit does not hide leaks; it buys enough headroom for
|
||||
diagnostics and watchdog recovery.
|
||||
"""
|
||||
if resource is None:
|
||||
return {"status": "unsupported"}
|
||||
try:
|
||||
soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE)
|
||||
except Exception as exc:
|
||||
return {"status": "error", "error": str(exc)}
|
||||
|
||||
# On Unix, RLIM_INFINITY is commonly a large int; keep the logic explicit
|
||||
# so tests can use ordinary integers without depending on platform values.
|
||||
desired = int(target)
|
||||
if hard not in (-1, getattr(resource, "RLIM_INFINITY", object())):
|
||||
desired = min(desired, int(hard))
|
||||
if soft >= desired:
|
||||
return {"status": "unchanged", "soft": soft, "hard": hard}
|
||||
try:
|
||||
resource.setrlimit(resource.RLIMIT_NOFILE, (desired, hard))
|
||||
except Exception as exc:
|
||||
return {"status": "error", "soft": soft, "hard": hard, "error": str(exc)}
|
||||
return {"status": "raised", "soft": desired, "hard": hard, "previous_soft": soft}
|
||||
|
||||
|
||||
def main() -> None:
|
||||
from api.config import print_startup_config, verify_hermes_imports, _HERMES_FOUND
|
||||
|
||||
print_startup_config()
|
||||
|
||||
fd_limit = _raise_fd_soft_limit()
|
||||
if fd_limit.get("status") == "raised":
|
||||
print(
|
||||
f"[ok] Raised file descriptor soft limit "
|
||||
f"{fd_limit.get('previous_soft')} -> {fd_limit.get('soft')}",
|
||||
flush=True,
|
||||
)
|
||||
elif fd_limit.get("status") == "error":
|
||||
print(f"[!!] WARNING: Could not raise file descriptor limit: {fd_limit.get('error')}", flush=True)
|
||||
|
||||
# Fix sensitive file permissions before doing anything else
|
||||
fix_credential_permissions()
|
||||
|
||||
|
||||
66
tests/test_issue1458_stability_hardening.py
Normal file
66
tests/test_issue1458_stability_hardening.py
Normal file
@@ -0,0 +1,66 @@
|
||||
"""Regression coverage for issue #1458 persistent-host hardening."""
|
||||
import json
|
||||
import urllib.request
|
||||
|
||||
from tests._pytest_port import BASE
|
||||
|
||||
|
||||
def _get(path):
|
||||
with urllib.request.urlopen(BASE + path, timeout=10) as r:
|
||||
return json.loads(r.read()), r.status
|
||||
|
||||
|
||||
def test_health_exposes_accept_loop_heartbeat():
|
||||
data, status = _get("/health")
|
||||
|
||||
assert status == 200
|
||||
heartbeat = data.get("accept_loop")
|
||||
assert isinstance(heartbeat, dict)
|
||||
assert isinstance(heartbeat.get("requests_total"), int)
|
||||
assert heartbeat["requests_total"] >= 1
|
||||
assert isinstance(heartbeat.get("last_request_at"), (int, float))
|
||||
assert heartbeat["last_request_at"] > 0
|
||||
|
||||
|
||||
def test_deep_health_exercises_session_project_and_sqlite_paths():
|
||||
data, status = _get("/health?deep=1")
|
||||
|
||||
assert status == 200
|
||||
assert data["status"] == "ok"
|
||||
checks = data.get("checks")
|
||||
assert isinstance(checks, dict)
|
||||
assert checks["streams_lock"]["status"] == "ok"
|
||||
assert isinstance(checks["streams_lock"].get("active_streams"), int)
|
||||
assert checks["sessions"]["status"] == "ok"
|
||||
assert isinstance(checks["sessions"].get("count"), int)
|
||||
assert checks["projects"]["status"] == "ok"
|
||||
assert isinstance(checks["projects"].get("count"), int)
|
||||
# The isolated test home may not have a Hermes state.db yet. Deep health
|
||||
# should still report the state-db probe explicitly so watchdogs can tell
|
||||
# whether sqlite was checked or absent.
|
||||
assert checks["state_db"]["status"] in {"ok", "missing"}
|
||||
|
||||
|
||||
def test_server_raises_fd_soft_limit_when_resource_allows(monkeypatch):
|
||||
import server
|
||||
|
||||
calls = []
|
||||
|
||||
class FakeResource:
|
||||
RLIMIT_NOFILE = object()
|
||||
|
||||
@staticmethod
|
||||
def getrlimit(which):
|
||||
return (256, 8192)
|
||||
|
||||
@staticmethod
|
||||
def setrlimit(which, limits):
|
||||
calls.append((which, limits))
|
||||
|
||||
monkeypatch.setattr(server, "resource", FakeResource, raising=False)
|
||||
|
||||
result = server._raise_fd_soft_limit(target=4096)
|
||||
|
||||
assert result["status"] == "raised"
|
||||
assert result["soft"] == 4096
|
||||
assert calls == [(FakeResource.RLIMIT_NOFILE, (4096, 8192))]
|
||||
Reference in New Issue
Block a user