wenjin272 commented on code in PR #955:
URL: https://github.com/apache/flink-agents/pull/955#discussion_r4013495115


##########
plan/src/main/java/org/apache/flink/agents/plan/actions/ToolCallAction.java:
##########
@@ -232,84 +241,90 @@ private static void executeSequentially(
             Map<String, String> error,
             Map<String, ToolResponse> responses) {
         for (ToolCallExecution execution : executions) {
+            Outcome<ToolResponse> outcome = null;
+            Instant resultObservedAt = null;
             try {
                 ToolResponse response =
                         toolCallAsync
                                 ? ctx.durableExecuteAsync(execution.callable)
                                 : ctx.durableExecute(execution.callable);
+                resultObservedAt = Instant.now();
+                outcome = Outcome.success(response);
                 recordToolResponse(execution.id, response, success, error, 
responses);
-                if (response.isError()) {
-                    ExecutionReporters.failed(
-                            ctx,
-                            ExecutionReporter.EntityTypes.TOOL,
-                            execution.name,
-                            execution.entityMetadata,
-                            new RuntimeException(response.getError()),
-                            
ExecutionReporter.ProblemCategories.TOOL_CALL_FAILED);
-                } else {
-                    ExecutionReporters.succeeded(
-                            ctx,
-                            ExecutionReporter.EntityTypes.TOOL,
-                            execution.name,
-                            execution.entityMetadata);
-                }
             } catch (Exception e) {

Review Comment:
   Could we preserve the previous terminal-reporting behavior when a Java Tool 
throws an `Error`? The timestamp refactor removed the `catch (Error)` branches 
from both the sequential and parallel paths. An `Error` now bypasses `catch 
(Exception)`, and `finally` calls `reportExecution()` with a null `outcome`, 
producing `created`/`started` without a matching `failed` event or incrementing 
`numOfToolCallsFailed`, before the original `Error` continues to propagate. 
Please carry this failure into the final reporting path, emit `failedAt`, and 
then rethrow the original `Error`. A regression test using `AssertionError` or 
`LinkageError` would help preserve this behavior.



-- 
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]

Reply via email to