mirror of
https://github.com/AstrBotDevs/AstrBot
synced 2026-07-15 17:30:13 +08:00
fix: harden runtime cleanup review fixes
Continue terminating remaining providers and disable MCP servers even if one provider terminate hook fails. Also add InitialLoader failure-path coverage and extract guarded plugin routes into a shared constant for easier review and maintenance.
This commit is contained in:
@@ -36,6 +36,23 @@ PLUGIN_UPDATE_CONCURRENCY = (
|
||||
3 # limit concurrent updates to avoid overwhelming plugin sources
|
||||
)
|
||||
|
||||
RUNTIME_GUARDED_PLUGIN_PATHS = {
|
||||
"/plugin/get",
|
||||
"/plugin/install",
|
||||
"/plugin/install-upload",
|
||||
"/plugin/update",
|
||||
"/plugin/update-all",
|
||||
"/plugin/uninstall",
|
||||
"/plugin/uninstall-failed",
|
||||
"/plugin/off",
|
||||
"/plugin/on",
|
||||
"/plugin/reload-failed",
|
||||
"/plugin/reload",
|
||||
"/plugin/readme",
|
||||
"/plugin/changelog",
|
||||
"/plugin/source/get-failed-plugins",
|
||||
}
|
||||
|
||||
|
||||
@dataclass
|
||||
class RegistrySource:
|
||||
@@ -80,22 +97,7 @@ class PluginRoute(Route):
|
||||
)
|
||||
for path, definition in list(self.routes.items()):
|
||||
method, handler = definition
|
||||
if path in {
|
||||
"/plugin/get",
|
||||
"/plugin/install",
|
||||
"/plugin/install-upload",
|
||||
"/plugin/update",
|
||||
"/plugin/update-all",
|
||||
"/plugin/uninstall",
|
||||
"/plugin/uninstall-failed",
|
||||
"/plugin/off",
|
||||
"/plugin/on",
|
||||
"/plugin/reload-failed",
|
||||
"/plugin/reload",
|
||||
"/plugin/readme",
|
||||
"/plugin/changelog",
|
||||
"/plugin/source/get-failed-plugins",
|
||||
}:
|
||||
if path in RUNTIME_GUARDED_PLUGIN_PATHS:
|
||||
self.routes[path] = (method, self._guard_runtime_ready(handler))
|
||||
self.register_routes()
|
||||
|
||||
|
||||
@@ -419,6 +419,34 @@ class TestProviderManagerCleanup:
|
||||
assert provider_manager.curr_stt_provider_inst is None
|
||||
assert provider_manager.curr_tts_provider_inst is None
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_terminate_continues_when_individual_provider_terminate_fails(self):
|
||||
"""Test terminate keeps cleaning remaining providers and MCP servers after one provider fails."""
|
||||
provider_manager = build_provider_manager_for_tests()
|
||||
provider_manager.llm_tools.disable_mcp_server = AsyncMock()
|
||||
|
||||
failing_provider = MagicMock()
|
||||
failing_provider.meta.return_value.id = "failing"
|
||||
failing_provider.terminate = AsyncMock(side_effect=RuntimeError("terminate failed"))
|
||||
|
||||
healthy_provider = MagicMock()
|
||||
healthy_provider.meta.return_value.id = "healthy"
|
||||
healthy_provider.terminate = AsyncMock()
|
||||
|
||||
provider_manager.provider_insts = [failing_provider, healthy_provider]
|
||||
provider_manager.inst_map = {
|
||||
"failing": failing_provider,
|
||||
"healthy": healthy_provider,
|
||||
}
|
||||
|
||||
with patch("astrbot.core.provider.manager.logger") as mock_logger:
|
||||
await provider_manager.terminate()
|
||||
|
||||
failing_provider.terminate.assert_awaited_once()
|
||||
healthy_provider.terminate.assert_awaited_once()
|
||||
provider_manager.llm_tools.disable_mcp_server.assert_awaited_once()
|
||||
mock_logger.error.assert_called()
|
||||
|
||||
|
||||
class TestContextRuntimeRegistrations:
|
||||
"""Tests for runtime registration containers on Context."""
|
||||
|
||||
@@ -75,3 +75,32 @@ async def test_initial_loader_start_awaits_initialize_core_and_schedules_runtime
|
||||
assert call_order[:3] == ["initialize_core", "create_task", "dashboard_init"]
|
||||
assert len(created_tasks) == 1
|
||||
assert lifecycle.runtime_bootstrap_task is created_tasks[0]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_initial_loader_start_returns_without_partial_start_when_initialize_core_fails():
|
||||
"""Test InitialLoader.start aborts cleanly if initialize_core fails."""
|
||||
loader = InitialLoader(MagicMock(), MagicMock())
|
||||
|
||||
lifecycle = MagicMock()
|
||||
lifecycle.runtime_bootstrap_task = None
|
||||
expected_error = RuntimeError("core init failed")
|
||||
lifecycle.initialize_core = AsyncMock(side_effect=expected_error)
|
||||
lifecycle.bootstrap_runtime = AsyncMock()
|
||||
lifecycle.start = AsyncMock()
|
||||
|
||||
with (
|
||||
patch(
|
||||
"astrbot.core.initial_loader.AstrBotCoreLifecycle", return_value=lifecycle
|
||||
),
|
||||
patch("astrbot.core.initial_loader.AstrBotDashboard") as dashboard_cls,
|
||||
patch("astrbot.core.initial_loader.asyncio.create_task") as create_task,
|
||||
):
|
||||
await loader.start()
|
||||
|
||||
lifecycle.initialize_core.assert_awaited_once()
|
||||
dashboard_cls.assert_not_called()
|
||||
create_task.assert_not_called()
|
||||
lifecycle.bootstrap_runtime.assert_not_called()
|
||||
lifecycle.start.assert_not_called()
|
||||
assert lifecycle.runtime_bootstrap_task is None
|
||||
|
||||
Reference in New Issue
Block a user