Port session_store path to anyio so it works under trio#990
Open
dsmilkov wants to merge 1 commit into
Open
Conversation
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #990 +/- ##
=======================================
Coverage ? 89.31%
=======================================
Files ? 23
Lines ? 3988
Branches ? 0
=======================================
Hits ? 3562
Misses ? 426
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…ons to anyio so session_store works under trio The SDK core is anyio-based and works on both asyncio and trio, but the session_store path (TranscriptMirrorBatcher, session_resume helpers, _derive_infos_via_load) used raw asyncio primitives, so passing session_store= to query() / ClaudeSDKClient crashed under trio at the first batcher flush. Port those three modules to anyio (Lock/sleep/fail_after/CapacityLimiter/ create_task_group) and the SDK's own spawn_detached. flush() simplifies to awaiting _drain(); close() shields its final flush so the last batch lands under a cancelled scope. No behavior change for asyncio callers. New tests/test_session_store_anyio.py runs 7 tests across both backends, limited to backend-divergent paths. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
c037b4a to
aa615ea
Compare
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.
The bug
The SDK's core (
_internal/query.py,transport/subprocess_cli.py) isanyio-based and works on both asyncio and trio — it even ships
_internal/_task_compat.spawn_detachedprecisely so background tasks workon either backend. But the
session_storecode path (TranscriptMirrorBatcher,parts of
session_resume.py, and_derive_infos_via_loadinsessions.py)uses raw
asyncio.ensure_future/asyncio.Lock/asyncio.wait_for/asyncio.sleep/asyncio.Semaphore/asyncio.gather.Under a trio app, passing
session_store=toquery()orClaudeSDKClientcrashes at the first batcher flush with:
(an
asyncio.Taskyielded to trio's event loop).The fix
Port those three modules to anyio, reusing the SDK's existing
spawn_detached:transcript_mirror_batcher.py—anyio.Lock,anyio.sleep,anyio.fail_after; eager-flush fires viaspawn_detached(self._drain())(handle stored in
_flush_task: TaskHandle | None);flush()simplyawaits
_drain()(the lock already serializes with in-flight eagerdrains, so the old Task + compare-and-null dance was redundant);
close()shields its final flush so the last batch lands even whenteardown runs under a cancelled scope.
_swallow_done_exceptionisremoved —
spawn_detachedhandles exception surfacing on both backends.session_resume.py—_with_timeout→anyio.fail_after;_rmtree_with_retry→anyio.sleep+anyio.get_cancelled_exc_class().The synchronous happy-path rmtree is preserved so cleanup still runs
under a cancelled scope.
sessions.py—_derive_infos_via_load→anyio.CapacityLimiter+anyio.create_task_group()with per-tasktry/except Exception, preserving the "one adapter failure degradesthat row" semantics.
Behavior
asyncio primitives;
spawn_detachedon asyncio isloop.create_task().append()that raisesasyncio.TimeoutErrorfrom its own internals is now retried(previously short-circuited). On 3.11+
asyncio.TimeoutError is TimeoutErrorso there's no change. Adapter-internal timeouts aren'tour
send_timeout, so retrying them is arguably more correct._derive_infos_via_loadno longer swallowsBaseException(cancellation / KeyboardInterrupt) from a
store.load()into anempty-summary row — it propagates.
Tests
tests/test_session_store_anyio.py: 7 tests ×["asyncio", "trio"],deliberately limited to backend-divergent paths — the existing per-module
suites already cover behavior in depth under pytest-asyncio. Covered: the
spawn_detachedeager-flush path (the original crash site), eager-flush /flush()interleaving order, send-timeout reporting viafail_after,close()flushing under a cancelled scope,_with_timeouttimeoutconversion,
_rmtree_with_retryunder a cancelled scope, andlist_sessions_from_storewith one failing load.tests/test_transcript_mirror.pyupdated for the removed_swallow_done_exception, theanyio.sleeppatch target, and theTaskHandletype of_flush_task.mypy --strictclean, ruff clean.