Skip to content

Commit ec93811

Browse files
committed
fix(server): preserve interrupts for protocol errors
This fixes #1183
1 parent 4db8bd1 commit ec93811

2 files changed

Lines changed: 69 additions & 18 deletions

File tree

‎server-common/src/main/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandler.java‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1261,16 +1261,18 @@ public void run() {
12611261
// These are expected business errors but should be tracked
12621262
LOGGER.warn("Agent execution threw A2AError for task {}: {} - {}",
12631263
taskId, e.getClass().getSimpleName(), e.getMessage(), e);
1264-
emitter.fail(e);
1264+
enqueueErrorPreservingInterrupt(emitter, e);
12651265
} catch (RuntimeException e) {
12661266
// Log unexpected runtime exceptions at ERROR level
12671267
// These indicate bugs in agent implementation
12681268
LOGGER.error("Agent execution threw unexpected RuntimeException for task {}", taskId, e);
1269-
reportAgentExecutionFailure(emitter, "Agent execution failed: " + e.getMessage());
1269+
enqueueErrorPreservingInterrupt(emitter,
1270+
new InternalError("Agent execution failed: " + e.getMessage()));
12701271
} catch (Exception e) {
12711272
// Log other exceptions at ERROR level
12721273
LOGGER.error("Agent execution threw unexpected Exception for task {}", taskId, e);
1273-
reportAgentExecutionFailure(emitter, "Agent execution failed: " + e.getMessage());
1274+
enqueueErrorPreservingInterrupt(emitter,
1275+
new InternalError("Agent execution failed: " + e.getMessage()));
12741276
}
12751277
LOGGER.debug("Agent execution completed for task {}", taskId);
12761278
// The consumer (running on the Vert.x worker thread) handles queue lifecycle.
@@ -1310,10 +1312,10 @@ public void run() {
13101312
return runnable;
13111313
}
13121314

1313-
private void reportAgentExecutionFailure(AgentEmitter emitter, String message) {
1315+
private void enqueueErrorPreservingInterrupt(AgentEmitter emitter, A2AError error) {
13141316
boolean wasInterrupted = Thread.interrupted();
13151317
try {
1316-
emitter.fail(new InternalError(message));
1318+
emitter.fail(error);
13171319
} finally {
13181320
if (wasInterrupted) {
13191321
Thread.currentThread().interrupt();

‎server-common/src/test/java/org/a2aproject/sdk/server/requesthandlers/DefaultRequestHandlerTest.java‎

Lines changed: 62 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1697,6 +1697,35 @@ public void cancel(RequestContext context, AgentEmitter emitter) {
16971697

16981698
@Test
16991699
void interruptedTerminalEnqueueStillReportsAgentFailure() throws Exception {
1700+
assertInterruptedTerminalEnqueueReportsError(
1701+
(context, emitter) -> emitter.complete(), InternalError.class, null, true, false);
1702+
}
1703+
1704+
@Test
1705+
void interruptedProtocolErrorEnqueueStillReportsAgentFailure() throws Exception {
1706+
CountDownLatch agentReadyToBeInterrupted = new CountDownLatch(1);
1707+
CountDownLatch agentWait = new CountDownLatch(1);
1708+
try {
1709+
assertInterruptedTerminalEnqueueReportsError((context, emitter) -> {
1710+
agentReadyToBeInterrupted.countDown();
1711+
try {
1712+
agentWait.await();
1713+
} catch (InterruptedException e) {
1714+
Thread.currentThread().interrupt();
1715+
throw new UnsupportedOperationError();
1716+
}
1717+
}, UnsupportedOperationError.class, agentReadyToBeInterrupted, false, true);
1718+
} finally {
1719+
agentWait.countDown();
1720+
}
1721+
}
1722+
1723+
private void assertInterruptedTerminalEnqueueReportsError(
1724+
AgentExecutorMethod agentAction,
1725+
Class<? extends A2AError> expectedErrorClass,
1726+
CountDownLatch agentReadyToBeInterrupted,
1727+
boolean interruptBlockedTerminalEnqueue,
1728+
boolean returnImmediately) throws Exception {
17001729
EventQueueUtil.stop(mainEventBusProcessor);
17011730
queueManager = new InMemoryQueueManager((TaskStateProvider) taskStore, mainEventBus) {
17021731
@Override
@@ -1710,7 +1739,8 @@ public EventQueue.EventQueueBuilder createBaseEventQueueBuilder(String taskId) {
17101739

17111740
CountDownLatch workingEventProcessing = new CountDownLatch(1);
17121741
CountDownLatch releaseWorkingEvent = new CountDownLatch(1);
1713-
CountDownLatch fallbackErrorProcessed = new CountDownLatch(1);
1742+
CountDownLatch reportedErrorProcessed = new CountDownLatch(1);
1743+
AtomicReference<A2AError> reportedError = new AtomicReference<>();
17141744
mainEventBusProcessor.setCallback(new MainEventBusProcessorCallback() {
17151745
@Override
17161746
public void onEventProcessed(String taskId, Event event) {
@@ -1722,8 +1752,9 @@ public void onEventProcessed(String taskId, Event event) {
17221752
} catch (InterruptedException e) {
17231753
Thread.currentThread().interrupt();
17241754
}
1725-
} else if (event instanceof InternalError) {
1726-
fallbackErrorProcessed.countDown();
1755+
} else if (event instanceof A2AError error && expectedErrorClass.isInstance(error)) {
1756+
reportedError.set(error);
1757+
reportedErrorProcessed.countDown();
17271758
}
17281759
}
17291760

@@ -1760,7 +1791,7 @@ public void onTaskFinalized(String taskId) {
17601791
agentThread.set(Thread.currentThread());
17611792
executorRan.countDown();
17621793
emitter.startWork();
1763-
emitter.complete();
1794+
agentAction.invoke(context, emitter);
17641795
};
17651796

17661797
MessageSendParams params = MessageSendParams.builder()
@@ -1770,7 +1801,7 @@ public void onTaskFinalized(String taskId) {
17701801
.parts(new TextPart("hello"))
17711802
.build())
17721803
.configuration(MessageSendConfiguration.builder()
1773-
.returnImmediately(false)
1804+
.returnImmediately(returnImmediately)
17741805
.acceptedOutputModes(List.of())
17751806
.build())
17761807
.build();
@@ -1783,23 +1814,41 @@ public void onTaskFinalized(String taskId) {
17831814
assertTrue(executorRan.await(5, TimeUnit.SECONDS), "Executor should have run");
17841815
Thread worker = agentThread.get();
17851816
assertNotNull(worker);
1786-
assertTrue(awaitSemaphoreAcquire(worker, false),
1787-
"Terminal enqueue should wait while the queue is full");
1817+
if (interruptBlockedTerminalEnqueue) {
1818+
assertTrue(awaitSemaphoreAcquire(worker, false),
1819+
"Terminal enqueue should wait while the queue is full");
1820+
} else {
1821+
assertNotNull(agentReadyToBeInterrupted);
1822+
assertTrue(agentReadyToBeInterrupted.await(5, TimeUnit.SECONDS),
1823+
"Agent should be ready to throw the protocol error");
1824+
}
17881825

17891826
worker.interrupt();
17901827

17911828
assertTrue(awaitSemaphoreAcquire(worker, false),
1792-
"Fallback enqueue should wait with the interrupt temporarily cleared");
1829+
"Error enqueue should wait with the interrupt temporarily cleared");
17931830
releaseWorkingEvent.countDown();
1794-
assertTrue(agentFinished.await(5, TimeUnit.SECONDS), "Agent failure reporting should finish");
1831+
assertTrue(agentFinished.await(5, TimeUnit.SECONDS), "Agent error reporting should finish");
17951832
return interruptStatusAfterAgent.get();
17961833
});
17971834

1798-
assertThrows(InternalError.class,
1799-
() -> requestHandler.onMessageSend(params, NULL_CONTEXT));
1835+
String taskId = "";
1836+
if (returnImmediately) {
1837+
Task task = assertInstanceOf(Task.class, requestHandler.onMessageSend(params, NULL_CONTEXT));
1838+
taskId = task.id();
1839+
} else {
1840+
assertThrows(expectedErrorClass,
1841+
() -> requestHandler.onMessageSend(params, NULL_CONTEXT));
1842+
}
18001843

1801-
assertTrue(fallbackErrorProcessed.await(5, TimeUnit.SECONDS),
1802-
"The fallback InternalError should be processed by the event bus");
1844+
assertTrue(reportedErrorProcessed.await(5, TimeUnit.SECONDS),
1845+
"The agent error should be processed by the event bus");
1846+
assertInstanceOf(expectedErrorClass, reportedError.get());
1847+
if (returnImmediately) {
1848+
Task storedTask = taskStore.get(taskId);
1849+
assertNotNull(storedTask);
1850+
assertEquals(TaskState.TASK_STATE_FAILED, storedTask.status().state());
1851+
}
18031852
assertTrue(interruptWasRestored.get(5, TimeUnit.SECONDS),
18041853
"The agent worker's interrupt status should be restored after reporting the error");
18051854
} finally {

0 commit comments

Comments
 (0)