fix(sessions): surface get_cli_sessions() failures via logger.warning (#769)
Logs warnings instead of silently returning [] on DB errors. Fixes #634.
This commit is contained in:
@@ -1,5 +1,10 @@
|
|||||||
# Hermes Web UI -- Changelog
|
# Hermes Web UI -- Changelog
|
||||||
|
|
||||||
|
## [v0.50.118] — 2026-04-20
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
- **CLI sessions: silent failure now logged** — `get_cli_sessions()` no longer swallows DB errors silently. If `state.db` is missing the `source` column (older hermes-agent) or has any other schema/lock issue, a warning is now logged with the DB path and a hint to upgrade hermes-agent. This makes "Show CLI sessions in sidebar has no effect" diagnosable from the server log instead of requiring code archaeology. (#634)
|
||||||
|
|
||||||
## [v0.50.117] — 2026-04-20
|
## [v0.50.117] — 2026-04-20
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|||||||
@@ -297,6 +297,21 @@ def get_cli_sessions() -> list:
|
|||||||
with sqlite3.connect(str(db_path)) as conn:
|
with sqlite3.connect(str(db_path)) as conn:
|
||||||
conn.row_factory = sqlite3.Row
|
conn.row_factory = sqlite3.Row
|
||||||
cur = conn.cursor()
|
cur = conn.cursor()
|
||||||
|
# Introspect schema to handle older hermes-agent versions that
|
||||||
|
# may not have a 'source' column. Without this check the query raises
|
||||||
|
# OperationalError which is silently swallowed, causing the empty-list bug.
|
||||||
|
cur.execute("PRAGMA table_info(sessions)")
|
||||||
|
_session_cols = {row[1] for row in cur.fetchall()}
|
||||||
|
if 'source' not in _session_cols:
|
||||||
|
import logging as _logging
|
||||||
|
_logging.getLogger(__name__).warning(
|
||||||
|
"get_cli_sessions(): state.db at %s has no 'source' column "
|
||||||
|
"(older hermes-agent?). CLI sessions unavailable. "
|
||||||
|
"Upgrade hermes-agent to fix this.",
|
||||||
|
db_path,
|
||||||
|
)
|
||||||
|
return cli_sessions
|
||||||
|
|
||||||
cur.execute("""
|
cur.execute("""
|
||||||
SELECT s.id, s.title, s.model, s.message_count,
|
SELECT s.id, s.title, s.model, s.message_count,
|
||||||
s.started_at, s.source,
|
s.started_at, s.source,
|
||||||
@@ -332,8 +347,14 @@ def get_cli_sessions() -> list:
|
|||||||
'source_tag': _source,
|
'source_tag': _source,
|
||||||
'is_cli_session': True,
|
'is_cli_session': True,
|
||||||
})
|
})
|
||||||
except Exception:
|
except Exception as _cli_err:
|
||||||
# DB schema changed, locked, or corrupted -- silently degrade
|
# DB schema changed, locked, or corrupted -- log warning so admins can diagnose.
|
||||||
|
# Still degrade gracefully (don't crash the WebUI).
|
||||||
|
import logging as _logging
|
||||||
|
_logging.getLogger(__name__).warning(
|
||||||
|
"get_cli_sessions() failed — check state.db schema or path (%s): %s",
|
||||||
|
db_path, _cli_err,
|
||||||
|
)
|
||||||
return []
|
return []
|
||||||
|
|
||||||
return cli_sessions
|
return cli_sessions
|
||||||
|
|||||||
75
tests/test_issue634.py
Normal file
75
tests/test_issue634.py
Normal file
@@ -0,0 +1,75 @@
|
|||||||
|
"""
|
||||||
|
Tests for #634: CLI sessions not visible when setting is enabled.
|
||||||
|
|
||||||
|
Root cause: get_cli_sessions() swallowed all errors silently (bare except → return []).
|
||||||
|
Users with older hermes-agent versions (missing 'source' column in state.db) got
|
||||||
|
an empty list with no log output, making diagnosis impossible.
|
||||||
|
|
||||||
|
Fixes:
|
||||||
|
1. Schema introspection: check for 'source' column before querying, log a warning
|
||||||
|
if missing and return early.
|
||||||
|
2. Exception path: log a warning instead of silently returning [].
|
||||||
|
"""
|
||||||
|
import pathlib
|
||||||
|
import re
|
||||||
|
|
||||||
|
MODELS_PY = pathlib.Path(__file__).parent.parent / 'api' / 'models.py'
|
||||||
|
src = MODELS_PY.read_text(encoding='utf-8')
|
||||||
|
|
||||||
|
|
||||||
|
class TestCliSessionsErrorSurface:
|
||||||
|
"""get_cli_sessions() must log warnings instead of silently returning []."""
|
||||||
|
|
||||||
|
def test_schema_introspection_present(self):
|
||||||
|
"""The function must check for the 'source' column before querying."""
|
||||||
|
assert "PRAGMA table_info(sessions)" in src
|
||||||
|
|
||||||
|
def test_missing_source_column_logs_warning(self):
|
||||||
|
"""If 'source' column is absent, a warning is logged."""
|
||||||
|
# The warning message must mention the missing column and how to fix it
|
||||||
|
assert "no 'source' column" in src or "has no 'source' column" in src
|
||||||
|
|
||||||
|
def test_missing_source_column_suggests_upgrade(self):
|
||||||
|
"""Warning message must suggest upgrading hermes-agent."""
|
||||||
|
assert "Upgrade hermes-agent" in src or "upgrade hermes-agent" in src.lower()
|
||||||
|
|
||||||
|
def test_exception_path_logs_warning(self):
|
||||||
|
"""The except clause must call logger.warning, not silently pass."""
|
||||||
|
# Find the exception handler in get_cli_sessions
|
||||||
|
func_start = src.find("def get_cli_sessions()")
|
||||||
|
func_end = src.find("\ndef ", func_start + 1)
|
||||||
|
func_body = src[func_start:func_end] if func_end != -1 else src[func_start:]
|
||||||
|
assert "warning(" in func_body, \
|
||||||
|
"get_cli_sessions() exception handler must call logging.warning()"
|
||||||
|
|
||||||
|
def test_exception_path_includes_db_path(self):
|
||||||
|
"""The warning must include the db_path for diagnosability."""
|
||||||
|
func_start = src.find("def get_cli_sessions()")
|
||||||
|
func_end = src.find("\ndef ", func_start + 1)
|
||||||
|
func_body = src[func_start:func_end] if func_end != -1 else src[func_start:]
|
||||||
|
# db_path should appear in the warning call
|
||||||
|
warning_pos = func_body.find("warning(")
|
||||||
|
warning_block = func_body[warning_pos:warning_pos + 300]
|
||||||
|
assert "db_path" in warning_block, \
|
||||||
|
"Warning must include db_path so admins can find the problematic database"
|
||||||
|
|
||||||
|
def test_still_returns_empty_on_error(self):
|
||||||
|
"""Function must still return [] after logging (graceful degradation)."""
|
||||||
|
# After the warning, it should return cli_sessions (the empty list) not raise
|
||||||
|
func_start = src.find("def get_cli_sessions()")
|
||||||
|
func_end = src.find("\ndef ", func_start + 1)
|
||||||
|
func_body = src[func_start:func_end] if func_end != -1 else src[func_start:]
|
||||||
|
# Must have a 'return' after the warning call
|
||||||
|
warning_pos = func_body.find("_cli_err:")
|
||||||
|
after_warning = func_body[warning_pos:warning_pos + 400]
|
||||||
|
assert "return" in after_warning, \
|
||||||
|
"Function must return after the warning (not raise)"
|
||||||
|
|
||||||
|
def test_source_column_check_before_sql_query(self):
|
||||||
|
"""Schema check must happen before the main SQL SELECT."""
|
||||||
|
pragma_pos = src.find("PRAGMA table_info(sessions)")
|
||||||
|
select_pos = src.find("SELECT s.id, s.title, s.model")
|
||||||
|
assert pragma_pos != -1, "PRAGMA check not found"
|
||||||
|
assert select_pos != -1, "SELECT query not found"
|
||||||
|
assert pragma_pos < select_pos, \
|
||||||
|
"Schema introspection must run before the main SQL query"
|
||||||
Reference in New Issue
Block a user