Skip to content

Commit c5d8d4a

Browse files
0xMinkedelauna
authored andcommitted
fix(Task): match the abort error message in presentAssistantMessageSafe
The helper's catch handler was distinguishing abort from real failures via `this.abort` state at catch time, which has a real TOCTOU window: a non-abort throw followed by an abort flip between throw and catch microtask would silently swallow the real error. Switched to matching `error.message.endsWith("aborted")` — the literal contract presentAssistantMessage itself throws on abort. Same suppression for the abort case, no swallow window for real errors. Added a test that pins the message-based discriminator: a non-abort error with `this.abort = true` now correctly logs (would have been swallowed under the state-based check). Also added a regression test for the abort-message-match path so a future refactor can't drift back to the state check.
1 parent 1691955 commit c5d8d4a

2 files changed

Lines changed: 69 additions & 1 deletion

File tree

src/core/task/Task.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -365,7 +365,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
365365
*/
366366
private presentAssistantMessageSafe(): void {
367367
void presentAssistantMessage(this).catch((error) => {
368-
if (this.abort) {
368+
// Discriminate on the error message rather than `this.abort` state,
369+
// which can flip between the throw and the catch microtask running:
370+
// a real failure followed by an abort flip would otherwise be
371+
// silently swallowed, and a stale abort error logged as a failure.
372+
// The abort throw site in presentAssistantMessage emits a message
373+
// ending in "aborted" (matching the other abort-throw contracts in
374+
// this file), so we suppress exactly that.
375+
if (error instanceof Error && error.message.endsWith("aborted")) {
369376
return
370377
}
371378
console.error(`[Task#presentAssistantMessage] task ${this.taskId}.${this.instanceId} failed:`, error)

src/core/task/__tests__/Task.spec.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2513,6 +2513,67 @@ describe("Cline", () => {
25132513
boom,
25142514
)
25152515
})
2516+
2517+
it("logs a non-abort error even when this.abort flips true after the throw", async () => {
2518+
// Pins that the message-based discriminator is load-bearing, not the
2519+
// state check. Under the previous `if (this.abort) return` guard this
2520+
// case (a genuine downstream failure racing with an abort flip between
2521+
// the throw and the catch microtask) would silently swallow the error.
2522+
const assistantMessageModule = await import("../../assistant-message")
2523+
const realError = new Error("genuine downstream failure")
2524+
const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(realError)
2525+
2526+
const task = new Task({
2527+
provider: mockProvider,
2528+
apiConfiguration: mockApiConfig,
2529+
task: "test task",
2530+
startTask: false,
2531+
})
2532+
2533+
await flushMicrotasks()
2534+
consoleErrorSpy.mockClear()
2535+
2536+
// Simulate the TOCTOU race: abort flips between throw and catch.
2537+
task.abort = true
2538+
;(task as any).presentAssistantMessageSafe()
2539+
await flushMicrotasks()
2540+
2541+
expect(presentSpy).toHaveBeenCalledTimes(1)
2542+
expect(consoleErrorSpy).toHaveBeenCalledWith(
2543+
expect.stringContaining("[Task#presentAssistantMessage] task"),
2544+
realError,
2545+
)
2546+
})
2547+
2548+
it("suppresses an abort-pattern error by message match even when this.abort is false", async () => {
2549+
// Pins the inverse: message wins over state. A stale abort rejection
2550+
// arriving before `this.abort` has been observed as true must still be
2551+
// suppressed, so the catch handler never logs the expected
2552+
// cancellation rejection as a real failure.
2553+
const assistantMessageModule = await import("../../assistant-message")
2554+
const abortError = new Error("[Task#presentAssistantMessage] task t.i aborted")
2555+
const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(abortError)
2556+
2557+
const task = new Task({
2558+
provider: mockProvider,
2559+
apiConfiguration: mockApiConfig,
2560+
task: "test task",
2561+
startTask: false,
2562+
})
2563+
2564+
await flushMicrotasks()
2565+
consoleErrorSpy.mockClear()
2566+
2567+
expect(task.abort).toBeFalsy()
2568+
;(task as any).presentAssistantMessageSafe()
2569+
await flushMicrotasks()
2570+
2571+
expect(presentSpy).toHaveBeenCalledTimes(1)
2572+
const presentErrors = consoleErrorSpy.mock.calls.filter(
2573+
(call) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"),
2574+
)
2575+
expect(presentErrors).toHaveLength(0)
2576+
})
25162577
})
25172578
})
25182579

0 commit comments

Comments
 (0)