Skip to content

Commit a645ef2

Browse files
bobbai00claude
andcommitted
fix(agent-service): unify operator-failure semantics across consumers
getOperatorErrorText pins the one derivation of "operator failed" the new errorMessages type forces (a fatal error carrying message text), replacing the divergent .length and joined-truthiness checks. In the failure branch, surface per-operator errors for any terminal state (success=false can come with state "Completed"), label CompilationFailed results distinctly, and preserve the Killed state on timeout instead of collapsing to Failed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 83b49d4 commit a645ef2

5 files changed

Lines changed: 123 additions & 15 deletions

File tree

agent-service/src/agent/tools/result-formatting.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,15 @@
1818
*/
1919

2020
import { OperatorResultMode, type OperatorExecutionSummary, type SampleRow } from "../../types/execution";
21-
import { formatExecuteOperatorResult, getOperatorWarnings, getVisibleResultHeaders } from "./tools-utility";
21+
import {
22+
formatExecuteOperatorResult,
23+
getOperatorErrorText,
24+
getOperatorWarnings,
25+
getVisibleResultHeaders,
26+
} from "./tools-utility";
2227

2328
export function formatOperatorResult(operatorId: string, opInfo: OperatorExecutionSummary): string {
24-
const errorText = opInfo.errorMessages.map(e => e.message).join("; ");
29+
const errorText = getOperatorErrorText(opInfo);
2530
if (errorText) {
2631
return `[ERROR] ${errorText}`;
2732
}

agent-service/src/agent/tools/tools-utility.spec.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,43 @@ import {
2525
formatModifyOperatorResult,
2626
formatExecuteOperatorResult,
2727
formatOperatorError,
28+
getOperatorErrorText,
2829
getVisibleResultHeaders,
2930
} from "./tools-utility";
31+
import { OperatorState, WorkflowFatalErrorType, type WorkflowFatalError } from "../../types/execution";
32+
33+
function makeFatal(message: string): WorkflowFatalError {
34+
return {
35+
type: { name: WorkflowFatalErrorType.EXECUTION_FAILURE },
36+
timestamp: { seconds: 0, nanos: 0 },
37+
message,
38+
details: "",
39+
operatorId: "",
40+
workerId: "",
41+
};
42+
}
43+
44+
describe("getOperatorErrorText", () => {
45+
test("joins the messages of every fatal error", () => {
46+
const opInfo = { state: OperatorState.FAILED, errorMessages: [makeFatal("e1"), makeFatal("e2")] };
47+
expect(getOperatorErrorText(opInfo)).toBe("e1; e2");
48+
});
49+
50+
test("ignores errors whose message is empty", () => {
51+
const opInfo = { state: OperatorState.FAILED, errorMessages: [makeFatal(""), makeFatal("real")] };
52+
expect(getOperatorErrorText(opInfo)).toBe("real");
53+
});
54+
55+
test("returns an empty string when every error message is empty", () => {
56+
const opInfo = { state: OperatorState.COMPLETED, errorMessages: [makeFatal("")] };
57+
expect(getOperatorErrorText(opInfo)).toBe("");
58+
});
59+
60+
test("returns an empty string when there are no errors", () => {
61+
const opInfo = { state: OperatorState.COMPLETED, errorMessages: [] };
62+
expect(getOperatorErrorText(opInfo)).toBe("");
63+
});
64+
});
3065

3166
describe("getVisibleResultHeaders", () => {
3267
test("returns every key", () => {

agent-service/src/agent/tools/tools-utility.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,16 @@
1919

2020
import type { OperatorExecutionSummary } from "../../types/execution";
2121

22+
// The single definition of "this operator failed": some fatal error carries
23+
// message text. The engine can emit console ERRORs with empty text, which do
24+
// not count, matching the previous `error` field's truthiness semantics.
25+
export function getOperatorErrorText(opInfo: OperatorExecutionSummary): string {
26+
return opInfo.errorMessages
27+
.map(e => e.message)
28+
.filter(Boolean)
29+
.join("; ");
30+
}
31+
2232
export function getVisibleResultHeaders(row: Record<string, any>): string[] {
2333
return Object.keys(row);
2434
}

agent-service/src/agent/tools/workflow-execution-tools.spec.ts

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -320,6 +320,39 @@ describe("executeOperatorAndFormat - execution failures", () => {
320320
expect(info.errorMessages[0].message).toContain("kaboom");
321321
});
322322

323+
test("extracts per-operator errors even when the aggregate state stays Completed", async () => {
324+
const { state, target } = makeLinearState();
325+
// A console ERROR marks the run unsuccessful without flipping the state.
326+
const summary: WorkflowExecutionSummary = {
327+
success: false,
328+
state: WorkflowExecutionState.COMPLETED,
329+
operators: {
330+
[target]: { state: OperatorState.COMPLETED, errorMessages: [makeFatal("console kaboom")] },
331+
},
332+
errors: [],
333+
};
334+
setFetchResolving(jsonResponse(summary));
335+
336+
const out = await executeOperatorAndFormat(state, makeConfig(), target);
337+
expect(out).toContain("Execution error:");
338+
expect(out).toContain(`${target}: console kaboom`);
339+
});
340+
341+
test("labels compilation failures distinctly from runtime errors", async () => {
342+
const { state, target } = makeLinearState();
343+
const summary: WorkflowExecutionSummary = {
344+
success: false,
345+
state: WorkflowExecutionState.COMPILATION_FAILED,
346+
operators: {},
347+
errors: ["schema mismatch on port 0"],
348+
};
349+
setFetchResolving(jsonResponse(summary));
350+
351+
const out = await executeOperatorAndFormat(state, makeConfig(), target);
352+
expect(out).toContain("Compilation error:");
353+
expect(out).toContain("schema mismatch on port 0");
354+
});
355+
323356
test("reports a timeout when the workflow was KILLED (no onResult callback)", async () => {
324357
const { state, target } = makeLinearState();
325358
const summary: WorkflowExecutionSummary = {
@@ -335,6 +368,25 @@ describe("executeOperatorAndFormat - execution failures", () => {
335368
expect(out).toContain("Workflow execution was killed (timeout).");
336369
});
337370

371+
test("stores the Killed state in the synthetic summary when the workflow was KILLED", async () => {
372+
const { state, target } = makeLinearState();
373+
const summary: WorkflowExecutionSummary = {
374+
success: false,
375+
state: WorkflowExecutionState.KILLED,
376+
operators: {},
377+
errors: [],
378+
};
379+
setFetchResolving(jsonResponse(summary));
380+
381+
const onResult = mock(() => {});
382+
await executeOperatorAndFormat(state, makeConfig(), target, { onResult });
383+
384+
expect(onResult).toHaveBeenCalledTimes(1);
385+
const [, info] = onResult.mock.calls[0] as unknown as [string, OperatorExecutionSummary];
386+
expect(info.state).toBe(OperatorState.KILLED);
387+
expect(info.errorMessages[0].message).toContain("killed (timeout)");
388+
});
389+
338390
test("surfaces general errors when the HTTP request itself fails (ERROR state)", async () => {
339391
const { state, target } = makeLinearState();
340392
setFetchResolving(new Response("backend detail", { status: 500, statusText: "Server Error" }));

agent-service/src/agent/tools/workflow-execution-tools.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { tool } from "ai";
2222
import {
2323
createErrorResult,
2424
formatExecuteOperatorResult,
25+
getOperatorErrorText,
2526
getOperatorWarnings,
2627
getVisibleResultHeaders,
2728
} from "./tools-utility";
@@ -342,7 +343,8 @@ async function executeWorkflowHttp(
342343

343344
function formatExecutionError(
344345
operatorErrors?: Array<{ operatorId: string; error: string }>,
345-
generalErrors?: string[]
346+
generalErrors?: string[],
347+
generalErrorsLabel: string = "Error:"
346348
): string {
347349
const lines: string[] = ["Execution failed due to the following error:"];
348350

@@ -354,7 +356,7 @@ function formatExecutionError(
354356
}
355357

356358
if (generalErrors && generalErrors.length > 0) {
357-
lines.push("Error:");
359+
lines.push(generalErrorsLabel);
358360
for (const error of generalErrors) {
359361
lines.push(` ${error}`);
360362
}
@@ -449,21 +451,25 @@ export async function executeOperatorAndFormat(
449451
});
450452

451453
if (!result.success) {
452-
const operatorErrors =
453-
result.state === WorkflowExecutionState.FAILED
454-
? Object.entries(result.operators)
455-
.filter(([_, op]) => op.errorMessages.length)
456-
.map(([opId, op]) => ({ operatorId: opId, error: op.errorMessages.map(e => e.message).join("; ") }))
457-
: undefined;
454+
// Per-operator errors can accompany any terminal state (e.g. a console
455+
// ERROR leaves state "Completed" with success=false), so never gate on it.
456+
const operatorErrors = Object.entries(result.operators)
457+
.map(([opId, op]) => ({ operatorId: opId, error: getOperatorErrorText(op) }))
458+
.filter(({ error }) => error);
458459

459460
const generalErrors =
460461
result.state === WorkflowExecutionState.KILLED ? ["Workflow execution was killed (timeout)."] : result.errors;
461462

462-
const errorText = formatExecutionError(operatorErrors, generalErrors);
463+
const generalErrorsLabel =
464+
result.state === WorkflowExecutionState.COMPILATION_FAILED ? "Compilation error:" : "Error:";
465+
466+
const errorText = formatExecutionError(operatorErrors, generalErrors, generalErrorsLabel);
463467

464468
if (options.onResult) {
465469
const errorInfo: OperatorExecutionSummary = {
466-
state: OperatorState.FAILED,
470+
// Preserve the killed state so a timeout is distinguishable from a
471+
// genuine operator failure downstream.
472+
state: result.state === WorkflowExecutionState.KILLED ? OperatorState.KILLED : OperatorState.FAILED,
467473
errorMessages: [makeExecutionFailure(errorText, operatorId)],
468474
};
469475
options.onResult(operatorId, errorInfo);
@@ -477,11 +483,11 @@ export async function executeOperatorAndFormat(
477483
return createErrorResult(formatExecutionError(undefined, [`No result found for operator: ${operatorId}`]));
478484
}
479485

480-
if (opInfo.errorMessages.length) {
486+
const opError = getOperatorErrorText(opInfo);
487+
if (opError) {
481488
if (options.onResult) {
482489
options.onResult(operatorId, opInfo);
483490
}
484-
const opError = opInfo.errorMessages.map(e => e.message).join("; ");
485491
return createErrorResult(formatExecutionError([{ operatorId, error: opError }]));
486492
}
487493

@@ -496,7 +502,7 @@ export async function executeOperatorAndFormat(
496502
// Notify for every operator in the execution so upstream stats are also stored.
497503
if (options.onResult) {
498504
for (const [opId, info] of Object.entries(result.operators)) {
499-
if (info && !info.errorMessages.length) {
505+
if (info && !getOperatorErrorText(info)) {
500506
options.onResult(opId, info);
501507
}
502508
}

0 commit comments

Comments
 (0)