refactor(gateway): verdict helpers live in run_shutdown; keep the early failure return; dedupe test setup
- _exit_with_failure_verdict / _resolve_gateway_exit_verdict move out of the run.py facade into run_shutdown.py, which already owns _restart_via_service and GATEWAY_SERVICE_RESTART_EXIT_CODE (no alias import needed). - The running-shutdown tail keeps its early `return False` on a failure verdict, as on main, so a failure exit does not first drain cron/MCP; the helper still re-checks it for the startup-abort path. - The three start_gateway tests share one _patch_aborted_startup helper.
This commit is contained in:
@@ -2027,7 +2027,7 @@ from gateway.run_voice import GatewayVoiceMixin
|
||||
from gateway.run_adapters import GatewayAdapterLifecycleMixin
|
||||
from gateway.run_topics import GatewayTopicThreadsMixin
|
||||
from gateway.run_turn import GatewayTurnMixin
|
||||
from gateway.run_shutdown import GatewayShutdownMixin
|
||||
from gateway.run_shutdown import GatewayShutdownMixin, _exit_with_failure_verdict, _resolve_gateway_exit_verdict
|
||||
from gateway.run_busy import GatewayBusySessionMixin
|
||||
from gateway.run_config_loaders import GatewayConfigLoadersMixin
|
||||
from gateway.run_startup import GatewayStartupMixin
|
||||
@@ -2046,8 +2046,7 @@ from gateway.restart import (
|
||||
DEFAULT_GATEWAY_CRON_DRAIN_TIMEOUT,
|
||||
DEFAULT_GATEWAY_RESTART_AFTER_TURN_TIMEOUT,
|
||||
DEFAULT_GATEWAY_RESTART_DRAIN_TIMEOUT,
|
||||
DEFAULT_GATEWAY_SIGNAL_INTERRUPT_GRACE_TIMEOUT,
|
||||
GATEWAY_SERVICE_RESTART_EXIT_CODE as _GATEWAY_SERVICE_RESTART_EXIT_CODE)
|
||||
DEFAULT_GATEWAY_SIGNAL_INTERRUPT_GRACE_TIMEOUT)
|
||||
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -5088,38 +5087,6 @@ def _start_gateway_start_cron_and_housekeeping(runner):
|
||||
return cron_stop, cron_provider, cron_thread, housekeeping_thread
|
||||
|
||||
|
||||
def _exit_with_failure_verdict(runner) -> bool:
|
||||
"""True (after logging the reason) when the runner asked for a failure exit."""
|
||||
if not runner.should_exit_with_failure:
|
||||
return False
|
||||
if runner.exit_reason:
|
||||
logger.error("Gateway exiting with failure: %s", runner.exit_reason)
|
||||
return True
|
||||
|
||||
|
||||
def _resolve_gateway_exit_verdict(runner, signal_initiated_shutdown: bool) -> bool:
|
||||
"""Resolve the process verdict after either startup abort or normal shutdown."""
|
||||
if _exit_with_failure_verdict(runner):
|
||||
return False
|
||||
if runner.exit_code is not None:
|
||||
raise SystemExit(runner.exit_code)
|
||||
if signal_initiated_shutdown and not runner._restart_requested:
|
||||
logger.info(
|
||||
"Exiting with code 1 (signal-initiated shutdown without restart "
|
||||
"request) so the service manager can revive the gateway."
|
||||
)
|
||||
return False
|
||||
# Older restart paths may not set ``runner.exit_code``; retain the service-restart fallback.
|
||||
if runner._restart_via_service:
|
||||
logger.info(
|
||||
"Exiting with code %d (service-restart requested) so the service "
|
||||
"manager relaunches the gateway.",
|
||||
_GATEWAY_SERVICE_RESTART_EXIT_CODE,
|
||||
)
|
||||
raise SystemExit(_GATEWAY_SERVICE_RESTART_EXIT_CODE)
|
||||
return True
|
||||
|
||||
|
||||
async def _start_gateway_shutdown_tail(
|
||||
runner, _control_server, cron_stop: threading.Event, cron_provider,
|
||||
cron_thread: threading.Thread, housekeeping_thread: threading.Thread,
|
||||
@@ -5139,6 +5106,8 @@ async def _start_gateway_shutdown_tail(
|
||||
stop_nous_auth_keepalive()
|
||||
|
||||
_best_effort(_stop_keepalive)
|
||||
if _exit_with_failure_verdict(runner):
|
||||
return False
|
||||
|
||||
# Never join(): an in-flight cron delivery is a coroutine on THIS loop; a sync join would drop it.
|
||||
# Stop cron scheduler + housekeeping cleanly. These MUST be awaited cooperatively, not join()ed. A cron
|
||||
|
||||
@@ -30,6 +30,38 @@ from gateway.shutdown_watchdog import arm_shutdown_watchdog, resolve_shutdown_wa
|
||||
# Log-record parity with the origin module.
|
||||
logger = logging.getLogger("gateway.run")
|
||||
|
||||
|
||||
def _exit_with_failure_verdict(runner) -> bool:
|
||||
"""True (after logging the reason) when the runner asked for a failure exit."""
|
||||
if not runner.should_exit_with_failure:
|
||||
return False
|
||||
if runner.exit_reason:
|
||||
logger.error("Gateway exiting with failure: %s", runner.exit_reason)
|
||||
return True
|
||||
|
||||
|
||||
def _resolve_gateway_exit_verdict(runner, signal_initiated_shutdown: bool) -> bool:
|
||||
"""Resolve the process verdict after either startup abort or normal shutdown."""
|
||||
if _exit_with_failure_verdict(runner):
|
||||
return False
|
||||
if runner.exit_code is not None:
|
||||
raise SystemExit(runner.exit_code)
|
||||
if signal_initiated_shutdown and not runner._restart_requested:
|
||||
logger.info(
|
||||
"Exiting with code 1 (signal-initiated shutdown without restart "
|
||||
"request) so the service manager can revive the gateway."
|
||||
)
|
||||
return False
|
||||
# Older restart paths may not set ``runner.exit_code``; retain the service-restart fallback.
|
||||
if runner._restart_via_service:
|
||||
logger.info(
|
||||
"Exiting with code %d (service-restart requested) so the service "
|
||||
"manager relaunches the gateway.",
|
||||
GATEWAY_SERVICE_RESTART_EXIT_CODE,
|
||||
)
|
||||
raise SystemExit(GATEWAY_SERVICE_RESTART_EXIT_CODE)
|
||||
return True
|
||||
|
||||
# Windows has no bash/setsid chain: a tiny detached Python watcher waits for the gateway PID to
|
||||
# exit (bounded), then spawns ``hermes gateway restart``.
|
||||
_WINDOWS_RESTART_WATCHER = """
|
||||
|
||||
@@ -167,6 +167,18 @@ async def test_startup_aborts_when_restart_begins_during_platform_connect(tmp_pa
|
||||
)
|
||||
|
||||
|
||||
def _patch_aborted_startup(monkeypatch, runner_cls):
|
||||
"""Run start_gateway() against a runner that aborts before running mode."""
|
||||
monkeypatch.setattr("gateway.status.get_running_pid", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.acquire_gateway_runtime_lock", lambda: True)
|
||||
monkeypatch.setattr("gateway.status.write_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.remove_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.release_gateway_runtime_lock", lambda: None)
|
||||
monkeypatch.setattr("tools.skills_sync.sync_skills", lambda quiet=True: None)
|
||||
monkeypatch.setattr("hermes_logging.setup_logging", lambda hermes_home, mode: None)
|
||||
monkeypatch.setattr("gateway.run.GatewayRunner", runner_cls)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_start_gateway_does_not_start_cron_after_aborted_startup(tmp_path, monkeypatch):
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
@@ -199,14 +211,7 @@ async def test_start_gateway_does_not_start_cron_after_aborted_startup(tmp_path,
|
||||
nonlocal cron_started
|
||||
cron_started = True
|
||||
|
||||
monkeypatch.setattr("gateway.status.get_running_pid", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.acquire_gateway_runtime_lock", lambda: True)
|
||||
monkeypatch.setattr("gateway.status.write_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.remove_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.release_gateway_runtime_lock", lambda: None)
|
||||
monkeypatch.setattr("tools.skills_sync.sync_skills", lambda quiet=True: None)
|
||||
monkeypatch.setattr("hermes_logging.setup_logging", lambda hermes_home, mode: None)
|
||||
monkeypatch.setattr("gateway.run.GatewayRunner", AbortedStartupRunner)
|
||||
_patch_aborted_startup(monkeypatch, AbortedStartupRunner)
|
||||
monkeypatch.setattr("gateway.run._start_cron_ticker", fail_if_cron_starts)
|
||||
monkeypatch.setattr("tools.mcp_tool_lifecycle.shutdown_mcp_servers", lambda: None)
|
||||
|
||||
@@ -248,14 +253,7 @@ async def test_start_gateway_preserves_service_restart_fallback_after_aborted_st
|
||||
nonlocal cron_started
|
||||
cron_started = True
|
||||
|
||||
monkeypatch.setattr("gateway.status.get_running_pid", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.acquire_gateway_runtime_lock", lambda: True)
|
||||
monkeypatch.setattr("gateway.status.write_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.remove_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.release_gateway_runtime_lock", lambda: None)
|
||||
monkeypatch.setattr("tools.skills_sync.sync_skills", lambda quiet=True: None)
|
||||
monkeypatch.setattr("hermes_logging.setup_logging", lambda hermes_home, mode: None)
|
||||
monkeypatch.setattr("gateway.run.GatewayRunner", AbortedStartupRunner)
|
||||
_patch_aborted_startup(monkeypatch, AbortedStartupRunner)
|
||||
monkeypatch.setattr("gateway.run._start_cron_ticker", fail_if_cron_starts)
|
||||
monkeypatch.setattr("tools.mcp_tool_lifecycle.shutdown_mcp_servers", lambda: None)
|
||||
|
||||
@@ -311,14 +309,7 @@ async def test_start_gateway_classifies_startup_signal_exit(
|
||||
nonlocal cron_started
|
||||
cron_started = True
|
||||
|
||||
monkeypatch.setattr("gateway.status.get_running_pid", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.acquire_gateway_runtime_lock", lambda: True)
|
||||
monkeypatch.setattr("gateway.status.write_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.remove_pid_file", lambda: None)
|
||||
monkeypatch.setattr("gateway.status.release_gateway_runtime_lock", lambda: None)
|
||||
monkeypatch.setattr("tools.skills_sync.sync_skills", lambda quiet=True: None)
|
||||
monkeypatch.setattr("hermes_logging.setup_logging", lambda hermes_home, mode: None)
|
||||
monkeypatch.setattr("gateway.run.GatewayRunner", AbortedStartupRunner)
|
||||
_patch_aborted_startup(monkeypatch, AbortedStartupRunner)
|
||||
monkeypatch.setattr(
|
||||
"gateway.run._start_gateway_make_shutdown_signal_handler", capture_signal_state
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user