fix(routes): never mask the launch error during cleanup

_cleanup_chat_start_launch_failure runs inside the launch-failure handler,
but the session re-resolve only caught KeyError. An unreadable or
undeserializable session store raises something else, and that escaped the
helper, replacing the worker-launch error the caller is about to report and
aborting the reset after the registries were already cleared.

Contain the resolution-and-reset so the helper cannot raise. The KeyError
branch keeps its meaning (deleted session, return without saving); anything
else is logged at debug like the existing save() failure path.
This commit is contained in:
James Drake committed 2026-09-19 00:29:07 -05:00
1 parent 218164f6c8
commit 047dfa40a5
2 files changed
+44

No files matched your search

+12
View File
@@ -22824,6 +22824,12 @@ def _cleanup_chat_start_launch_failure(session, stream_id: str) -> None:
# could resurrect a session deleted while the launch was failing. Same
# pattern as the #1533 race fix (routes.py:3077) and the anchor-scene
# write guard (routes.py:5140).
#
# This runs while the original launch failure is being handled, so it must
# never raise. Lock acquisition and session resolution can fail on their own
# (I/O, deserialization), and an escaping error here would mask the launch
# failure the caller is about to report while leaving the reset half done.
try:
with _get_session_agent_lock(session.session_id):
try:
canonical = get_session(session.session_id)
@@ -22844,6 +22850,12 @@ def _cleanup_chat_start_launch_failure(session, stream_id: str) -> None:
stream_id,
exc_info=True,
)
except Exception:
logger.debug(
"Failed to reset session state after worker launch failure for %s",
stream_id,
exc_info=True,
)
def _is_hidden_empty_session(s) -> bool:
@@ -183,3 +183,35 @@ def test_gateway_launch_failure_cleanup_does_not_resurrect_deleted_session(monke
assert stale.active_stream_id == "old-stream"
assert stale.pending_user_message == "pending prompt"
assert saved == [], "cleanup must not save a deleted session"
def test_gateway_launch_failure_cleanup_never_masks_the_launch_error(monkeypatch):
"""A resolution failure during cleanup must not replace the launch error.
Cleanup runs inside the launch-failure handler, so an error it fails to
contain is reported instead of the real launch failure. Resolution can fail
with something other than KeyError when the session store is unreadable or
cannot be deserialized.
"""
saved = []
def failing_resolver(sid, metadata_only=False):
raise OSError("session store unreadable")
monkeypatch.setattr(routes, "get_session", failing_resolver)
stale = _make_session(
"session-launch-resolution-error",
active_stream_id="old-stream",
pending_user_message="pending prompt",
save=lambda: saved.append("stale"),
)
_register_failed_stream(stale.session_id, "old-stream")
# Must not raise: the caller is already handling the launch failure.
routes._cleanup_chat_start_launch_failure(stale, "old-stream")
# The registry half still completed, and nothing was saved.
assert config.session_writeback_owner(stale.session_id) is None
assert "old-stream" not in config.STREAM_SESSION_OWNERS
assert "old-stream" not in config.STREAMS
assert saved == []