fix(transcript_mirror): swallow CancelledError in eager-flush done callback#943
Open
SuperMarioYL wants to merge 1 commit intoanthropics:mainfrom
Open
Conversation
…llback
Replace the lambda passed to ``add_done_callback`` with a small named
function that skips cancelled tasks before calling ``Task.exception()``.
Why
---
In Python 3.8+, ``Task.exception()`` raises ``CancelledError`` when the
task was cancelled. The previous lambda did not handle that, so asyncio's
callback machinery logged
ERROR asyncio: Exception in callback ... <lambda>(<Task cancell...>)
File ".../transcript_mirror_batcher.py", line 91, in <lambda>
self._flush_task.add_done_callback(lambda t: t.exception())
asyncio.exceptions.CancelledError
every time an in-flight eager-flush task was cancelled — i.e. on every
event-loop teardown with pending eager flushes, and whenever
``Query.close()`` cancels child tasks while a drain is mid-flight.
Functionality is unaffected (the inner ``_drain`` already handles
cancellation correctly via the ``async with self._lock``). The fix is
purely about silencing the ``Exception in callback`` log noise that
otherwise pollutes test output and production logs whenever the SDK
shuts down with pending eager flushes.
Test
----
Added ``test_eager_flush_cancellation_does_not_log_callback_error``
which:
1. Builds a batcher with ``max_pending_entries=0`` so any enqueue
schedules an eager flush.
2. Uses a hanging store so the eager flush blocks indefinitely.
3. Cancels the in-flight flush task and waits for the done-callback
to fire.
4. Asserts that the ``asyncio`` logger recorded no
``Exception in callback`` records at ERROR level.
The test fails on master (one CancelledError record) and passes
with the fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #930.
The eager-flush task in
TranscriptMirrorBatcher.enqueueis registered withThe intent is to retrieve the task's exception so asyncio doesn't warn about an unretrieved one, but on a cancelled task
Task.exception()itself raisesCancelledError(Python 3.8+). asyncio's callback machinery then logsevery time an in-flight eager-flush task is cancelled — i.e. on every event-loop teardown with pending eager flushes, and whenever
Query.close()cancels child tasks while a drain is mid-flight. Functionality is unaffected (the inner_drainhandles cancellation correctly viaasync with self._lock); the bug is pure log noise but it pollutes test output and production logs for users running withsession_store_flush=\"eager\".Fix
Replace the lambda with a small named
_swallow_done_exceptionhelper that skips cancelled tasks before calling.exception():Equivalent to the inline form
lambda t: None if t.cancelled() else t.exception()but names the intent.Test
Added
TestTranscriptMirrorBatcher.test_eager_flush_cancellation_does_not_log_callback_error:max_pending_entries=0so any enqueue schedules an eager flush.caplog.at_level("ERROR", logger="asyncio")block.asynciologger recorded noException in callbackrecords at ERROR level.Verified the test fails on master (one CancelledError record) and passes with the fix.
Validation
python -m ruff check src/ tests/— cleanpython -m ruff format --check src/ tests/— cleanpython -m mypy src/— clean (Success: no issues found in 24 source files)python -m pytest tests/— 746 passed, 5 skipped (full suite)Notes
try/exceptin_drainandcloseis untouched.Captured log teardowntraceback (referenced in Bug: TranscriptMirrorBatcher.enqueue's add_done_callback lambda raises CancelledError when task is cancelled #930) is the same bug's symptom; this fix should clear that log noise from those tests as well.