runFiber: a throw from the finally block discards the fiber's successful result #2104

Summary

Agent._runFiberInternal does its cleanup inside a finally, and that cleanup can throw. When it does, it replaces whatever the fiber body produced — so a fiber that completed successfully is reported to the caller as a failure and its return value is lost.

packages/agents/src/index.ts (current main, ~L5752):

} finally {
  this._runFiberActiveFibers.delete(id);
  this._withAgentSpan(
    "finalize_fiber",
    "fiber",
    { "cloudflare.agents.fiber.id": id, "cloudflare.agents.fiber.name": name },
    () => {
      this.sql`DELETE FROM cf_agents_runs WHERE id = ${id}`;
    }
  );
  dispose();

The DELETE is a storage operation, so it can fail for reasons entirely unrelated to the work the fiber just did. Durable Object storage occasionally returns an opaque internal error; reference = <id> — when that lands on this one statement, the fiber's result is thrown away.

Impact

This is most visible through @cloudflare/think, which wraps every chat turn in a fiber (_runChatRecoveryFiber → _runFiberWithStashWrapper). The failure mode we observed in production:

1. The model call completes and the tool executions complete — both have OK spans.
2. The turn's fiber finishes and the finally runs.
3. The DELETE throws.
4. think catches it, emits chat:turn:finish with status: "error", and rethrows.
5. The application posts a generic error to the user. The generated answer is discarded and the user has to ask again.

Span parentage shows the shape clearly: finalize_fiber is a child of chat_turn, and both are marked ERROR with SqlError, even though everything the turn was asked to do had already succeeded.

Note the same block's dispose() already tolerates failure — the facet keepAlive release is .catch()ed and only logs. The DELETE is the one unguarded step.

Suggested fix

Make finalization non-throwing, so bookkeeping can never replace the body's outcome:

try {
  this._withAgentSpan("finalize_fiber", "fiber", { ... }, () => {
    this.sql`DELETE FROM cf_agents_runs WHERE id = ${id}`;
  });
} catch (e) {
  console.error("[Agent] Failed to finalize fiber; leaving run row for recovery:", e);
}

Leaving the row behind looks like the right trade rather than a leak: _checkRunFibers already reconciles leftover rows on the next load, and for chat fibers _classifyRecoveredThinkTurn treats a stream that already reached a terminal status as neither retry nor continue — so a completed turn is reconciled rather than replayed, and the reply is not duplicated.

Happy to open a PR if the approach looks right.

Versions

agents@0.20.1, @cloudflare/think@0.15.1. Verified the same code is present on main as of 2026-08-12.