mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-20 15:33:54 +00:00
fix(title): contain auto-title thread exceptions instead of dumping tracebacks to the terminal (#65792)
auto_title_session runs as a bare daemon-thread target. Any exception
escaping it hits the default threading excepthook and sprays a raw
traceback into the user's terminal mid-session. The canonical trigger
is the post-'hermes update' stale-module window: the function's lazy
imports read NEW source from disk while already-imported modules
(agent.portal_tags) are still the OLD cached version, producing an
ImportError that repeats on every auto-title attempt until the
long-running process restarts (seen live after 9ce0e67f2 added
set_conversation_context).
The public entrypoint now wraps the body in a catch-all that logs one
WARNING naming the likely cause ('restart the running Hermes process'),
routes the exception through the existing failure_callback channel
(user-visible warning in CLI, debug-suppressed in gateway per #23246),
and never re-raises. This also makes the function honor its own
docstring contract ('silently skips if title generation fails').
This commit is contained in:
parent
f1af945f6c
commit
31a3822b80
2 changed files with 89 additions and 0 deletions
|
|
@ -133,7 +133,52 @@ def auto_title_session(
|
|||
- session_db is None
|
||||
- session already has a title (user-set or previously auto-generated)
|
||||
- title generation fails
|
||||
|
||||
Never lets an exception escape: this is a daemon-thread target, and an
|
||||
escaping exception would spray a raw traceback into the user's terminal
|
||||
via the default threading excepthook. The canonical trigger is the
|
||||
post-``hermes update`` stale-module window, where this function's lazy
|
||||
imports read NEW source from disk while already-cached modules
|
||||
(``agent.portal_tags`` etc.) are still the OLD version — the resulting
|
||||
ImportError repeats on every auto-title attempt until the long-running
|
||||
process restarts.
|
||||
"""
|
||||
try:
|
||||
_auto_title_session(
|
||||
session_db,
|
||||
session_id,
|
||||
user_message,
|
||||
assistant_response,
|
||||
failure_callback=failure_callback,
|
||||
main_runtime=main_runtime,
|
||||
title_callback=title_callback,
|
||||
)
|
||||
except Exception as e:
|
||||
# WARNING (not debug) so operators see it in agent.log; the message
|
||||
# names the likely cause so "restart the process" is discoverable.
|
||||
logger.warning(
|
||||
"Auto-title failed (harmless; if this started after an update, "
|
||||
"restart the running Hermes process): %s",
|
||||
e,
|
||||
)
|
||||
logger.debug("Auto-title traceback", exc_info=True)
|
||||
if failure_callback is not None:
|
||||
try:
|
||||
failure_callback("title generation", e)
|
||||
except Exception:
|
||||
logger.debug("Auto-title failure_callback raised", exc_info=True)
|
||||
|
||||
|
||||
def _auto_title_session(
|
||||
session_db,
|
||||
session_id: str,
|
||||
user_message: str,
|
||||
assistant_response: str,
|
||||
failure_callback: Optional[FailureCallback] = None,
|
||||
main_runtime: dict = None,
|
||||
title_callback: Optional[TitleCallback] = None,
|
||||
) -> None:
|
||||
"""Body of :func:`auto_title_session` — see its docstring."""
|
||||
if not session_db or not session_id:
|
||||
return
|
||||
|
||||
|
|
|
|||
|
|
@ -256,6 +256,50 @@ class TestAutoTitleSession:
|
|||
auto_title_session(db, "sess-1", "hi", "hello")
|
||||
db.set_session_title.assert_not_called()
|
||||
|
||||
def test_never_raises_when_body_throws(self):
|
||||
"""Daemon-thread target must swallow ALL exceptions (e.g. the
|
||||
post-update stale-module ImportError) instead of spraying a raw
|
||||
traceback into the terminal via the default threading excepthook."""
|
||||
db = MagicMock()
|
||||
db.get_session_title.return_value = None
|
||||
|
||||
with patch(
|
||||
"agent.title_generator._auto_title_session",
|
||||
side_effect=ImportError(
|
||||
"cannot import name 'set_conversation_context' from 'agent.portal_tags'"
|
||||
),
|
||||
):
|
||||
auto_title_session(db, "sess-1", "hi", "hello") # must not raise
|
||||
|
||||
def test_body_exception_routed_to_failure_callback(self):
|
||||
db = MagicMock()
|
||||
db.get_session_title.return_value = None
|
||||
seen = []
|
||||
|
||||
boom = ImportError("stale module")
|
||||
with patch("agent.title_generator._auto_title_session", side_effect=boom):
|
||||
auto_title_session(
|
||||
db,
|
||||
"sess-1",
|
||||
"hi",
|
||||
"hello",
|
||||
failure_callback=lambda task, exc: seen.append((task, exc)),
|
||||
)
|
||||
assert seen == [("title generation", boom)]
|
||||
|
||||
def test_failure_callback_errors_also_swallowed(self):
|
||||
db = MagicMock()
|
||||
db.get_session_title.return_value = None
|
||||
|
||||
def bad_cb(task, exc):
|
||||
raise RuntimeError("callback itself broke")
|
||||
|
||||
with patch(
|
||||
"agent.title_generator._auto_title_session",
|
||||
side_effect=ImportError("stale module"),
|
||||
):
|
||||
auto_title_session(db, "sess-1", "hi", "hello", failure_callback=bad_cb)
|
||||
|
||||
|
||||
class TestMaybeAutoTitle:
|
||||
"""Tests for maybe_auto_title() — the fire-and-forget entry point."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue