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
+65 -21

No files matched your search

+32 -20
View File
@@ -22824,26 +22824,38 @@ 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).
with _get_session_agent_lock(session.session_id):
try:
canonical = get_session(session.session_id)
except KeyError:
return # session deleted while the thread launch was failing
if getattr(canonical, "active_stream_id", None) != stream_id:
return # a successor turn already owns the session
canonical.active_stream_id = None
canonical.pending_user_message = None
canonical.pending_attachments = []
canonical.pending_started_at = None
canonical.pending_user_source = None
try:
canonical.save()
except Exception:
logger.debug(
"Failed to persist chat-start cleanup after worker launch failure for %s",
stream_id,
exc_info=True,
)
#
# 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)
except KeyError:
return # session deleted while the thread launch was failing
if getattr(canonical, "active_stream_id", None) != stream_id:
return # a successor turn already owns the session
canonical.active_stream_id = None
canonical.pending_user_message = None
canonical.pending_attachments = []
canonical.pending_started_at = None
canonical.pending_user_source = None
try:
canonical.save()
except Exception:
logger.debug(
"Failed to persist chat-start cleanup after worker launch failure for %s",
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:
+33 -1
View File
@@ -182,4 +182,36 @@ def test_gateway_launch_failure_cleanup_does_not_resurrect_deleted_session(monke
assert "old-stream" not in config.STREAMS
assert stale.active_stream_id == "old-stream"
assert stale.pending_user_message == "pending prompt"
assert saved == [], "cleanup must not save a deleted session"
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 == []