Have CopilotClient.stop() raise an exception group instead of returning a list of exceptions (#603)

The Zen of Python says, "Errors should never pass silently".

Author: brettcannon

python/copilot/client.py

@@ -315,7 +315,7 @@ async def start(self) -> None:
                         ) from e
             raise
 
-    async def stop(self) -> list["StopError"]:
+    async def stop(self) -> None:
         """
         Stop the CLI server and close all active sessions.
 
@@ -324,14 +324,14 @@ async def stop(self) -> list["StopError"]:
         2. Closes the JSON-RPC connection
         3. Terminates the CLI server process (if spawned by this client)
 
-        Returns:
-            A list of StopError objects containing error messages that occurred
-            during cleanup. An empty list indicates all cleanup succeeded.
+        Raises:
+            ExceptionGroup[StopError]: If any errors occurred during cleanup.
 
         Example:
-            >>> errors = await client.stop()
-            >>> if errors:
-            ...     for error in errors:
+            >>> try:
+            ...     await client.stop()
+            ... except* StopError as eg:
+            ...     for error in eg.exceptions:
             ...         print(f"Cleanup error: {error.message}")
         """
         errors: list[StopError] = []
@@ -360,7 +360,6 @@ async def stop(self) -> list["StopError"]:
         async with self._models_cache_lock:
             self._models_cache = None
 
-        # Kill CLI process
         # Kill CLI process (only if we spawned it)
         if self._process and not self._is_external_server:
             self._process.terminate()
@@ -374,7 +373,8 @@ async def stop(self) -> list["StopError"]:
         if not self._is_external_server:
             self._actual_port = None
 
-        return errors
+        if errors:
+            raise ExceptionGroup("errors during CopilotClient.stop()", errors)
 
     async def force_stop(self) -> None:
         """

python/copilot/types.py

@@ -619,11 +619,14 @@ def to_dict(self) -> dict:
 
 # Error information from client stop
 @dataclass
-class StopError:
-    """Error information from client stop"""
+class StopError(Exception):
+    """Error that occurred during client stop cleanup."""
 
     message: str  # Error message describing what failed during cleanup
 
+    def __post_init__(self) -> None:
+        Exception.__init__(self, self.message)
+
     @staticmethod
     def from_dict(obj: Any) -> StopError:
         assert isinstance(obj, dict)

python/e2e/test_client.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from copilot import CopilotClient, PermissionHandler
+from copilot import CopilotClient, PermissionHandler, StopError
 
 from .testharness import CLI_PATH
 
@@ -20,8 +20,7 @@ async def test_should_start_and_connect_to_server_using_stdio(self):
             assert pong.message == "pong: test message"
             assert pong.timestamp >= 0
 
-            errors = await client.stop()
-            assert len(errors) == 0
+            await client.stop()
             assert client.get_state() == "disconnected"
         finally:
             await client.force_stop()
@@ -38,14 +37,13 @@ async def test_should_start_and_connect_to_server_using_tcp(self):
             assert pong.message == "pong: test message"
             assert pong.timestamp >= 0
 
-            errors = await client.stop()
-            assert len(errors) == 0
+            await client.stop()
             assert client.get_state() == "disconnected"
         finally:
             await client.force_stop()
 
     @pytest.mark.asyncio
-    async def test_should_return_errors_on_failed_cleanup(self):
+    async def test_should_raise_exception_group_on_failed_cleanup(self):
         import asyncio
 
         client = CopilotClient({"cli_path": CLI_PATH})
@@ -59,9 +57,11 @@ async def test_should_return_errors_on_failed_cleanup(self):
             process.kill()
             await asyncio.sleep(0.1)
 
-            errors = await client.stop()
-            assert len(errors) > 0
-            assert "Failed to destroy session" in errors[0].message
+            with pytest.raises(ExceptionGroup) as exc_info:
+                await client.stop()
+            assert len(exc_info.value.exceptions) > 0
+            assert isinstance(exc_info.value.exceptions[0], StopError)
+            assert "Failed to destroy session" in exc_info.value.exceptions[0].message
         finally:
             await client.force_stop()
 

python/e2e/testharness/context.py

@@ -77,7 +77,10 @@ async def teardown(self, test_failed: bool = False):
             test_failed: If True, skip writing snapshots to avoid corruption.
         """
         if self._client:
-            await self._client.stop()
+            try:
+                await self._client.stop()
+            except ExceptionGroup:
+                pass  # stop() completes all cleanup before raising; safe to ignore in teardown
             self._client = None
 
         if self._proxy: