From 9282bceaf886efbb5052275822ade4c954a85f1e Mon Sep 17 00:00:00 2001 From: claude Date: Wed, 3 Jun 2026 04:13:53 +0000 Subject: [PATCH] fix: emit WARNING when Docker teardown ExitStack raises (issue #156) Replace the bare `except BaseException: pass` in the `teardown` closure with a `warn()` call that includes the container name and operation type ("compose-down"), so cleanup failures are visible in the log rather than silently discarded. Non-blocking: the exception is consumed and teardown continues, preserving the original error-propagation contract. Add test_docker_launch_teardown.py to lock the new behaviour: it injects a RuntimeError via a mocked `compose_down` callback and asserts the WARNING message contains the container name and operation label. --- bot_bottle/backend/docker/launch.py | 11 +- tests/unit/test_docker_launch_teardown.py | 145 ++++++++++++++++++++++ 2 files changed, 151 insertions(+), 5 deletions(-) create mode 100644 tests/unit/test_docker_launch_teardown.py diff --git a/bot_bottle/backend/docker/launch.py b/bot_bottle/backend/docker/launch.py index 800cf42..c6676e1 100644 --- a/bot_bottle/backend/docker/launch.py +++ b/bot_bottle/backend/docker/launch.py @@ -43,7 +43,7 @@ from pathlib import Path from typing import Callable, Generator from ...egress import egress_resolve_token_values -from ...log import info +from ...log import info, warn from . import network as network_mod from . import util as docker_mod from .bottle import DockerBottle @@ -87,10 +87,11 @@ def launch( def teardown() -> None: try: stack.close() - except BaseException: - # Teardown must not raise; swallow so the caller's - # __exit__ path can still propagate the original error. - pass + except BaseException as exc: + warn( + f"teardown failed for container {plan.container_name}" + f" (compose-down): {exc!r}" + ) try: # Step 1: agent image build. Sidecar images get built lazily by diff --git a/tests/unit/test_docker_launch_teardown.py b/tests/unit/test_docker_launch_teardown.py new file mode 100644 index 0000000..8e669d0 --- /dev/null +++ b/tests/unit/test_docker_launch_teardown.py @@ -0,0 +1,145 @@ +"""Unit: Docker launch teardown warning on ExitStack failure (issue #156). + +When a callback registered in the ExitStack raises during teardown, +the teardown function must emit a WARNING-level message that includes +the container name and operation type, rather than silently discarding +the exception. +""" + +from __future__ import annotations + +import contextlib +import io +import tempfile +import unittest +from pathlib import Path +from unittest import mock + +from bot_bottle.agent_provider import AgentProvisionPlan +from bot_bottle.backend import BottleSpec +from bot_bottle.backend.docker import launch as launch_mod +from bot_bottle.backend.docker.bottle_plan import DockerBottlePlan +from bot_bottle.egress import EgressPlan +from bot_bottle.git_gate import GitGatePlan +from bot_bottle.manifest import Manifest +from bot_bottle.pipelock import PipelockProxyPlan +from bot_bottle.workspace import workspace_plan + + +def _manifest() -> Manifest: + return Manifest.from_json_obj({ + "bottles": {"dev": {}}, + "agents": {"demo": {"skills": [], "prompt": "", "bottle": "dev"}}, + }) + + +def _plan(tmp: str) -> DockerBottlePlan: + stage = Path(tmp) + manifest = _manifest() + spec = BottleSpec( + manifest=manifest, + agent_name="demo", + copy_cwd=False, + user_cwd=tmp, + identity="test-teardown-00001", + ) + return DockerBottlePlan( + spec=spec, + stage_dir=stage, + git_gate_plan=GitGatePlan( + slug="test-teardown-00001", + entrypoint_script=stage / "entrypoint.sh", + hook_script=stage / "hook.sh", + access_hook_script=stage / "access-hook.sh", + upstreams=(), + ), + egress_plan=EgressPlan( + slug="test-teardown-00001", + routes_path=stage / "egress.yaml", + routes=(), + token_env_map={}, + ), + supervise_plan=None, + agent_provision=AgentProvisionPlan( + template="claude", + command="claude", + prompt_mode="append_file", + image="", + dockerfile="", + guest_env={}, + ), + workspace_plan=workspace_plan(spec, guest_home="/home/node"), + slug="test-teardown-00001", + container_name="bot-bottle-test-teardown-abc", + container_name_pinned=False, + image="bot-bottle-claude:latest", + derived_image="", + runtime_image="bot-bottle-claude:latest", + dockerfile_path="", + env_file=stage / "env", + forwarded_env={}, + prompt_file=stage / "prompt.txt", + proxy_plan=PipelockProxyPlan( + yaml_path=stage / "pipelock.yaml", + slug="test-teardown-00001", + ), + use_runsc=False, + ) + + +class TestTeardownWarning(unittest.TestCase): + def setUp(self) -> None: + self._tmp = tempfile.mkdtemp(prefix="docker-launch-teardown-test.") + + def tearDown(self) -> None: + import shutil + shutil.rmtree(self._tmp, ignore_errors=True) + + def test_teardown_failure_emits_warning_with_container_and_operation(self): + plan = _plan(self._tmp) + buf = io.StringIO() + + with mock.patch.object(launch_mod.docker_mod, "build_image"), \ + mock.patch.object( + launch_mod, "pipelock_tls_init", + return_value=(Path("/ca.crt"), Path("/ca.key")), + ), \ + mock.patch.object( + launch_mod, "egress_tls_init", + return_value=(Path("/egress_ca"), Path("/egress_cert")), + ), \ + mock.patch.object( + launch_mod.network_mod, "network_name_for_slug", + return_value="bb-internal-test", + ), \ + mock.patch.object( + launch_mod.network_mod, "network_egress_name_for_slug", + return_value="bb-egress-test", + ), \ + mock.patch.object( + launch_mod, "bottle_plan_to_compose", + return_value={"services": {"agent": {}}}, + ), \ + mock.patch.object( + launch_mod, "write_compose_file", + return_value=Path("/tmp/compose.yml"), + ), \ + mock.patch.object(launch_mod, "compose_up"), \ + mock.patch.object(launch_mod, "compose_dump_logs"), \ + mock.patch.object( + launch_mod, "compose_down", + side_effect=RuntimeError("network remove failed"), + ), \ + contextlib.redirect_stderr(buf): + provision = mock.Mock(return_value=None) + with launch_mod.launch(plan, provision=provision): + pass + + output = buf.getvalue() + self.assertIn("bot-bottle: warning:", output) + self.assertIn("bot-bottle-test-teardown-abc", output) + self.assertIn("compose-down", output) + + +if __name__ == "__main__": + unittest.main()