Repository navigation
Conversation
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.
Closing a listener after a delivery currently returns while RabbitMQ still has an active consumer.
listen()does not await closure of its nested merged generator, so cancellation of the pump tasks and queue iterators depends on a later async-generator finalizer. This leaves a window for the abandoned listener to receive/prefetch more deliveries.Wrap the merged generator in
contextlib.aclosingso explicit close or an exception thrown at the outer yield waits for the existing pump/queue cleanup. The shared broker channels remain available for subsequent listeners. There is no change to routing, acknowledgements, retries, or public signatures.Fixes #60. Assignment has been requested; no assignee has been set.
Validation actually run
Environment: Windows, Python 3.14.7, locked aio-pika 9.5.8 / aiormq 6.9.2, and a real isolated RabbitMQ 4.3.0 Docker container over loopback AMQP.
pytest tests/test_listener.py -q: 6 passed on the patch. The same tests on unmodified masterf63aa6057475f0c83605245b5255d6a9ffc9c589failed 6/6 in an isolated sequential run. Explicit close/throw fail on the first server-side consumer-count check; cancellation already cleans the first listener on baseline, but its subsequent explicitly closed listener fails the recovery check.aclose,athrow, cancellation of a pending receive, server-side consumer counts after close returns, acknowledgement, and reuse of the shared channel to receive/ack the next delivery. No AMQP client/server is mocked.-n 2, excluding two tests that require the unavailable delayed-message plugin: 36 passed, 2 failed. The exact two existing failure nodes also fail in the same environment on unmodified master:test_when_delivery_limit_exceeded__message_is_dead_lettered(QueueEmptynot raised), andTestStartup::test_when_declare_flag_not_passed_to_queue__broker_does_not_declare_queue_on_startup(RabbitMQ 4.3 rejects deprecated non-durable, non-exclusive queues). The baseline broader run, including the copied new tests, was 31 passed, 7 failed: these same two existing failures plus five new regression failures; the cancellation/recovery timing case can already pass on baseline due to deferred finalization.ty check ./taskiq_aio_pika ./testspassed. All repositoryprek run --all-fileshooks passed after staging both changed files, including typos and detect-secrets.git diff --cached --checkpassed.Not run locally: the two delayed-message-plugin tests, a complete Taskiq CLI worker-process end-to-end test, broker outage/reconnection, or the Python/OS CI matrix. Existing full-suite failures remain unresolved by this focused patch; CI needs its configured RabbitMQ/plugin environment.
Review status
AI-assisted investigation, implementation, and testing. Please keep this PR in Draft pending independent review and account-owner review. No completed human review is claimed.