fix: harden event consumer and queue - #1040
Open
ez-lbz wants to merge 5 commits into
Open
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.
What changed
1. A plain
Messageevent no longer terminates the streamProblem: In
EventConsumer.consumeAll(), everyMessageevent was markedisFinalEvent = true, so the first message emitted by an agent closed the stream immediately. Per the A2A protocol (§3.1.6) the stream MUST terminate only when the task reaches a terminal state (completed, failed, canceled, rejected). An intermediateMessagesent before the agent finishes (or a message-only response with follow-up events) incorrectly cut off the rest of the stream — a client could miss the task's terminal status update. Note the queue machinery already treatedMessageas non-final (MainQueue.isFinalEventonly considersTask/TaskStatusUpdateEvent), so the consumer was inconsistent with the queue.Fix (server-common/src/main/java/org/a2aproject/sdk/server/events/EventConsumer.java):
event instanceof Message → isFinalEvent = truebranch. The stream now terminates only on a finalTaskStatusUpdateEvent, a terminalTask, aQueueClosedEvent, anA2AError, or via the existing agent-completed grace period when no final event arrives.Fix (server-common/src/test/java/org/a2aproject/sdk/server/events/EventConsumerTest.java):
testConsumeMessageEvents: two messages are now both delivered (the stream stays open after the first message) and terminates when the queue is closed.Behavior change: message-only streams no longer close on the first
Message; they close when the task reaches a terminal state, or ~1.5s after the agent completes (existing agent-completed grace period) when no terminal event is emitted. Clients that stream messages with a task lifecycle see no change in event delivery order — only the stream-close trigger changes.2. Buffer-flush delay is configurable instead of hardcoded
Problem:
EventConsumerblocked its polling thread with a hardcodedThread.sleep(150)(BUFFER_FLUSH_DELAY_MS) before completing a stream. The value was not tunable for transports where the flush needs differ.Fix (EventConsumer.java):
a2a.eventconsumer.bufferFlushDelayMssystem property (default 150 ms, clamped to >= 0; invalid values fall back to the default). Semantics are preserved: the delay still runs only once per stream, after the final event is sent, to let the SSE write callback fire beforetube.complete().bufferFlushDelayMs()accessor and tests (testBufferFlushDelayMsDefaultsTo150,testBufferFlushDelayMsReadsConfiguredValue,testBufferFlushDelayMsRejectsInvalidValues).Behavior change: none unless the system property is set.
3. ChildQueue is bounded by the parent queue size
Problem:
ChildQueuebuilt itsLinkedBlockingDequewithout a capacity, soqueue.offer()always returnedtrueand a slow subscriber could grow the deque without limit (the "queue is full" → immediate-close path was dead code).Fix (server-common/src/main/java/org/a2aproject/sdk/server/events/EventQueue.java):
ChildQueuenow constructs its deque with the parent'squeueSize(new LinkedBlockingDeque<>(parent.getQueueSize())), restoring the intended overflow → immediate-close behavior for slow consumers.Fix (server-common/src/test/java/org/a2aproject/sdk/server/events/EventQueueTest.java):
testChildQueueIsBoundedByParentQueueSizeverifies the child deque'sremainingCapacityequals the configured queue size.4. Semaphore permit is released when MainEventBus.submit fails
Problem:
MainQueue.enqueueItemacquires a semaphore permit, then callsmainEventBus.submit(...). Ifsubmitthrows (e.g. interrupted), the permit was never released — the normal release only happens inMainEventBusProcessor.processEvent'sfinally, which never runs for an unsubmitted event. Repeated failures would leak all permits and permanently block event processing for the task.Fix (EventQueue.java):
mainEventBus.submit(...)in try/catch: on aRuntimeExceptionthe acquired permit is released before rethrowing. The success path is unchanged (release still happens once inMainEventBusProcessor).Fix (EventQueueTest.java):
testSemaphorePermitReleasedWhenSubmitFailsmocksMainEventBus.submitto throw and assertsmainQueue.size()returns to 0 (no leaked permit).Testing
mvn -pl server-common test— 452 tests run, 0 failures, 0 errors, 0 skipped (BUILD SUCCESS), including 5 new/updated regression tests.mvn -pl transport/jsonrpc,transport/grpc,transport/rest test— 124 tests run, 0 failures, 1 skipped (BUILD SUCCESS); the streaming transports that consume the realEventConsumerare unaffected.