Skip to content

Commit 933653c

Browse files
committed
fix(tools): Prevent session drop on MCP tool error
Enable MCP graceful error handling by default and retrieve background session context task exceptions. This prevents unhandled AnyIO TaskGroup transport failures from bubbling through the event loop and abruptly terminating live streaming sessions with 1006 abnormal closure. Change-Id: Ib4669fb57cddfc6ce61f9a3991b80db3f2b3e9ab
1 parent 84fa984 commit 933653c

3 files changed

Lines changed: 27 additions & 15 deletions

File tree

src/google/adk/features/_feature_registry.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ class FeatureConfig:
138138
FeatureStage.EXPERIMENTAL, default_on=True
139139
),
140140
FeatureName._MCP_GRACEFUL_ERROR_HANDLING: FeatureConfig(
141-
FeatureStage.EXPERIMENTAL, default_on=False
141+
FeatureStage.EXPERIMENTAL, default_on=True
142142
),
143143
FeatureName.PROGRESSIVE_SSE_STREAMING: FeatureConfig(
144144
FeatureStage.EXPERIMENTAL, default_on=True

src/google/adk/tools/mcp_tool/session_context.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,12 @@ async def start(self) -> ClientSession:
130130
if not self._task:
131131
self._task = asyncio.create_task(self._run())
132132

133+
def _retrieve_exception(t: asyncio.Task):
134+
if not t.cancelled():
135+
t.exception()
136+
137+
self._task.add_done_callback(_retrieve_exception)
138+
133139
await self._ready_event.wait()
134140

135141
if self._task.cancelled():

tests/unittests/tools/mcp_tool/test_mcp_session_manager.py

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -588,6 +588,9 @@ async def test_create_session_cleans_up_without_aclose_if_loop_is_different(
588588
self,
589589
):
590590
"""Verify that sessions from different loops are cleaned up without calling aclose()."""
591+
from google.adk.features import FeatureName
592+
from google.adk.features._feature_registry import temporary_feature_override
593+
591594
manager = MCPSessionManager(self.mock_stdio_connection_params)
592595

593596
# 1. Simulate a session created in a "different" loop
@@ -617,8 +620,11 @@ async def test_create_session_cleans_up_without_aclose_if_loop_is_different(
617620
mock_wait_for.return_value = new_session
618621
mock_session_context_class.return_value = AsyncMock()
619622

620-
# 3. Call create_session
621-
session = await manager.create_session()
623+
# 3. Call create_session with flag off to hit wait_for branch
624+
with temporary_feature_override(
625+
FeatureName._MCP_GRACEFUL_ERROR_HANDLING, False
626+
):
627+
session = await manager.create_session()
622628

623629
# 4. Verify results
624630
assert session == new_session
@@ -969,8 +975,8 @@ class TestMCPGracefulErrorHandlingFlagContract:
969975
loudly so we don't silently break GE's rollout.
970976
"""
971977

972-
def test_default_state_is_off_so_cl_is_a_noop(self):
973-
"""The CL must be a no-op until GE explicitly enables it."""
978+
def test_default_state_is_on(self):
979+
"""The fix must be enabled by default."""
974980
import os
975981

976982
from google.adk.features import FeatureName
@@ -981,34 +987,34 @@ def test_default_state_is_off_so_cl_is_a_noop(self):
981987
saved = {k: os.environ.pop(k) for k in (enable, disable) if k in os.environ}
982988
try:
983989
assert (
984-
is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING) is False
990+
is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING) is True
985991
)
986992
finally:
987993
os.environ.update(saved)
988994

989-
def test_env_var_enable_flips_flag_on_at_runtime(self):
990-
"""The env var GE will set must turn the fix on without a rebuild."""
995+
def test_env_var_disable_flips_flag_off_at_runtime(self):
996+
"""The env var must turn the fix off without a rebuild."""
991997
import os
992998

993999
from google.adk.features import FeatureName
9941000
from google.adk.features import is_feature_enabled
9951001

996-
enable = "ADK_ENABLE_MCP_GRACEFUL_ERROR_HANDLING"
997-
saved = os.environ.pop(enable, None)
1002+
disable = "ADK_DISABLE_MCP_GRACEFUL_ERROR_HANDLING"
1003+
saved = os.environ.pop(disable, None)
9981004
try:
999-
os.environ[enable] = "1"
1005+
os.environ[disable] = "1"
10001006
assert (
1001-
is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING) is True
1007+
is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING) is False
10021008
)
10031009
# And once it's removed, we revert. Confirms the value is read
10041010
# live from os.environ on every call (no caching, no binary push).
1005-
del os.environ[enable]
1011+
del os.environ[disable]
10061012
assert (
1007-
is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING) is False
1013+
is_feature_enabled(FeatureName._MCP_GRACEFUL_ERROR_HANDLING) is True
10081014
)
10091015
finally:
10101016
if saved is not None:
1011-
os.environ[enable] = saved
1017+
os.environ[disable] = saved
10121018

10131019
def test_env_var_disable_acts_as_kill_switch(self):
10141020
"""The disable env var lets consumers turn off without a rebuild."""

0 commit comments

Comments
 (0)