unbridled-41 opened a new pull request, #5118:
URL: https://github.com/apache/rocketmq-dashboard/pull/5118
### Issue linkage
No existing issue or open PR covers this path. Checked: `gh search
prs/issues --repo apache/rocketmq-dashboard` for `AiRunExecutor`,
`decide(RunContext`, `error terminal state` — the only open artifacts touching
this class are #4999 (terminal-frame duration, mine) and #4740, and neither
mentions the failure path below; the open issue #5003 is about the replayed
`run_finished` frame, not about the terminal state.
### Problem / Evidence
`AiRunExecutor.execute`
(`server/src/main/java/org/apache/rocketmq/studio/ops/ai/conversation/AiRunExecutor.java:274-294`)
catches `LlmGatewayException` and `RuntimeException` and then decides the
terminal state in `finally`. An `Error` matches neither catch, so `decide()`
(`:481-495`) sees an empty `Outcome` and falls through to its last branch:
```java
return new Terminal(RunStatus.COMPLETED, null, null, null, outcome, false);
```
Call path: `AiRunService.start` -> `AiRunExecutor.submit` -> worker ->
`execute(RunContext)` -> `streamWithLostResumeRetry` -> `finally {
finalizeRun(context, decide(context, outcome)) }` -> run row written in
`finalizeRun` (`:515-526`).
Observed **before** the fix — the new regression test, run against the
pre-fix revision of `AiRunExecutor` (`git checkout origin/rocketmq-studio --
.../AiRunExecutor.java`):
```text
INFO AiRunExecutor -- agent run 11 finished: status=COMPLETED reason=null
durationMs=13 ... endSeq=3
expected: "FAILED"
but was: "COMPLETED"
Tests run: 1, Failures: 1, Errors: 0
```
So a `StackOverflowError` while streaming — a deeply nested tool payload
through Jackson is the realistic producer — was written to the run row as a
completed turn with no answer, the transcript replays it as finished, and
`AgentRunHandle.stop` has nothing left to correct.
Observed **after** the fix: the same test reports `status=FAILED
reason=PROVIDER_ERROR`, error code `ai.run.internal_error`, `finishedAt` set,
emitter completed.
### Root cause / Fix
`Outcome.unexpected` only accepted a `RuntimeException`, and nothing caught
an `Error`. Catch `Error` too, record it in the same field (widened to
`Throwable`), let the existing `finally` write the terminal row, and rethrow so
the failure still reaches the worker's uncaught handler.
### Priority / fix confidence
- PRIORITY = **62** — impact 24/40 (a crashed run is presented as a
completed answer), scope 14/20 (every hosted agent run), reproducibility 16/20
(deterministic unit test), maintenance value 8/20 (restores the invariant the
sibling `RuntimeException` branch already implements).
- FIX_CONFIDENCE = **88** — the change mirrors the catch/record/decide path
that already exists for `RuntimeException`, and the reasoning is pinned by
`anUnexpectedProviderFailureShouldNotLeaveTheRunNonTerminalTest`.
### Tests
| Command | Result |
|---|---|
| `mvn -o -q test -Dtest=AiRunExecutorTest` | 19 tests, 0 failures |
| `mvn -o -q test -Dtest='org.apache.rocketmq.studio.ops.ai.**'` | 961
tests, 0 failures, 0 errors |
| `mvn -o test` (whole suite) | Tests run: 3288, Failures: 0, Errors: 17 |
| `mvn -o checkstyle:check` | clean |
The 17 whole-suite errors are the MySQL-backed `*IntegrationTest` /
`StudioApplicationTest` Spring context-load failures this environment always
produces; the errored set is byte-identical to the set measured on the
untouched base revision `4ee173ad`. One whole-suite run additionally hit
`ResourceOwnershipGuardTest#migrationConcurrentStartupIsIdempotentTest`
(`java.util.ConcurrentModificationException` inside H2's
`JdbcDatabaseMetaData.getIndexInfo`); that test fails 3/3 standalone on the
untouched base revision here as well, so it is an environment race, not this
change.
New test:
`AiRunExecutorTest#anErrorFromTheWorkerShouldNotBeRecordedAsACompletedRunTest`.
`AiRunTestSupport.StubAgentProvider` gained an `Error error` field next to its
`RuntimeException failure` so a test can reach paths only an `Error` takes.
### Risk
A worker that dies with an `Error` now finalises the row as FAILED before
the error propagates; nothing else reads `Outcome.unexpected`, and the
`finally` block already ran for every other exception type.
--
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]
