wenjin272 commented on code in PR #1111:
URL: https://github.com/apache/flink-agents/pull/1111#discussion_r4002867206
##########
plan/src/main/java/org/apache/flink/agents/plan/actions/ToolCallAction.java:
##########
@@ -206,6 +212,12 @@ private static void executeParallel(
for (int i = 0; i < outcomes.size(); i++) {
recordOutcome(executions.get(i), outcomes.get(i), ctx,
success, error, responses);
}
+ } catch (InterruptedException e) {
Review Comment:
The new catch only handles cases where `durableExecuteAllAsync()` throws
directly, but the production Java path uses `JavaRunnerContextImpl`, whose JDK
11 and JDK 21 batch executors convert a tool's `InterruptedException` into
`Outcome.failure`. `finalizeExecutedOutcomes()` then persists it as a durable
failure, and `recordOutcome()` still turns it into a tool error, so the action
can send a `ToolResponseEvent` and complete after cancellation. Could we
propagate the interruption from the batch execution boundary before finalizing
it, leaving the interrupted/unstarted slots pending while preserving already
completed outcomes? A regression test should exercise `JavaRunnerContextImpl`
rather than mocking `durableExecuteAllAsync()` to throw directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
