Skip to content

Commit a4c3ecc

Browse files
committed
fix(sender): settle sends when the loop terminates
Retain writer failures for active, queued and later sends. Preserve cancellation during shutdown and reject sends after closure, including cancellation before the sender loop starts. Developed and reviewed with AI assistance using Codex.
1 parent 4fc4841 commit a4c3ecc

3 files changed

Lines changed: 75 additions & 13 deletions

File tree

‎src/acp/task/sender.py‎

Lines changed: 63 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,27 @@ def __init__(self, writer: asyncio.StreamWriter, supervisor: TaskSupervisor) ->
2323
self._writer = writer
2424
self._queue: asyncio.Queue[_PendingSend | None] = asyncio.Queue()
2525
self._closed = False
26+
self._active: _PendingSend | None = None
27+
self._terminal_error: BaseException | None = None
28+
self._terminal_cancelled = False
2629
self._task = supervisor.create(self._loop(), name="acp.Sender.loop", on_error=self._on_error)
30+
self._task.add_done_callback(self._on_task_done)
2731

2832
async def send(self, payload: dict[str, Any]) -> None:
33+
if self._terminal_error is not None:
34+
if self._terminal_cancelled:
35+
raise asyncio.CancelledError
36+
error = self._terminal_error
37+
raise error
38+
if self._closed:
39+
raise asyncio.CancelledError
40+
if self._task.done():
41+
if self._task.cancelled():
42+
self._set_terminal(ConnectionError("Sender closed"), cancel_pending=True)
43+
raise asyncio.CancelledError
44+
error = self._task.exception() or ConnectionError("Sender closed")
45+
self._set_terminal(error)
46+
raise error
2947
data = (json.dumps(payload, separators=(",", ":")) + "\n").encode("utf-8")
3048
future: asyncio.Future[None] = asyncio.get_running_loop().create_future()
3149
await self._queue.put(_PendingSend(data, future))
@@ -46,18 +64,61 @@ async def _loop(self) -> None:
4664
item = await self._queue.get()
4765
if item is None:
4866
return
67+
self._active = item
4968
try:
5069
self._writer.write(item.payload)
5170
await self._writer.drain()
71+
except asyncio.CancelledError:
72+
self._set_terminal(ConnectionError("Sender closed"), cancel_pending=True)
73+
raise
5274
except Exception as exc:
53-
if not item.future.done():
54-
item.future.set_exception(exc)
75+
self._set_terminal(exc)
5576
raise
5677
else:
5778
if not item.future.done():
5879
item.future.set_result(None)
80+
finally:
81+
self._active = None
5982
except asyncio.CancelledError:
83+
self._set_terminal(ConnectionError("Sender closed"), cancel_pending=True)
84+
raise
85+
86+
def _set_terminal(self, error: BaseException, *, cancel_pending: bool = False) -> None:
87+
if self._terminal_error is not None:
6088
return
89+
self._terminal_error = error
90+
self._terminal_cancelled = cancel_pending
91+
if cancel_pending:
92+
self._cancel_pending()
93+
return
94+
if self._active is not None and not self._active.future.done():
95+
self._active.future.set_exception(error)
96+
while True:
97+
try:
98+
item = self._queue.get_nowait()
99+
except asyncio.QueueEmpty:
100+
break
101+
if item is not None and not item.future.done():
102+
item.future.set_exception(error)
103+
104+
def _cancel_pending(self) -> None:
105+
if self._active is not None and not self._active.future.done():
106+
self._active.future.cancel()
107+
while True:
108+
try:
109+
item = self._queue.get_nowait()
110+
except asyncio.QueueEmpty:
111+
break
112+
if item is not None and not item.future.done():
113+
item.future.cancel()
114+
115+
def _on_task_done(self, task: asyncio.Task[Any]) -> None:
116+
if task.cancelled():
117+
self._set_terminal(ConnectionError("Sender closed"), cancel_pending=True)
118+
else:
119+
error = task.exception()
120+
if error is not None:
121+
self._set_terminal(error)
61122

62123
def _on_error(self, task: asyncio.Task[Any], exc: BaseException) -> None:
63124
logging.exception("Send loop failed", exc_info=exc)

‎tests/test_connection_recovery.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
import asyncio
44
import contextlib
55
import json
6-
from typing import Any
6+
from typing import Any, cast
77
from unittest.mock import AsyncMock, MagicMock
88

99
import pytest
@@ -170,7 +170,7 @@ def tracking_process(message: dict[str, Any]) -> None:
170170
async def test_send_request_removes_pending_after_writer_failure() -> None:
171171
reader = asyncio.StreamReader()
172172
writer = _FailingWriter()
173-
conn = Connection(_noop_handler, writer, reader, listening=False)
173+
conn = Connection(_noop_handler, cast(asyncio.StreamWriter, writer), reader, listening=False)
174174
requests = [
175175
asyncio.create_task(conn.send_request("first"), name="request-first"),
176176
asyncio.create_task(conn.send_request("second"), name="request-second"),

‎tests/test_message_sender.py‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
import asyncio
44
import json
5-
from typing import Any
5+
from typing import Any, cast
66

77
import pytest
88

@@ -57,7 +57,7 @@ async def test_writer_failure_settles_active_queued_and_late_sends() -> None:
5757
error = BrokenPipeError("writer failed")
5858
writer = _ControlledWriter(error=error)
5959
supervisor = TaskSupervisor(source="test")
60-
sender = MessageSender(writer, supervisor)
60+
sender = MessageSender(cast(asyncio.StreamWriter, writer), supervisor)
6161
tasks: list[asyncio.Task[Any]] = []
6262

6363
try:
@@ -88,7 +88,7 @@ async def test_writer_failure_settles_active_queued_and_late_sends() -> None:
8888
async def test_supervisor_shutdown_settles_active_queued_and_late_sends() -> None:
8989
writer = _ControlledWriter()
9090
supervisor = TaskSupervisor(source="test")
91-
sender = MessageSender(writer, supervisor)
91+
sender = MessageSender(cast(asyncio.StreamWriter, writer), supervisor)
9292
tasks: list[asyncio.Task[Any]] = []
9393

9494
try:
@@ -105,8 +105,9 @@ async def test_supervisor_shutdown_settles_active_queued_and_late_sends() -> Non
105105
tasks.append(late)
106106
await _assert_tasks_finish(tasks)
107107

108-
for task in tasks:
109-
assert isinstance(task.exception(), ConnectionError)
108+
for task in tasks[:2]:
109+
assert task.cancelled()
110+
assert late.cancelled()
110111
finally:
111112
await _close_supervisor(supervisor, tasks)
112113

@@ -115,15 +116,15 @@ async def test_supervisor_shutdown_settles_active_queued_and_late_sends() -> Non
115116
async def test_shutdown_before_sender_loop_starts_rejects_later_send() -> None:
116117
writer = _ControlledWriter()
117118
supervisor = TaskSupervisor(source="test")
118-
sender = MessageSender(writer, supervisor)
119+
sender = MessageSender(cast(asyncio.StreamWriter, writer), supervisor)
119120
tasks: list[asyncio.Task[Any]] = []
120121

121122
try:
122123
await supervisor.shutdown()
123124
late = asyncio.create_task(sender.send({"id": 1}), name="send-after-shutdown")
124125
tasks.append(late)
125126
await _assert_tasks_finish(tasks)
126-
assert isinstance(late.exception(), ConnectionError)
127+
assert late.cancelled()
127128
finally:
128129
await _close_supervisor(supervisor, tasks)
129130

@@ -132,7 +133,7 @@ async def test_shutdown_before_sender_loop_starts_rejects_later_send() -> None:
132133
async def test_healthy_sends_preserve_order_and_graceful_close() -> None:
133134
writer = _ControlledWriter()
134135
supervisor = TaskSupervisor(source="test")
135-
sender = MessageSender(writer, supervisor)
136+
sender = MessageSender(cast(asyncio.StreamWriter, writer), supervisor)
136137
tasks: list[asyncio.Task[Any]] = []
137138
closing: asyncio.Task[None] | None = None
138139

@@ -157,7 +158,7 @@ async def test_healthy_sends_preserve_order_and_graceful_close() -> None:
157158
late = asyncio.create_task(sender.send({"id": 3}), name="send-after-close")
158159
tasks.append(late)
159160
await _assert_tasks_finish([late])
160-
assert isinstance(late.exception(), ConnectionError)
161+
assert late.cancelled()
161162
finally:
162163
if closing is not None and not closing.done():
163164
closing.cancel()

0 commit comments

Comments
 (0)