fix(api-server): return the events a failed invocation produced from /run

[kalenkevich]

/run_sse writes each event to the wire as it arrives, so a caller sees the structured node-error event — carrying nodeInfo.path, errorType, errorCode and attemptCount — when a node throws. /run returned HTTP 500 and a one-line string for the same invocation, and the caller had no way to tell which node failed:

/run      HTTP 500  {"error":"Failed to run agent: TypeError: e.toUpperCase is not a function"}
/run_sse  data: {"author":"my_function_node","nodeInfo":{"path":"function_node_pipeline.my_function_node"},
                 "isNodeError":true,"errorType":"TypeError","errorMessage":"...","attemptCount":1}

Root cause

const events: Event[] = [] was declared inside the try (dev/src/server/adk_api_server.ts:909), so it was not even in scope in the catch at :923. Hoisting it is the fix.

The events are always there to report, and that is guaranteed rather than incidental: Workflow.orchestrate emits the node-error event via ctx.emit (workflow.ts:338) and only then rethrows (:341), and runNodeAsInvocation drains the channel to completion in its for await before await settle surfaces the rejection. So the consumer always receives the error event first.

Why 500 and not 200

/run_sse ends with 200 because it has already flushed headers, so status parity with it is not really available. Keeping the 500:

• adk-python also fails the request (worker() collects the events, any non-SessionNotFoundError exception propagates to FastAPI's handler and the events are lost), so 200 would be a deliberate divergence.
• The Dev UI never calls /run — only /run_sse (dev/src/server/adk_api_client.ts:140, and confirmed in the prebuilt bundle). /run's clients are scripts and the adk api_server API, for which a failed invocation silently returning 200 is worse than a 500 that carries the payload.
• It does not break anyone branching on the status code.

The 200 success shape is unchanged (a bare Event[]); only the 500 body gains an events key, so a client already parsing {error} still works. /api/reasoning_engine (:956-975) had the identical bug and gets the same treatment.

Tests

dev/test/server/adk_api_server_test.ts — a workflow whose first node succeeds and whose second throws; asserts 500, the first node's event, and nodeInfo.path === 'wf.second' on the node-error event.

The HttpClient test helper discarded the response body on non-2xx, so no test could assert on an error body at all; it now carries data/text through. The three existing 500 assertions only read status and are unaffected (they also fail before any event is produced, so they stay green either way).

Full unit:dev green (300), tsc --noEmit clean.

Fixes #743

[AmaadMartin]

Approve. The fix is correct: events is hoisted out of the try in both /run (adk_api_server.ts:910) and /api/reasoning_engine (:958), so the catch reports the events the failed invocation produced. I verified this against source at head 1d83811. The sibling bug is fixed too, and the test restores getAgentFile in finally. No type suppressions and no new public types. CI is green on ubuntu, macOS, and Windows.

[AmaadMartin]

Approve. The fix is correct and minimal: it hoists events out of the try so the catch can report what the invocation produced, and it keeps the 500 status and the unchanged success shape. I verified the change against the head blobs, confirmed the two existing 500 tests read only .status, and confirmed nodeInfo is a real Event field. Two optional nits below; neither blocks. All CI checks passed at review time.

[AmaadMartin]

Nit, optional. Use the exported isNodeErrorEvent guard instead of an inline cast.

const nodeError = body?.events.find(
  (e) => (e as Event & {isNodeError?: boolean}).isNodeError,
);

The repo ships this guard (core/src/common.ts, exported from @google/adk). It drops the cast and narrows nodeError to NodeErrorEvent:

const nodeError = body?.events.find(isNodeErrorEvent);

[AmaadMartin]

Nit, optional. Trim this comment. The useful why is the first idea: the workflow emits the node-error event, then rethrows, so events holds it here. The rest restates the bug history, which belongs in the commit message.

// The workflow emits the node-error event, then rethrows, so `events`
// already holds it here; /run must report them like /run_sse does.

[AmaadMartin]

Approve. Re-reviewed at dfbe14b. The fix is correct: it hoists events out of the try in both /run (adk_api_server.ts:908) and /api/reasoning_engine (:949), so the 500 body reports the events. The success shapes are unchanged. My earlier comment nit is resolved. One optional nit stays open: the test still uses an inline cast, not the exported isNodeErrorEvent guard (adk_api_server_test.ts:695). CI was green at review time.