fix(gateway): log chat-abort terminal persistence failures instead of swallowing

guocn · guocn · commit 962ba790900b · 2026-06-29T23:31:45.000+08:00
The terminal session-store persistence promise in registerChatAbortController's cleanup path caught rejections with an empty handler, silently dropping both the error and any operator signal while still deleting the abort controller. In production, terminal states could fail to persist with zero trace in logs or telemetry, while the swallowed error masked database / file-system health issues. Surfaces the failure via the gateway/chat-abort subsystem logger (error level), including the run id and the underlying error, and preserves the existing controller cleanup so idempotent retry cleanup is not blocked. Fixes #97795
diff --git a/src/gateway/chat-abort.test.ts b/src/gateway/chat-abort.test.ts
@@ -16,6 +16,24 @@ import {
   updateChatRunProvider,
 } from "./chat-abort.js";
 
+// Capture the subsystem logger's error spy so the terminal-persistence failure
+// path can be asserted to emit a log line instead of swallowing it.
+const chatAbortLogError = vi.hoisted(() => vi.fn());
+vi.mock("../logging/subsystem.js", () => ({
+  createSubsystemLogger: vi.fn(() => ({
+    warn: vi.fn(),
+    info: vi.fn(),
+    error: chatAbortLogError,
+    debug: vi.fn(),
+    trace: vi.fn(),
+    fatal: vi.fn(),
+    raw: vi.fn(),
+    child: vi.fn(),
+    isEnabled: vi.fn(() => false),
+    subsystem: "test",
+  })),
+}));
+
 type ChatAbortPayload = {
   runId: string;
   sessionKey: string;
@@ -265,6 +283,46 @@ describe("registerChatAbortController", () => {
     expect(chatAbortControllers.has("run-persisting")).toBe(false);
   });
 
+  it("logs an error and still clears the entry when terminal persistence fails", async () => {
+    chatAbortLogError.mockClear();
+    const chatAbortControllers = new Map<string, ChatAbortControllerEntry>();
+    const registration = registerChatAbortController({
+      chatAbortControllers,
+      runId: "run-persist-failed",
+      sessionId: "sess-1",
+      sessionKey: "main",
+      timeoutMs: 60_000,
+    });
+    let rejectPersistence: (err: Error) => void = () => undefined;
+    const persistence = new Promise<void>((_resolve, reject) => {
+      rejectPersistence = reject;
+    });
+    if (!registration.entry) {
+      throw new Error("expected registered entry");
+    }
+    registration.entry.projectSessionActive = false;
+    registration.entry.projectSessionTerminalPersistence = persistence;
+
+    registration.cleanup();
+
+    // Entry is retained while persistence is still in flight.
+    expect(chatAbortControllers.has("run-persist-failed")).toBe(true);
+    expect(chatAbortLogError).not.toHaveBeenCalled();
+
+    const failure = new Error("disk full");
+    rejectPersistence(failure);
+    // Allow the rejection to propagate to the .catch() handler.
+    await expect(persistence).rejects.toBe(failure);
+    await Promise.resolve();
+
+    // Entry must still be cleaned up (no leak) AND the failure must be logged
+    // rather than silently swallowed — the regression this guards against.
+    expect(chatAbortControllers.has("run-persist-failed")).toBe(false);
+    expect(chatAbortLogError).toHaveBeenCalledTimes(1);
+    expect(chatAbortLogError.mock.calls[0][0]).toContain("run-persist-failed");
+    expect(chatAbortLogError.mock.calls[0][0]).toContain("disk full");
+  });
+
   it("retains registrations when terminal lifecycle was observed before caller cleanup", () => {
     const chatAbortControllers = new Map<string, ChatAbortControllerEntry>();
     const registration = registerChatAbortController({
diff --git a/src/gateway/chat-abort.ts b/src/gateway/chat-abort.ts
@@ -11,9 +11,12 @@ import { isAbortRequestText } from "../auto-reply/reply/abort-primitives.js";
 import type { OpenClawConfig } from "../config/types.openclaw.js";
 import { emitAgentEvent, getAgentEventLifecycleGeneration } from "../infra/agent-events.js";
 import { jsonUtf8Bytes } from "../infra/json-utf8-bytes.js";
+import { createSubsystemLogger } from "../logging/subsystem.js";
 import { projectLiveAssistantBufferedText } from "./live-chat-projector.js";
 import { createChatAbortMarker, type ChatAbortMarker } from "./server-chat-state.js";
 
+const log = createSubsystemLogger("gateway/chat-abort");
+
 const DEFAULT_CHAT_RUN_ABORT_GRACE_MS = 60_000;
 
 export type ChatAbortControllerEntry = {
@@ -173,7 +176,15 @@ export function registerChatAbortController(params: {
               params.chatAbortControllers.delete(params.runId);
             }
           })
-          .catch(() => {
+          .catch((err) => {
+            // The terminal session-store update is the persistence contract for a
+            // run's final state. If it fails we still must drop the abort controller
+            // (otherwise it leaks and blocks idempotent retry cleanup), but the
+            // failure must be visible to operators — otherwise terminal states can
+            // fail to persist in production with no trace in logs or telemetry.
+            log.error(
+              `chat-abort: failed to persist terminal session state for run ${params.runId}: ${err}`,
+            );
             if (params.chatAbortControllers.get(params.runId)?.controller === controller) {
               params.chatAbortControllers.delete(params.runId);
             }
