fix: return task immediately when blocking=False in on_message_send#952
Open
ruimgf wants to merge 4 commits intoa2aproject:mainfrom
Open
fix: return task immediately when blocking=False in on_message_send#952ruimgf wants to merge 4 commits intoa2aproject:mainfrom
ruimgf wants to merge 4 commits intoa2aproject:mainfrom
Conversation
When `blocking=False` is set in MessageSendConfiguration, the handler now returns the Task object immediately without waiting for executor events. Event consumption and push notifications are processed entirely in the background. Previously, even with `blocking=False`, the handler waited for the first event from the EventConsumer polling loop (0.5s timeout per iteration). When the event loop was busy with background work, this caused 5-7s delays before the HTTP response was sent, leading to client timeouts. The new non-blocking fast path: 1. Creates the task and starts the executor (via _setup_message_execution) 2. Returns the task in 'submitted' state immediately 3. Consumes events and sends push notifications in a background task The blocking path remains unchanged for backward compatibility. Fixes a2aproject#951 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a non-blocking execution path for message handling, allowing tasks to be returned immediately while processing continues in the background. A critical issue was identified where the task state is not persisted to the store before returning to the client, which may cause race conditions during subsequent task lookups.
- Extract non-blocking logic to _on_message_send_non_blocking to fix PLR0915 (too many statements) lint error - Update test_on_message_send_with_push_notification_in_non_blocking_request to test the new immediate-return behavior instead of mocking consume_and_break_on_interrupt - Update test_on_message_send_non_blocking history assertion to account for the immediate return having only the initial user message Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Move background consume logic to _consume_and_notify_in_background method to reduce argument count and keep non-blocking path inline in on_message_send - Fix test assertion: use push_store.get_info() instead of assert_awaited_once_with since InMemoryPushNotificationConfigStore is not a mock Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🧪 Code Coverage (vs
|
| Base | PR | Delta | |
|---|---|---|---|
| src/a2a/server/request_handlers/default_request_handler.py | 97.34% | 95.83% | 🔴 -1.51% |
| src/a2a/server/tasks/result_aggregator.py | 100.00% | 97.14% | 🔴 -2.86% |
| Total | 91.72% | 91.62% | 🔴 -0.10% |
Generated by coverage-comment.yml
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
1 participant
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
blocking=False, return theTaskimmediately without waiting for executor eventsProblem
Even with
blocking=False,on_message_sendwaits for the first event from theEventConsumerpolling loop (0.5s timeout per iteration). When the event loop is busy with background work from previous requests, this causes 5-7s delays before the HTTP response is sent, leading to client timeouts.The root cause is that
consume_and_break_on_interruptpolls theEventQueuewaiting for the executor to emit its first event. The executor runs viaasyncio.create_taskbut may not be scheduled by the event loop between consumer polls, especially under load.Solution
Add a non-blocking fast path at the top of
on_message_send:_setup_message_execution(creates task, starts executor)submittedstate immediatelyThis reduced HTTP response times from 5-7s to ~3ms in production.
Test plan
blocking=True(default) behavior is unchangedblocking=Falsereturns task immediately without waiting for eventsFixes #951
🤖 Generated with Claude Code