Skip to content

Commit af4589f

Browse files
committed
fix(streaming): stop suppressing context errors and publish DONE once
StreamingTaskMessageContext.__aexit__ returned close()'s TaskMessage, and a truthy __aexit__ return value suppresses the exception. An error raised inside `async with streaming_task_message_context(...)` was therefore swallowed: the caller carried on past the block, the half-written message was persisted as DONE, and a Temporal activity saw a success and never retried. __aexit__ now closes the context and returns False, matching the convention the inference-call context manager already follows. Separately, stream_update() published an explicit StreamTaskMessageDone itself and then called close(), which published a second DONE because _is_closed was still False. Two DONE frames reached the stream, and since close() drains the coalescing buffer before publishing, buffered deltas could land after the first one. The Done branch now delegates to close(), which reaps the buffer, publishes exactly one DONE and persists. Full and delta handling are unchanged. Verified with `uv run pytest -q -n 0` on tests/lib/core/services/adk/test_streaming.py: the three added regression tests fail before the fix (two "DID NOT RAISE RuntimeError", one "expected exactly one DONE publish, got 2") and the file is 37 passed after. tests/lib/adk, tests/lib/core/harness, tests/lib/test_claude_agents_hooks.py and the openai_agents test_streaming_model.py suite stay green.
1 parent 6d68f3a commit af4589f

2 files changed

Lines changed: 81 additions & 7 deletions

File tree

‎src/agentex/lib/core/services/adk/streaming.py‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -395,7 +395,14 @@ async def __aenter__(self) -> "StreamingTaskMessageContext":
395395
return await self.open()
396396

397397
async def __aexit__(self, exc_type, exc_val, exc_tb):
398-
return await self.close()
398+
"""Close the context, then let any exception from the body propagate.
399+
400+
Returning ``close()``'s truthy ``TaskMessage`` here would suppress it,
401+
silently persisting a half-written message as DONE and hiding the
402+
failure from the caller (and from Temporal's activity retries).
403+
"""
404+
await self.close()
405+
return False
399406

400407
async def open(self) -> "StreamingTaskMessageContext":
401408
self._is_closed = False
@@ -484,7 +491,9 @@ async def stream_update(self, update: TaskMessageUpdate) -> TaskMessageUpdate |
484491
485492
``StreamTaskMessageDone`` and ``StreamTaskMessageFull`` updates always
486493
publish synchronously regardless of mode so consumers and persistence
487-
stay in sync.
494+
stay in sync. A Done delegates its publish to ``close()``, which drains
495+
the buffer first and emits exactly one DONE; publishing it here as well
496+
would put a second DONE on the stream, ahead of the buffered deltas.
488497
"""
489498
if self._is_closed:
490499
raise ValueError("Context is already done")
@@ -501,6 +510,10 @@ async def stream_update(self, update: TaskMessageUpdate) -> TaskMessageUpdate |
501510
await self._buffer.add(update)
502511
return update
503512

513+
if isinstance(update, StreamTaskMessageDone):
514+
await self.close()
515+
return update
516+
504517
# A Full ends the stream and supersedes buffered deltas. Drain and stop
505518
# the buffer BEFORE publishing the Full, so leftover deltas land in order
506519
# (deltas -> Full) instead of trailing the terminal Full as a stale
@@ -511,10 +524,7 @@ async def stream_update(self, update: TaskMessageUpdate) -> TaskMessageUpdate |
511524

512525
result = await self._streaming_service.stream_update(update)
513526

514-
if isinstance(update, StreamTaskMessageDone):
515-
await self.close()
516-
return update
517-
elif isinstance(update, StreamTaskMessageFull):
527+
if isinstance(update, StreamTaskMessageFull):
518528
await self._agentex_client.messages.update(
519529
task_id=self.task_id,
520530
message_id=update.parent_task_message.id, # type: ignore[union-attr]

‎tests/lib/core/services/adk/test_streaming.py‎

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
ReasoningSummaryDelta,
2424
)
2525
from agentex.types.task_message_update import (
26+
StreamTaskMessageDone,
2627
StreamTaskMessageFull,
2728
StreamTaskMessageDelta,
2829
)
@@ -62,7 +63,8 @@ def _reasoning_summary(tm: TaskMessage, idx: int, s: str) -> StreamTaskMessageDe
6263
)
6364

6465

65-
async def _make_context(streaming_mode: str) -> tuple[StreamingTaskMessageContext, MagicMock, TaskMessage]:
66+
def _make_unopened_context(streaming_mode: str) -> tuple[StreamingTaskMessageContext, MagicMock, TaskMessage]:
67+
"""Wired-up context that has not been opened yet, for ``async with`` tests."""
6668
tm = TaskMessage(
6769
id="m1",
6870
task_id="t1",
@@ -81,6 +83,11 @@ async def _make_context(streaming_mode: str) -> tuple[StreamingTaskMessageContex
8183
streaming_service=svc,
8284
streaming_mode=streaming_mode, # type: ignore[arg-type]
8385
)
86+
return ctx, svc, tm
87+
88+
89+
async def _make_context(streaming_mode: str) -> tuple[StreamingTaskMessageContext, MagicMock, TaskMessage]:
90+
ctx, svc, tm = _make_unopened_context(streaming_mode)
8491
await ctx.open()
8592
return ctx, svc, tm
8693

@@ -597,3 +604,60 @@ async def test_full_is_terminal_publish_no_trailing_deltas(self) -> None:
597604
assert any(isinstance(u, StreamTaskMessageDelta) for u in published[:-1]), (
598605
"expected the buffered deltas to be published before the Full"
599606
)
607+
608+
609+
class TestContextDoesNotSuppressErrors:
610+
"""``__aexit__`` returned ``close()``'s TaskMessage, which is truthy, so an
611+
exception raised inside ``async with`` was swallowed: the caller carried on
612+
past the block, the half-written message was persisted DONE, and a Temporal
613+
activity saw a success and never retried."""
614+
615+
@pytest.mark.asyncio
616+
async def test_exception_inside_context_propagates(self) -> None:
617+
ctx, _svc, tm = _make_unopened_context("off")
618+
619+
with pytest.raises(RuntimeError, match="model blew up"):
620+
async with ctx as entered:
621+
await entered.stream_update(_text(tm, "partial"))
622+
raise RuntimeError("model blew up")
623+
624+
@pytest.mark.asyncio
625+
async def test_context_is_still_closed_when_body_raises(self) -> None:
626+
"""Not suppressing must not mean leaking: DONE is still published and
627+
persisted so consumers and the buffer ticker are not left hanging."""
628+
ctx, svc, _tm = _make_unopened_context("off")
629+
630+
with pytest.raises(RuntimeError):
631+
async with ctx:
632+
raise RuntimeError("model blew up")
633+
634+
assert ctx._is_closed
635+
published = [c.args[0] for c in svc.stream_update.await_args_list]
636+
assert isinstance(published[-1], StreamTaskMessageDone)
637+
update_kwargs = ctx._agentex_client.messages.update.call_args.kwargs
638+
assert update_kwargs["streaming_status"] == "DONE"
639+
640+
641+
class TestExplicitDonePublishesOnce:
642+
"""An explicit ``StreamTaskMessageDone`` used to be published by
643+
``stream_update`` and then published a second time by the ``close()`` it
644+
triggers, putting two DONE frames on the stream. Routing the terminal
645+
publish through ``close()`` also keeps buffered deltas ahead of the DONE."""
646+
647+
@pytest.mark.asyncio
648+
async def test_explicit_done_publishes_exactly_one_done(self) -> None:
649+
ctx, svc, tm = await _make_context("coalesced")
650+
await ctx.stream_update(_text(tm, "hello"))
651+
652+
await ctx.stream_update(StreamTaskMessageDone(parent_task_message=tm, type="done"))
653+
654+
published = [c.args[0] for c in svc.stream_update.await_args_list]
655+
dones = [u for u in published if isinstance(u, StreamTaskMessageDone)]
656+
assert len(dones) == 1, f"expected exactly one DONE publish, got {len(dones)}"
657+
assert published[-1] is dones[0], (
658+
f"DONE must be the terminal publish; saw trailing {type(published[-1]).__name__} after it"
659+
)
660+
assert ctx._agentex_client.messages.update.call_count == 1
661+
update_kwargs = ctx._agentex_client.messages.update.call_args.kwargs
662+
assert update_kwargs["content"]["content"] == "hello"
663+
assert update_kwargs["streaming_status"] == "DONE"

0 commit comments

Comments
 (0)