mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-26 03:30:36 +00:00
fix: clear stale saved session on 404 + structured api() errors
Two coupled fixes for the stale-empty-session regression: 1. api() in static/workspace.js now attaches HTTP context (.status, .statusText, .body) to thrown errors. Callers can branch on status without re-parsing the message string. 2. loadSession() in static/sessions.js: when a 404 comes back for the currently-saved active session ID, wipe the localStorage entry, clear the in-flight load marker, and rethrow so boot can fall through to the empty state. Previously the UI would stick on "Session not available in web UI." across reloads because the saved key never got removed. Click-into-404 (where the user clicked an existing list item that vanished) is unaffected — the cleanup is gated on !currentSid. Tests: tests/test_stale_empty_session_restore.py — 3 new assertions: * api() attaches status/statusText/body to errors * loadSession clears saved-stale-404 + rethrows * Click-into-404 does NOT clear the saved session (gate on !currentSid) Full suite: 3255 passed. Salvaged from contributor work in PR #1084. Co-authored-by: Hermes Agent <hermes@get-hermes.ai>
This commit is contained in:
@@ -338,6 +338,14 @@ async function loadSession(sid){
|
||||
if(_msgInner){
|
||||
if(e.status===404){
|
||||
_msgInner.innerHTML='<div style="display:flex;align-items:center;justify-content:center;height:100%;color:var(--text-muted);font-size:14px;padding:40px;text-align:center;">Session not available in web UI.</div>';
|
||||
// If this 404 was for the saved active-session ID (not a click-into request),
|
||||
// wipe the stale localStorage value and rethrow so boot can fall through to
|
||||
// the empty-state instead of sticking to a broken "Session not available" view.
|
||||
if(!currentSid&&localStorage.getItem('hermes-webui-session')===sid){
|
||||
localStorage.removeItem('hermes-webui-session');
|
||||
if (_loadingSessionId === sid) _loadingSessionId = null;
|
||||
throw e;
|
||||
}
|
||||
} else {
|
||||
_msgInner.innerHTML='<div style="display:flex;align-items:center;justify-content:center;height:100%;color:var(--text-muted);font-size:14px;padding:40px;text-align:center;">Failed to load session. Try switching sessions or refreshing.</div>';
|
||||
if(typeof showToast==='function') showToast('Failed to load session',3000,'error');
|
||||
|
||||
+9
-2
@@ -16,8 +16,15 @@ async function api(path,opts={}){
|
||||
const text=await res.text();
|
||||
// Parse JSON error body and surface the human-readable message,
|
||||
// rather than showing raw JSON like {"error":"Profile 'x' does not exist."}
|
||||
try{const j=JSON.parse(text);throw new Error(j.error||j.message||text);}
|
||||
catch(e){if(e instanceof SyntaxError)throw new Error(text);throw e;}
|
||||
let message=text;
|
||||
try{const j=JSON.parse(text);message=j.error||j.message||text;}catch(e){}
|
||||
// Attach the raw HTTP context so callers can branch on status (404 stale-session
|
||||
// cleanup, 401 redirect, 503 retry, etc.) without re-parsing the message string.
|
||||
const err=new Error(message);
|
||||
err.status=res.status;
|
||||
err.statusText=res.statusText;
|
||||
err.body=text;
|
||||
throw err;
|
||||
}
|
||||
const ct=res.headers.get('content-type')||'';
|
||||
return ct.includes('application/json')?res.json():res.text();
|
||||
|
||||
@@ -36,12 +36,18 @@ class TestPWAAuthRedirect:
|
||||
"workspace.js api() must redirect to /login on 401"
|
||||
|
||||
def test_workspace_js_401_before_throw(self):
|
||||
"""The 401 redirect must come before the generic error throw."""
|
||||
"""The 401 redirect must come before any error throw."""
|
||||
src = _workspace_js()
|
||||
idx_401 = src.find("res.status===401")
|
||||
# api() may throw via `throw new Error(...)` or via the structured
|
||||
# `const err=new Error(...); ... throw err;` pattern that attaches HTTP
|
||||
# context for callers. Either is fine — what matters is the 401 redirect
|
||||
# short-circuits before the generic throw.
|
||||
idx_throw = src.find("throw new Error")
|
||||
if idx_throw == -1:
|
||||
idx_throw = src.find("throw err")
|
||||
assert idx_401 != -1, "401 guard not found in workspace.js"
|
||||
assert idx_throw != -1, "throw not found in workspace.js"
|
||||
assert idx_throw != -1, "no error throw found in workspace.js"
|
||||
assert idx_401 < idx_throw, \
|
||||
"401 redirect must appear before the generic throw in workspace.js"
|
||||
|
||||
|
||||
@@ -0,0 +1,83 @@
|
||||
"""Regression tests for stale empty sessions after a WebUI restart.
|
||||
|
||||
When a saved session ID returns 404 (e.g. the session was deleted from another
|
||||
browser, or a state DB rotation removed it), the prior behavior was to show
|
||||
\"Session not available in web UI.\" and stick there forever — the saved
|
||||
localStorage entry never got cleared, so every reload reproduced the broken
|
||||
state.
|
||||
|
||||
These tests lock in:
|
||||
1. ``api()`` attaches HTTP context (``.status``, ``.statusText``, ``.body``)
|
||||
to thrown errors so callers can branch on status without re-parsing text.
|
||||
2. ``loadSession()`` clears the stale ``hermes-webui-session`` key on a 404
|
||||
for the currently-saved session and rethrows, so boot can fall through
|
||||
to the empty state.
|
||||
"""
|
||||
|
||||
from pathlib import Path
|
||||
import re
|
||||
|
||||
|
||||
REPO = Path(__file__).parent.parent
|
||||
WORKSPACE_JS = (REPO / "static" / "workspace.js").read_text(encoding="utf-8")
|
||||
SESSIONS_JS = (REPO / "static" / "sessions.js").read_text(encoding="utf-8")
|
||||
|
||||
|
||||
def _api_body() -> str:
|
||||
m = re.search(r"async function api\(path,opts=.*?\n\}", WORKSPACE_JS, re.DOTALL)
|
||||
assert m, "api() function must exist in workspace.js"
|
||||
return m.group(0)
|
||||
|
||||
|
||||
def _load_session_error_block() -> str:
|
||||
start = SESSIONS_JS.find("data = await api(`/api/session?")
|
||||
assert start > 0, "loadSession metadata request not found"
|
||||
catch_idx = SESSIONS_JS.find("} catch(e) {", start)
|
||||
assert catch_idx > start, "loadSession metadata catch block not found"
|
||||
end = SESSIONS_JS.find("return;", catch_idx)
|
||||
assert end > catch_idx, "loadSession metadata catch return not found"
|
||||
return SESSIONS_JS[catch_idx:end]
|
||||
|
||||
|
||||
def test_api_http_errors_preserve_response_status():
|
||||
"""Callers must be able to distinguish stale-session 404s from generic failures."""
|
||||
body = _api_body()
|
||||
assert re.search(r"\w+\.status\s*=\s*res\.status", body), (
|
||||
"api() must attach res.status to thrown HTTP errors"
|
||||
)
|
||||
assert re.search(r"\w+\.statusText\s*=\s*res\.statusText", body), (
|
||||
"api() must attach res.statusText to thrown HTTP errors"
|
||||
)
|
||||
assert re.search(r"\w+\.body\s*=\s*text", body), (
|
||||
"api() must attach the raw error body to thrown HTTP errors"
|
||||
)
|
||||
|
||||
|
||||
def test_load_session_clears_saved_stale_404_and_rethrows_to_boot():
|
||||
"""A missing saved session should be removed and let boot show the empty state."""
|
||||
block = _load_session_error_block()
|
||||
assert "e.status===404" in block, "loadSession must keep a 404-specific branch"
|
||||
assert "localStorage.getItem('hermes-webui-session')===sid" in block, (
|
||||
"loadSession must only clear the saved active session key"
|
||||
)
|
||||
assert "localStorage.removeItem('hermes-webui-session')" in block, (
|
||||
"loadSession must clear stale saved session IDs on 404"
|
||||
)
|
||||
assert "_loadingSessionId = null" in block, (
|
||||
"loadSession must clear the in-flight load marker before rethrowing"
|
||||
)
|
||||
assert re.search(r"throw\s+e", block), (
|
||||
"loadSession must rethrow the stale saved-session 404 so boot can fall "
|
||||
"through to the no-session empty state"
|
||||
)
|
||||
|
||||
|
||||
def test_click_into_404_does_not_clear_saved_session():
|
||||
"""A 404 from a click into a different session must NOT clear the saved active key."""
|
||||
block = _load_session_error_block()
|
||||
# The clearing branch is gated on `!currentSid` — i.e. only when the
|
||||
# request was for the *saved* active session, not a user click on another.
|
||||
assert "!currentSid" in block, (
|
||||
"404 cleanup must be gated on !currentSid so user-initiated clicks "
|
||||
"into a missing session don't wipe the saved active-session key"
|
||||
)
|
||||
Reference in New Issue
Block a user