Conversation
After a transient stream error (UNAVAILABLE, UNKNOWN, INTERNAL), next_message() reconnected and then returned None instead of reading from the new stream. It now reads from the new stream, and raises after MAX_RECONNECT_ATTEMPTS reconnects in one call. The async client does the same and now also retries INTERNAL. The request iterator of a failed stream kept waiting on the shared send queue, so after a reconnect it took the next ack and sent it to the dead stream. Each stream now has its own send queue, and closing a stream ends its request iterator. close() now stays closed: a reconnect already in progress no longer reopens the stream. The close function from subscribe_with_handler waits up to SUBSCRIPTION_CLOSE_TIMEOUT_SECONDS for the handler thread, so test_subscribe_topic_with_handler no longer leaves that thread running and its coverage no longer changes between runs. Fixes dapr#1230 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Casper Nielsen <casper@diagrid.io>
A stream the server ends cleanly (StopIteration in the sync client, grpc.aio.EOF in the async one) now goes through the same bounded reconnect as UNAVAILABLE, UNKNOWN and INTERNAL, instead of failing with "Error while fetching message". The async docstring no longer promises None at end of stream. Reconnects inside one next_message() call now wait a short jittered exponential backoff between attempts. close() ends that wait at once. After MAX_RECONNECT_ATTEMPTS the clients raise the new StreamReconnectError, a subclass of Exception. The async start() now raises StreamInactiveError when close() runs during the initial read, as the sync one does. A cancellation while the subscription is still open is re-raised. Each message remembers the send queue of the stream it came from, and respond() drops a response to a message from a replaced stream, with a debug log. The runtime only logs an error for such an ack, but it never has to see it now. The sync close function logs a warning when the handler thread is still running after SUBSCRIPTION_CLOSE_TIMEOUT_SECONDS. The async subscription logs through a module logger instead of print(). The unreachable queue.Empty handler is gone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Casper Nielsen <casper@diagrid.io>
Resolve conflicts with dapr#1234 in the sync subscription. Keep this branch's per-stream send queue with a None sentinel and activation under the lock, and take dapr#1234's activation before the stream is opened and its reset to inactive when the initial read fails. Keep dapr#1234's tests. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Casper Nielsen <casper@diagrid.io>
The sync handler loop gave up after one failed reconnect: it slept 5 seconds, then read from the still-inactive stream, got StreamInactiveError and exited. It now retries the reconnect every SUBSCRIPTION_RECONNECT_BACKOFF_SECONDS until the close function is called. The close function wakes a loop that is waiting in the backoff, so it no longer takes up to 5 seconds to return. An exception from the handler was handled as a stream error, so the stream was reconnected and the message was delivered again. It is now logged with its traceback, the message gets a retry response, and the loop reads the next message. The async handler loop now handles errors the same way as the sync one. Before, any error other than StreamInactiveError ended the task, and the error only showed up later as "Task exception was never retrieved". The task is now kept referenced, so it can't be garbage-collected while it runs, and the close function waits up to SUBSCRIPTION_CLOSE_TIMEOUT_SECONDS for it to finish, except when called from inside the handler. test_subscribe_topic_with_handler is restored. Fixes dapr#1233 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Casper Nielsen <casper@diagrid.io>
When next_message() gives up or raises a non-retryable error, the handler loop now waits SUBSCRIPTION_RECONNECT_BACKOFF_SECONDS before reconnecting, in both clients. Closing ends the wait at once. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Casper Nielsen <casper@diagrid.io>
3 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1235 +/- ##
==========================================
+ Coverage 83.89% 84.38% +0.48%
==========================================
Files 123 123
Lines 10265 10400 +135
==========================================
+ Hits 8612 8776 +164
+ Misses 1653 1624 -29 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
3 tasks
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.
Description
Based on #1231. Merge it after #1231. Until #1231 merges, this diff also shows #1231's commits; the changes of its own are the last two commits (0844c95, 942c58d). It reuses
_closed,_close_stream(), the per-stream send queue and the fake sidecar hooks from #1231.subscribe_with_handlercould stop delivering messages without any error. This PR fixes it in both clients.SUBSCRIPTION_RECONNECT_BACKOFF_SECONDS(5) until the close function is called. The close function also ends a loop that is waiting to retry, so it returns right away.next_message()gives up (after its own bounded reconnects from Return the next message after a streaming subscription reconnects #1231) or raises a non-retryable error, the loop now waitsSUBSCRIPTION_RECONNECT_BACKOFF_SECONDSbefore reconnecting. Before, a stream that kept failing right after connecting was reconnected with no pause, forever. Closing ends the wait at once.StreamInactiveErrorended the handler task, and it only showed up later as "Task exception was never retrieved". The task now follows the same rules as the sync loop. It reconnects on stream errors, retries after a failed reconnect, and stops only after the caller has closed the subscription.grpc.aioraisesasyncio.CancelledErrorwhen reading a stream thatclose()cancelled. That counts as a normal exit only after a close, and is re-raised otherwise.SUBSCRIPTION_CLOSE_TIMEOUT_SECONDSfor the handler task to finish. It skips the wait when it is called from inside the handler.Tests:
test_subscribe_topic_with_handleris restored.test_subscribe_topic_with_handler_currently_reconnects_after_handler_errordocumented the old handler-error behaviour. It is replaced by a test that expects a retry response.Issue reference
Please reference the issue this PR will close: Fixes #1233
Checklist
RELEASE NOTE: FIX
subscribe_with_handlerkeeps retrying while the sidecar is unavailable, pauses between reconnects, does not reconnect on handler errors, and the async version no longer stops silently.🤖 Generated with Claude Code