Skip to content

Commit c509f70

Browse files
torrid-fishclaude
andcommitted
fix(accent): cancel in-flight chunk tasks on client disconnect
schedule_chunks created detached asyncio tasks that kept scraping OJAD even after the client went away (PR #53 review): on the streaming endpoint a disconnect just stopped consuming the generator, and on the collected endpoint a cancelled handler orphaned its tasks. Add a shared cancel_pending helper and call it from a finally in both endpoints, so a disconnect (GeneratorExit into the stream, or the collected handler being cancelled) tears down any still-pending chunk. A TaskGroup would scope the tasks automatically, but async with TaskGroup() inside the streaming async generator wraps the aclose() GeneratorExit into a BaseExceptionGroup, so explicit cancellation is the only shape that closes the stream cleanly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent eb72d9d commit c509f70

2 files changed

Lines changed: 77 additions & 40 deletions

File tree

api/accent/pipeline.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,14 @@ def schedule_chunks(
244244
httpx errors. Cap in-flight work so well-behaved inputs still
245245
parallelise (a 4-chunk paragraph fans out fully) without hammering
246246
upstream.
247+
248+
Tasks run detached on the event loop; the caller owns their lifetime
249+
and MUST drain them through `cancel_pending` in a `finally` so a client
250+
disconnect doesn't leave them scraping OJAD for a response nobody reads.
251+
(A `TaskGroup` would tie the lifetime up automatically, but `async with
252+
TaskGroup()` inside the streaming async generator wraps the `aclose()`
253+
`GeneratorExit` into a `BaseExceptionGroup` — so explicit cancellation is
254+
the only shape that closes the stream cleanly.)
247255
"""
248256
semaphore = asyncio.Semaphore(4)
249257

@@ -258,3 +266,19 @@ async def run_chunk(line: str) -> AccentResponse:
258266
)
259267

260268
return [asyncio.create_task(run_chunk(text)) for _, _, text in chunks]
269+
270+
271+
async def cancel_pending(tasks: list[asyncio.Task[AccentResponse]]) -> None:
272+
"""Cancel any not-yet-finished chunk tasks and await their teardown.
273+
274+
Called from both endpoints' `finally` so a client disconnect — the
275+
`GeneratorExit` thrown into the streaming response, or the collected
276+
request handler being cancelled — stops in-flight OJAD scrapes instead
277+
of orphaning them. On normal completion every task is already done, so
278+
this is a no-op `gather` over finished tasks.
279+
"""
280+
for task in tasks:
281+
if not task.done():
282+
task.cancel()
283+
if tasks:
284+
await asyncio.gather(*tasks, return_exceptions=True)

api/accent/routes.py

Lines changed: 53 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
from fastapi.responses import StreamingResponse
2626

2727
from api.accent.models import AccentResponse, ErrorInfo, Request, WordAccentResult
28-
from api.accent.pipeline import build_chunks, schedule_chunks
28+
from api.accent.pipeline import build_chunks, cancel_pending, schedule_chunks
2929
from api.dependencies import get_http_client
3030

3131
logger = logging.getLogger("api")
@@ -73,26 +73,32 @@ async def mark_accent(
7373
worst_status = 200
7474
first_error: ErrorInfo | None = None
7575
first_warning: str | None = None
76-
for (chunk_idx, sub_idx, _text), task in zip(chunks, tasks):
77-
try:
78-
resp = await task
79-
except Exception as exc:
80-
logger.exception(f"Chunk {chunk_idx}.{sub_idx} failed")
81-
detail = str(exc) or repr(exc) or type(exc).__name__
82-
if first_error is None:
83-
first_error = ErrorInfo(code=500, message=f"Error: {detail}")
84-
worst_status = max(worst_status, 500)
85-
continue
86-
if resp.result:
87-
merged.extend(resp.result)
88-
if resp.status > worst_status:
89-
worst_status = resp.status
90-
if resp.error is not None and first_error is None:
91-
first_error = resp.error
92-
# Like `error`, `warning` keeps the first chunk's value — chunks all
93-
# degrade the same way (e.g. OJAD down), so one message suffices.
94-
if resp.warning is not None and first_warning is None:
95-
first_warning = resp.warning
76+
# `cancel_pending` in the finally stops in-flight chunks if the client
77+
# disconnects mid-request (the handler is cancelled) instead of leaving
78+
# them scraping OJAD; on a normal run every task is already done.
79+
try:
80+
for (chunk_idx, sub_idx, _text), task in zip(chunks, tasks):
81+
try:
82+
resp = await task
83+
except Exception as exc:
84+
logger.exception(f"Chunk {chunk_idx}.{sub_idx} failed")
85+
detail = str(exc) or repr(exc) or type(exc).__name__
86+
if first_error is None:
87+
first_error = ErrorInfo(code=500, message=f"Error: {detail}")
88+
worst_status = max(worst_status, 500)
89+
continue
90+
if resp.result:
91+
merged.extend(resp.result)
92+
if resp.status > worst_status:
93+
worst_status = resp.status
94+
if resp.error is not None and first_error is None:
95+
first_error = resp.error
96+
# Like `error`, `warning` keeps the first chunk's value — chunks
97+
# all degrade the same way (e.g. OJAD down), so one suffices.
98+
if resp.warning is not None and first_warning is None:
99+
first_warning = resp.warning
100+
finally:
101+
await cancel_pending(tasks)
96102

97103
return AccentResponse(
98104
status=worst_status,
@@ -134,24 +140,31 @@ async def generate() -> AsyncIterator[bytes]:
134140
render_katakana_furigana=request.render_katakana_furigana,
135141
script=request.script,
136142
)
137-
for (chunk_idx, sub_idx, _text), task in zip(chunks, tasks):
138-
try:
139-
resp = await task
140-
payload: dict[str, Any] = {
141-
"chunk": chunk_idx,
142-
"subchunk": sub_idx,
143-
**resp.model_dump(),
144-
}
145-
except Exception as exc:
146-
logger.exception(f"Streaming chunk {chunk_idx}.{sub_idx} failed")
147-
detail = str(exc) or repr(exc) or type(exc).__name__
148-
payload = {
149-
"chunk": chunk_idx,
150-
"subchunk": sub_idx,
151-
"status": 500,
152-
"result": None,
153-
"error": {"code": 500, "message": f"Error: {detail}"},
154-
}
155-
yield (json.dumps(payload, ensure_ascii=False) + "\n").encode("utf-8")
143+
# If the client disconnects mid-stream, Starlette throws GeneratorExit
144+
# into the paused `yield`; the finally then cancels every still-pending
145+
# chunk instead of running them to completion against OJAD for output
146+
# nobody will read.
147+
try:
148+
for (chunk_idx, sub_idx, _text), task in zip(chunks, tasks):
149+
try:
150+
resp = await task
151+
payload: dict[str, Any] = {
152+
"chunk": chunk_idx,
153+
"subchunk": sub_idx,
154+
**resp.model_dump(),
155+
}
156+
except Exception as exc:
157+
logger.exception(f"Streaming chunk {chunk_idx}.{sub_idx} failed")
158+
detail = str(exc) or repr(exc) or type(exc).__name__
159+
payload = {
160+
"chunk": chunk_idx,
161+
"subchunk": sub_idx,
162+
"status": 500,
163+
"result": None,
164+
"error": {"code": 500, "message": f"Error: {detail}"},
165+
}
166+
yield (json.dumps(payload, ensure_ascii=False) + "\n").encode("utf-8")
167+
finally:
168+
await cancel_pending(tasks)
156169

157170
return StreamingResponse(generate(), media_type="application/x-ndjson")

0 commit comments

Comments
 (0)