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:
1 parent
218164f6c8
commit
047dfa40a5
2 files changed
+44
No files matched your search
@@ -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 == []
|
||||
Reference in new issue
Block a user