Skip to content

Commit 1f92d82

Browse files
committed
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 fd5d3c9 commit 1f92d82

2 files changed

Lines changed: 80 additions & 15 deletions

File tree

src/core/task/Task.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,7 +370,14 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
370370
*/
371371
private presentAssistantMessageSafe(): void {
372372
void presentAssistantMessage(this).catch((error) => {
373-
if (this.abort) {
373+
// Discriminate on the error message rather than `this.abort` state,
374+
// which can flip between the throw and the catch microtask running:
375+
// a real failure followed by an abort flip would otherwise be
376+
// silently swallowed, and a stale abort error logged as a failure.
377+
// The abort throw site in presentAssistantMessage emits a message
378+
// ending in "aborted" (matching the other abort-throw contracts in
379+
// this file), so we suppress exactly that.
380+
if (error instanceof Error && error.message.endsWith("aborted")) {
374381
return
375382
}
376383
console.error(`[Task#presentAssistantMessage] task ${this.taskId}.${this.instanceId} failed:`, error)
@@ -544,7 +551,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
544551
.deref()
545552
?.postStateToWebviewWithoutTaskHistory()
546553
.catch((error) => {
547-
console.error("[Task#messageQueueStateChangedHandler] postStateToWebviewWithoutTaskHistory failed:", error)
554+
console.error(
555+
"[Task#messageQueueStateChangedHandler] postStateToWebviewWithoutTaskHistory failed:",
556+
error,
557+
)
548558
})
549559
}
550560

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

Lines changed: 68 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1774,11 +1774,9 @@ describe("Cline", () => {
17741774

17751775
it("logs (instead of crashing) when startTask rejects from the constructor", async () => {
17761776
const boom = new Error("startTask boom")
1777-
const startTaskSpy = vi
1778-
.spyOn(Task.prototype as any, "startTask")
1779-
.mockImplementation(async () => {
1780-
throw boom
1781-
})
1777+
const startTaskSpy = vi.spyOn(Task.prototype as any, "startTask").mockImplementation(async () => {
1778+
throw boom
1779+
})
17821780

17831781
new Task({
17841782
provider: mockProvider,
@@ -1796,11 +1794,9 @@ describe("Cline", () => {
17961794

17971795
it("logs (instead of crashing) when resumeTaskFromHistory rejects from the constructor", async () => {
17981796
const boom = new Error("resume boom")
1799-
const resumeSpy = vi
1800-
.spyOn(Task.prototype as any, "resumeTaskFromHistory")
1801-
.mockImplementation(async () => {
1802-
throw boom
1803-
})
1797+
const resumeSpy = vi.spyOn(Task.prototype as any, "resumeTaskFromHistory").mockImplementation(async () => {
1798+
throw boom
1799+
})
18041800

18051801
new Task({
18061802
provider: mockProvider,
@@ -1900,9 +1896,7 @@ describe("Cline", () => {
19001896
it("logs non-abort rejections from presentAssistantMessageSafe", async () => {
19011897
const assistantMessageModule = await import("../../assistant-message")
19021898
const boom = new Error("present boom")
1903-
const presentSpy = vi
1904-
.spyOn(assistantMessageModule, "presentAssistantMessage")
1905-
.mockRejectedValue(boom)
1899+
const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(boom)
19061900

19071901
const task = new Task({
19081902
provider: mockProvider,
@@ -1921,6 +1915,67 @@ describe("Cline", () => {
19211915
boom,
19221916
)
19231917
})
1918+
1919+
it("logs a non-abort error even when this.abort flips true after the throw", async () => {
1920+
// Pins that the message-based discriminator is load-bearing, not the
1921+
// state check. Under the previous `if (this.abort) return` guard this
1922+
// case (a genuine downstream failure racing with an abort flip between
1923+
// the throw and the catch microtask) would silently swallow the error.
1924+
const assistantMessageModule = await import("../../assistant-message")
1925+
const realError = new Error("genuine downstream failure")
1926+
const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(realError)
1927+
1928+
const task = new Task({
1929+
provider: mockProvider,
1930+
apiConfiguration: mockApiConfig,
1931+
task: "test task",
1932+
startTask: false,
1933+
})
1934+
1935+
await flushMicrotasks()
1936+
consoleErrorSpy.mockClear()
1937+
1938+
// Simulate the TOCTOU race: abort flips between throw and catch.
1939+
task.abort = true
1940+
;(task as any).presentAssistantMessageSafe()
1941+
await flushMicrotasks()
1942+
1943+
expect(presentSpy).toHaveBeenCalledTimes(1)
1944+
expect(consoleErrorSpy).toHaveBeenCalledWith(
1945+
expect.stringContaining("[Task#presentAssistantMessage] task"),
1946+
realError,
1947+
)
1948+
})
1949+
1950+
it("suppresses an abort-pattern error by message match even when this.abort is false", async () => {
1951+
// Pins the inverse: message wins over state. A stale abort rejection
1952+
// arriving before `this.abort` has been observed as true must still be
1953+
// suppressed, so the catch handler never logs the expected
1954+
// cancellation rejection as a real failure.
1955+
const assistantMessageModule = await import("../../assistant-message")
1956+
const abortError = new Error("[Task#presentAssistantMessage] task t.i aborted")
1957+
const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(abortError)
1958+
1959+
const task = new Task({
1960+
provider: mockProvider,
1961+
apiConfiguration: mockApiConfig,
1962+
task: "test task",
1963+
startTask: false,
1964+
})
1965+
1966+
await flushMicrotasks()
1967+
consoleErrorSpy.mockClear()
1968+
1969+
expect(task.abort).toBeFalsy()
1970+
;(task as any).presentAssistantMessageSafe()
1971+
await flushMicrotasks()
1972+
1973+
expect(presentSpy).toHaveBeenCalledTimes(1)
1974+
const presentErrors = consoleErrorSpy.mock.calls.filter(
1975+
(call) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"),
1976+
)
1977+
expect(presentErrors).toHaveLength(0)
1978+
})
19241979
})
19251980
})
19261981

0 commit comments

Comments
 (0)