Skip to content

Commit b302339

Browse files
committed
fix: improve cleanup error handling in CopilotClient and CopilotSession to propagate errors correctly
1 parent 468fd5c commit b302339

3 files changed

Lines changed: 13 additions & 171 deletions

File tree

python/copilot/client.py

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
import asyncio
1616
import inspect
17-
import logging
1817
import os
1918
import re
2019
import subprocess
@@ -238,8 +237,10 @@ async def __aexit__(
238237
Exit the async context manager.
239238
240239
Performs graceful cleanup by destroying all active sessions and stopping
241-
the CLI server. If cleanup errors occur, they are logged but do not
242-
prevent the context from exiting.
240+
the CLI server. If a cleanup error occurs and no exception was raised
241+
inside the context, the cleanup error is propagated. If an exception
242+
was already raised inside the context, the cleanup error is suppressed
243+
so the original exception is not masked.
243244
244245
Args:
245246
exc_type: The type of exception that occurred, if any.
@@ -250,14 +251,10 @@ async def __aexit__(
250251
False to propagate any exception that occurred in the context.
251252
"""
252253
try:
253-
stop_errors = await self.stop()
254-
# Log any session destruction errors
255-
if stop_errors:
256-
for error in stop_errors:
257-
logging.warning("Error during session cleanup in CopilotClient: %s", error)
254+
await self.stop()
258255
except Exception:
259-
# Log the error but don't raise - we want cleanup to always complete
260-
logging.warning("Error during CopilotClient cleanup", exc_info=True)
256+
if exc_type is None:
257+
raise
261258
return False
262259

263260
@property

python/copilot/session.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@
77

88
import asyncio
99
import inspect
10-
import logging
1110
import threading
1211
from types import TracebackType
1312
from typing import Any, Callable, Optional
@@ -113,8 +112,10 @@ async def __aexit__(
113112
Exit the async context manager.
114113
115114
Automatically destroys the session and releases all associated resources.
116-
If an error occurs during cleanup, it is logged but does not prevent the
117-
context from exiting.
115+
If a cleanup error occurs and no exception was raised inside the context,
116+
the cleanup error is propagated. If an exception was already raised inside
117+
the context, the cleanup error is suppressed so the original exception is
118+
not masked.
118119
119120
Args:
120121
exc_type: The type of exception that occurred, if any.
@@ -127,8 +128,8 @@ async def __aexit__(
127128
try:
128129
await self.destroy()
129130
except Exception:
130-
# Log the error but don't raise - we want cleanup to always complete
131-
logging.warning("Error during CopilotSession cleanup", exc_info=True)
131+
if exc_type is None:
132+
raise
132133
return False
133134

134135
@property

python/e2e/test_context_managers.py

Lines changed: 0 additions & 156 deletions
This file was deleted.

0 commit comments

Comments
 (0)