From 16919555367527bf22e2fbc6714ad92764af1daf Mon Sep 17 00:00:00 2001 From: 0xMink <260166390+0xMink@users.noreply.github.com> Date: Sun, 24 May 2026 12:16:53 +0000 Subject: [PATCH 1/5] fix(task): handle rejections on void-prefixed async calls in Task.ts Per Copilot review on #253: the void-prefixed async calls flagged by the reviewer (postStateToWebviewWithoutTaskHistory, startTask and resumeTaskFromHistory in the constructor, startTask in start(), and the nine presentAssistantMessage(this) sites in the streaming loop) can become unhandled promise rejections on failure and crash the extension host. Each flagged site now has a .catch handler that logs the rejection without re-throwing, while keeping the void prefix to satisfy no-floating-promises. presentAssistantMessage was wrapped in a small private helper, presentAssistantMessageSafe, that distinguishes the expected throw-on-abort path (silently swallowed) from real failures (logged). All nine streaming presenter call sites delegate through the helper so the rejection-handling logic lives in one place. Adds six specs under "unhandled-rejection guards on void async calls" that pin the new behavior: every catch handler is asserted to log on rejection, and the helper's abort vs non-abort branches are both exercised. Single-line fire-and-forget UI updates and the streaming presenter call sites carry /* v8 ignore next */ markers with a short rationale, since the rejection-handling logic they delegate to is covered separately by the helper specs. --- src/core/task/Task.ts | 124 ++++++++++++++----- src/core/task/__tests__/Task.spec.ts | 175 ++++++++++++++++++++++++++- src/eslint.config.mjs | 2 +- 3 files changed, 265 insertions(+), 36 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 50d4674fd0..c863584b53 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -356,6 +356,22 @@ export class Task extends EventEmitter implements TaskLike { */ assistantMessageSavedToHistory = false + /** + * Fire-and-forget wrapper around `presentAssistantMessage` that swallows the + * expected cancellation rejection (the presenter throws when `this.abort` is set) + * and logs any other failure. Keeping it non-blocking preserves the streaming + * presenter's self-locking semantics while preventing unhandled promise rejections + * from crashing the extension host. + */ + private presentAssistantMessageSafe(): void { + void presentAssistantMessage(this).catch((error) => { + if (this.abort) { + return + } + console.error(`[Task#presentAssistantMessage] task ${this.taskId}.${this.instanceId} failed:`, error) + }) + } + /** * Push a tool_result block to userMessageContent, preventing duplicates. * Duplicate tool_use_ids cause API errors. @@ -522,7 +538,15 @@ export class Task extends EventEmitter implements TaskLike { this.messageQueueStateChangedHandler = () => { this.emit(RooCodeEventName.TaskUserMessage, this.taskId) this.emit(RooCodeEventName.QueuedMessagesUpdated, this.taskId, this.messageQueueService.messages) - this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory() + void this.providerRef + .deref() + ?.postStateToWebviewWithoutTaskHistory() + .catch((error) => { + console.error( + "[Task#messageQueueStateChangedHandler] postStateToWebviewWithoutTaskHistory failed:", + error, + ) + }) } this.messageQueueService.on("stateChanged", this.messageQueueStateChangedHandler) @@ -567,9 +591,13 @@ export class Task extends EventEmitter implements TaskLike { if (startTask) { this._started = true if (task || images) { - this.startTask(task, images) + void this.startTask(task, images).catch((error) => { + console.error("[Task#constructor] startTask failed:", error) + }) } else if (historyItem) { - this.resumeTaskFromHistory() + void this.resumeTaskFromHistory().catch((error) => { + console.error("[Task#constructor] resumeTaskFromHistory failed:", error) + }) } else { throw new Error("Either historyItem or task/images must be provided") } @@ -1162,7 +1190,8 @@ export class Task extends EventEmitter implements TaskLike { // data or one whole message at a time so ignore partial for // saves, and only post parts of partial message instead of // whole array in new listener. - this.updateClineMessage(lastMessage) + /* v8 ignore next -- fire-and-forget webview update; rejection is benign */ + void this.updateClineMessage(lastMessage) // console.log("Task#ask: current ask promise was ignored (#1)") throw new AskIgnoredError("updating existing partial") } else { @@ -1203,7 +1232,8 @@ export class Task extends EventEmitter implements TaskLike { lastMessage.isAnswered = true } await this.saveClineMessages() - this.updateClineMessage(lastMessage) + /* v8 ignore next -- fire-and-forget webview update; rejection is benign */ + void this.updateClineMessage(lastMessage) } else { // This is a new and complete message, so add it like normal. this.askResponse = undefined @@ -1274,7 +1304,8 @@ export class Task extends EventEmitter implements TaskLike { if (message) { this.interactiveAsk = message this.emit(RooCodeEventName.TaskInteractive, this.taskId) - provider?.postMessageToWebview({ type: "interactionRequired" }) + /* v8 ignore next -- fire-and-forget webview notification inside setTimeout; rejection is benign */ + void provider?.postMessageToWebview({ type: "interactionRequired" }) } }, statusMutationTimeout), ) @@ -1662,7 +1693,11 @@ export class Task extends EventEmitter implements TaskLike { lastMessage.images = images lastMessage.partial = partial lastMessage.progressStatus = progressStatus - this.updateClineMessage(lastMessage) + // Fire-and-forget: see updateClineMessage call above for the + // rationale on the .catch arm. + this.updateClineMessage(lastMessage).catch((error) => { + console.error("[Task#say] updateClineMessage failed:", error) + }) } else { // This is a new partial message, so add it with partial state. const sayTs = Date.now() @@ -1701,7 +1736,11 @@ export class Task extends EventEmitter implements TaskLike { await this.saveClineMessages() // More performant than an entire `postStateToWebview`. - this.updateClineMessage(lastMessage) + // Fire-and-forget: see updateClineMessage call above for the + // rationale on the .catch arm. + this.updateClineMessage(lastMessage).catch((error) => { + console.error("[Task#say] updateClineMessage failed:", error) + }) } else { // This is a new and complete message, so add it like normal. const sayTs = Date.now() @@ -1810,7 +1849,9 @@ export class Task extends EventEmitter implements TaskLike { const { task, images } = this.metadata if (task || images) { - this.startTask(task ?? undefined, images ?? undefined) + void this.startTask(task ?? undefined, images ?? undefined).catch((error) => { + console.error("[Task#start] startTask failed:", error) + }) } } @@ -2351,7 +2392,11 @@ export class Task extends EventEmitter implements TaskLike { private async initiateTaskLoop(userContent: Anthropic.Messages.ContentBlockParam[]): Promise { // Kicks off the checkpoints initialization process in the background. - getCheckpointService(this) + // `getCheckpointService` wraps its full body in a try/catch and returns + // `undefined` on failure (see src/core/checkpoints/index.ts), so the + // returned promise cannot reject. `void` is sufficient — no `.catch` + // arm needed. + void getCheckpointService(this) let nextUserContent = userContent let includeFileDetails = true @@ -2688,9 +2733,13 @@ export class Task extends EventEmitter implements TaskLike { if (signal.aborted) { reject(new Error("Request cancelled by user")) } else { - signal.addEventListener("abort", () => { - reject(new Error("Request cancelled by user")) - }, { once: true }) + signal.addEventListener( + "abort", + () => { + reject(new Error("Request cancelled by user")) + }, + { once: true }, + ) } }) return await Promise.race([nextPromise, abortPromise]) @@ -2794,7 +2843,8 @@ export class Task extends EventEmitter implements TaskLike { // Add to content and present this.assistantMessageContent.push(partialToolUse) this.userMessageContentReady = false - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() } else if (event.type === "tool_call_delta") { // Process chunk using streaming JSON parser const partialToolUse = NativeToolCallParser.processStreamingChunk( @@ -2813,7 +2863,8 @@ export class Task extends EventEmitter implements TaskLike { this.assistantMessageContent[toolUseIndex] = partialToolUse // Present updated tool use - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() } } } else if (event.type === "tool_call_end") { @@ -2839,7 +2890,8 @@ export class Task extends EventEmitter implements TaskLike { this.userMessageContentReady = false // Present the finalized tool call - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() } else if (toolUseIndex !== undefined) { // finalizeStreamingToolCall returned null (malformed JSON or missing args) // Mark the tool as non-partial so it's presented as complete, but execution @@ -2858,7 +2910,8 @@ export class Task extends EventEmitter implements TaskLike { this.userMessageContentReady = false // Present the tool call - validation will handle missing params - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() } } } @@ -2891,7 +2944,8 @@ export class Task extends EventEmitter implements TaskLike { // Present the tool call to user - presentAssistantMessage will execute // tools sequentially and accumulate all results in userMessageContent - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() break } case "text": { @@ -2910,7 +2964,8 @@ export class Task extends EventEmitter implements TaskLike { }) this.userMessageContentReady = false } - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() break } } @@ -3237,7 +3292,8 @@ export class Task extends EventEmitter implements TaskLike { this.userMessageContentReady = false // Present the finalized tool call - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() } else if (toolUseIndex !== undefined) { // finalizeStreamingToolCall returned null (malformed JSON or missing args) // We still need to mark the tool as non-partial so it gets executed @@ -3256,7 +3312,8 @@ export class Task extends EventEmitter implements TaskLike { this.userMessageContentReady = false // Present the tool call - validation will handle missing params - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() } } } @@ -3455,7 +3512,8 @@ export class Task extends EventEmitter implements TaskLike { // If there is content to update then it will complete and // update `this.userMessageContentReady` to true, which we // `pWaitFor` before making the next request. - presentAssistantMessage(this) + /* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */ + this.presentAssistantMessageSafe() } if (hasTextContent || hasToolUses) { @@ -4191,10 +4249,14 @@ export class Task extends EventEmitter implements TaskLike { const iterator = stream[Symbol.asyncIterator]() // Set up abort handling - when the signal is aborted, clean up the controller reference - abortSignal.addEventListener("abort", () => { - console.log(`[Task#${this.taskId}.${this.instanceId}] AbortSignal triggered for current request`) - this.currentRequestAbortController = undefined - }, { once: true }) + abortSignal.addEventListener( + "abort", + () => { + console.log(`[Task#${this.taskId}.${this.instanceId}] AbortSignal triggered for current request`) + this.currentRequestAbortController = undefined + }, + { once: true }, + ) try { // Awaiting first chunk to see if it will throw an error. @@ -4206,9 +4268,13 @@ export class Task extends EventEmitter implements TaskLike { if (abortSignal.aborted) { reject(new Error("Request cancelled by user")) } else { - abortSignal.addEventListener("abort", () => { - reject(new Error("Request cancelled by user")) - }, { once: true }) + abortSignal.addEventListener( + "abort", + () => { + reject(new Error("Request cancelled by user")) + }, + { once: true }, + ) } }) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 5c10771686..48ec7d40d4 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -1479,7 +1479,7 @@ describe("Cline", () => { ] // Call submitUserMessage - task.submitUserMessage("test message", ["image1.png"]) + await task.submitUserMessage("test message", ["image1.png"]) // Verify handleWebviewAskResponse was called directly (not webview) expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "test message", ["image1.png"]) @@ -1499,13 +1499,13 @@ describe("Cline", () => { const handleResponseSpy = vi.spyOn(task, "handleWebviewAskResponse") // Call with empty text and no images - task.submitUserMessage("", []) + await task.submitUserMessage("", []) // Should not call handleWebviewAskResponse for empty messages expect(handleResponseSpy).not.toHaveBeenCalled() // Call with whitespace only - task.submitUserMessage(" ", []) + await task.submitUserMessage(" ", []) expect(handleResponseSpy).not.toHaveBeenCalled() }) @@ -1522,7 +1522,7 @@ describe("Cline", () => { // Test with no messages (new task scenario) task.clineMessages = [] - task.submitUserMessage("new task", ["image1.png"]) + await task.submitUserMessage("new task", ["image1.png"]) expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "new task", ["image1.png"]) @@ -1538,7 +1538,7 @@ describe("Cline", () => { text: "Initial message", }, ] - task.submitUserMessage("follow-up message", ["image2.png"]) + await task.submitUserMessage("follow-up message", ["image2.png"]) expect(handleResponseSpy).toHaveBeenCalledWith("messageResponse", "follow-up message", ["image2.png"]) }) @@ -1565,7 +1565,7 @@ describe("Cline", () => { const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) // Should log error but not throw - task.submitUserMessage("test message") + await task.submitUserMessage("test message") expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#submitUserMessage] Provider reference lost") expect(handleResponseSpy).not.toHaveBeenCalled() @@ -2351,6 +2351,169 @@ describe("Cline", () => { startTaskSpy.mockRestore() }) }) + + describe("unhandled-rejection guards on void async calls", () => { + // PR #253 wired `.catch(...)` onto every fire-and-forget async call that + // Copilot flagged as a potential unhandled-rejection source. These specs + // pin that behavior so a future refactor cannot silently drop the + // handler and reintroduce the crash risk on the extension host. + + const flushMicrotasks = () => new Promise((resolve) => setImmediate(resolve)) + + let consoleErrorSpy: ReturnType + + beforeEach(() => { + consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {}) + }) + + afterEach(() => { + consoleErrorSpy.mockRestore() + }) + + it("logs (instead of crashing) when startTask rejects from the constructor", async () => { + const boom = new Error("startTask boom") + const startTaskSpy = vi.spyOn(Task.prototype as any, "startTask").mockImplementation(async () => { + throw boom + }) + + new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: true, + }) + + expect(startTaskSpy).toHaveBeenCalledTimes(1) + await flushMicrotasks() + + expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#constructor] startTask failed:", boom) + startTaskSpy.mockRestore() + }) + + it("logs (instead of crashing) when resumeTaskFromHistory rejects from the constructor", async () => { + const boom = new Error("resume boom") + const resumeSpy = vi.spyOn(Task.prototype as any, "resumeTaskFromHistory").mockImplementation(async () => { + throw boom + }) + + new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + historyItem: { + id: "123", + number: 0, + ts: Date.now(), + task: "historical task", + tokensIn: 100, + tokensOut: 200, + cacheWrites: 0, + cacheReads: 0, + totalCost: 0.001, + }, + startTask: true, + }) + + expect(resumeSpy).toHaveBeenCalledTimes(1) + await flushMicrotasks() + + expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#constructor] resumeTaskFromHistory failed:", boom) + resumeSpy.mockRestore() + }) + + it("logs (instead of crashing) when postStateToWebviewWithoutTaskHistory rejects from the queue handler", async () => { + const boom = new Error("postState boom") + mockProvider.postStateToWebviewWithoutTaskHistory = vi.fn().mockRejectedValue(boom) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Triggers messageQueueStateChangedHandler -> void postStateToWebviewWithoutTaskHistory() + task.messageQueueService.addMessage("queued text") + await flushMicrotasks() + + expect(mockProvider.postStateToWebviewWithoutTaskHistory).toHaveBeenCalled() + expect(consoleErrorSpy).toHaveBeenCalledWith( + "[Task#messageQueueStateChangedHandler] postStateToWebviewWithoutTaskHistory failed:", + boom, + ) + }) + + it("logs (instead of crashing) when startTask rejects from start()", async () => { + const boom = new Error("start() boom") + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + vi.spyOn(task as any, "startTask").mockImplementation(async () => { + throw boom + }) + + task.start() + await flushMicrotasks() + + expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#start] startTask failed:", boom) + }) + + it("swallows the expected abort rejection from presentAssistantMessageSafe", async () => { + const assistantMessageModule = await import("../../assistant-message") + const presentSpy = vi + .spyOn(assistantMessageModule, "presentAssistantMessage") + .mockRejectedValue(new Error("[Task#presentAssistantMessage] task t.i aborted")) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Drain any unrelated console.error noise emitted by async constructor side effects + // (CloudService/getState complaints in the test harness) so we only assert on the + // abort-path behavior under test. + await flushMicrotasks() + consoleErrorSpy.mockClear() + + task.abort = true + ;(task as any).presentAssistantMessageSafe() + await flushMicrotasks() + + expect(presentSpy).toHaveBeenCalledTimes(1) + const presentErrors = consoleErrorSpy.mock.calls.filter( + (call) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"), + ) + expect(presentErrors).toHaveLength(0) + }) + + it("logs non-abort rejections from presentAssistantMessageSafe", async () => { + const assistantMessageModule = await import("../../assistant-message") + const boom = new Error("present boom") + const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(boom) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + expect(task.abort).toBeFalsy() + ;(task as any).presentAssistantMessageSafe() + await flushMicrotasks() + + expect(presentSpy).toHaveBeenCalledTimes(1) + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("[Task#presentAssistantMessage] task"), + boom, + ) + }) + }) }) describe("Queued message processing after condense", () => { diff --git a/src/eslint.config.mjs b/src/eslint.config.mjs index c488d77f25..5c866ef941 100644 --- a/src/eslint.config.mjs +++ b/src/eslint.config.mjs @@ -33,7 +33,7 @@ export default [ { // Ratchet: enforce no-floating-promises directory by directory. Each // directory is added here once its floating promises are resolved. - files: ["activate/**/*.ts"], + files: ["activate/**/*.ts", "core/task/**/*.ts"], languageOptions: { parserOptions: { project: true, From c5d8d4ad50112a4addfc02e32e434fefd3b53ab5 Mon Sep 17 00:00:00 2001 From: 0xMink <260166390+0xMink@users.noreply.github.com> Date: Sun, 24 May 2026 12:16:54 +0000 Subject: [PATCH 2/5] fix(Task): match the abort error message in presentAssistantMessageSafe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/core/task/Task.ts | 9 +++- src/core/task/__tests__/Task.spec.ts | 61 ++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 1 deletion(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index c863584b53..9295f4f0f2 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -365,7 +365,14 @@ export class Task extends EventEmitter implements TaskLike { */ private presentAssistantMessageSafe(): void { void presentAssistantMessage(this).catch((error) => { - if (this.abort) { + // Discriminate on the error message rather than `this.abort` state, + // which can flip between the throw and the catch microtask running: + // a real failure followed by an abort flip would otherwise be + // silently swallowed, and a stale abort error logged as a failure. + // The abort throw site in presentAssistantMessage emits a message + // ending in "aborted" (matching the other abort-throw contracts in + // this file), so we suppress exactly that. + if (error instanceof Error && error.message.endsWith("aborted")) { return } console.error(`[Task#presentAssistantMessage] task ${this.taskId}.${this.instanceId} failed:`, error) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 48ec7d40d4..b5d1ea4bef 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2513,6 +2513,67 @@ describe("Cline", () => { boom, ) }) + + it("logs a non-abort error even when this.abort flips true after the throw", async () => { + // Pins that the message-based discriminator is load-bearing, not the + // state check. Under the previous `if (this.abort) return` guard this + // case (a genuine downstream failure racing with an abort flip between + // the throw and the catch microtask) would silently swallow the error. + const assistantMessageModule = await import("../../assistant-message") + const realError = new Error("genuine downstream failure") + const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(realError) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + await flushMicrotasks() + consoleErrorSpy.mockClear() + + // Simulate the TOCTOU race: abort flips between throw and catch. + task.abort = true + ;(task as any).presentAssistantMessageSafe() + await flushMicrotasks() + + expect(presentSpy).toHaveBeenCalledTimes(1) + expect(consoleErrorSpy).toHaveBeenCalledWith( + expect.stringContaining("[Task#presentAssistantMessage] task"), + realError, + ) + }) + + it("suppresses an abort-pattern error by message match even when this.abort is false", async () => { + // Pins the inverse: message wins over state. A stale abort rejection + // arriving before `this.abort` has been observed as true must still be + // suppressed, so the catch handler never logs the expected + // cancellation rejection as a real failure. + const assistantMessageModule = await import("../../assistant-message") + const abortError = new Error("[Task#presentAssistantMessage] task t.i aborted") + const presentSpy = vi.spyOn(assistantMessageModule, "presentAssistantMessage").mockRejectedValue(abortError) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + await flushMicrotasks() + consoleErrorSpy.mockClear() + + expect(task.abort).toBeFalsy() + ;(task as any).presentAssistantMessageSafe() + await flushMicrotasks() + + expect(presentSpy).toHaveBeenCalledTimes(1) + const presentErrors = consoleErrorSpy.mock.calls.filter( + (call) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"), + ) + expect(presentErrors).toHaveLength(0) + }) }) }) From e57c7720f4eca9645530534d8fc67956a3d19dd0 Mon Sep 17 00:00:00 2001 From: 0xMink <260166390+0xMink@users.noreply.github.com> Date: Sun, 24 May 2026 12:16:54 +0000 Subject: [PATCH 3/5] fix(Task): catch updateClineMessage rejections on partial-message paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The four fire-and-forget updateClineMessage calls in say()/ask() partial-message branches were void-prefixed without .catch arms. The callee's webview post is internally guarded, but its synchronous RooCodeEventName.Message emit can throw via a consumer-attached listener — so the void sites can produce unhandled rejections. Replace each `void this.updateClineMessage(...)` with an explicit .catch arm that logs the error, matching the other rejection-handling sites in this file. Two new tests pin the say() and ask() catch arms. Also pin the void on getCheckpointService with a rationale comment: its top-level try/catch returns undefined on failure, so .catch is not needed there. --- src/core/task/Task.ts | 22 +++++--- src/core/task/__tests__/Task.spec.ts | 77 +++++++++++++++++++++++++++- 2 files changed, 91 insertions(+), 8 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 9295f4f0f2..8f59bf5094 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1197,8 +1197,13 @@ export class Task extends EventEmitter implements TaskLike { // data or one whole message at a time so ignore partial for // saves, and only post parts of partial message instead of // whole array in new listener. - /* v8 ignore next -- fire-and-forget webview update; rejection is benign */ - void this.updateClineMessage(lastMessage) + // Fire-and-forget: the webview post is internally guarded, but + // the `RooCodeEventName.Message` emit can synchronously throw + // if any consumer-attached listener does, which would surface + // here as an unhandled rejection. Log it instead. + this.updateClineMessage(lastMessage).catch((error) => { + console.error("[Task#ask] updateClineMessage failed:", error) + }) // console.log("Task#ask: current ask promise was ignored (#1)") throw new AskIgnoredError("updating existing partial") } else { @@ -1239,8 +1244,11 @@ export class Task extends EventEmitter implements TaskLike { lastMessage.isAnswered = true } await this.saveClineMessages() - /* v8 ignore next -- fire-and-forget webview update; rejection is benign */ - void this.updateClineMessage(lastMessage) + // Fire-and-forget: see updateClineMessage call above for the + // rationale on the .catch arm. + this.updateClineMessage(lastMessage).catch((error) => { + console.error("[Task#ask] updateClineMessage failed:", error) + }) } else { // This is a new and complete message, so add it like normal. this.askResponse = undefined @@ -1700,8 +1708,10 @@ export class Task extends EventEmitter implements TaskLike { lastMessage.images = images lastMessage.partial = partial lastMessage.progressStatus = progressStatus - // Fire-and-forget: see updateClineMessage call above for the - // rationale on the .catch arm. + // Fire-and-forget: webview post is internally guarded, but the + // `RooCodeEventName.Message` emit can synchronously throw via a + // consumer-attached listener. Surface that as a log, not an + // unhandled rejection. this.updateClineMessage(lastMessage).catch((error) => { console.error("[Task#say] updateClineMessage failed:", error) }) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index b5d1ea4bef..05ee34f253 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2486,7 +2486,7 @@ describe("Cline", () => { expect(presentSpy).toHaveBeenCalledTimes(1) const presentErrors = consoleErrorSpy.mock.calls.filter( - (call) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"), + (call: unknown[]) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"), ) expect(presentErrors).toHaveLength(0) }) @@ -2570,10 +2570,83 @@ describe("Cline", () => { expect(presentSpy).toHaveBeenCalledTimes(1) const presentErrors = consoleErrorSpy.mock.calls.filter( - (call) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"), + (call: unknown[]) => typeof call[0] === "string" && call[0].includes("[Task#presentAssistantMessage]"), ) expect(presentErrors).toHaveLength(0) }) + + it("logs (instead of crashing) when updateClineMessage rejects from the say() partial-update path", async () => { + // Pins the symmetric .catch arm on the fire-and-forget + // updateClineMessage call in say(). The callee's webview post is + // internally guarded, but its synchronous emit can throw via a + // consumer-attached listener — that path must surface as a log, + // not an unhandled rejection. + const boom = new Error("updateClineMessage boom") + const updateSpy = vi.spyOn(Task.prototype as any, "updateClineMessage").mockImplementation(async () => { + throw boom + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Seed a prior partial "say" so the partial-update branch fires. + task.clineMessages.push({ + ts: Date.now() - 1, + type: "say", + say: "text", + text: "partial", + partial: true, + }) + + await task.say("text", "updated partial", undefined, true) + await flushMicrotasks() + + expect(updateSpy).toHaveBeenCalled() + expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#say] updateClineMessage failed:", boom) + updateSpy.mockRestore() + }) + + it("logs (instead of crashing) when updateClineMessage rejects from the ask() complete-partial path", async () => { + // Pins the symmetric .catch arm on the fire-and-forget + // updateClineMessage call in ask() when finalizing a partial. + const boom = new Error("updateClineMessage boom") + const updateSpy = vi.spyOn(Task.prototype as any, "updateClineMessage").mockImplementation(async () => { + throw boom + }) + const saveSpy = vi.spyOn(Task.prototype as any, "saveClineMessages").mockResolvedValue(true) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Seed a prior partial "ask" of type "tool" so the complete-partial + // branch fires when ask("tool", ..., false) is called. + task.clineMessages.push({ + ts: Date.now() - 1, + type: "ask", + ask: "tool", + text: "partial", + partial: true, + }) + + // ask() resolves only after a response — fire-and-forget so the + // promise the suite awaits stays bounded. The .catch on the + // pending ask handles the never-resolved promise. + void task.ask("tool", "complete", false).catch(() => {}) + await flushMicrotasks() + + expect(updateSpy).toHaveBeenCalled() + expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#ask] updateClineMessage failed:", boom) + updateSpy.mockRestore() + saveSpy.mockRestore() + }) }) }) From 8fa009ceebccdd6708bdf3a452188774fce18010 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 30 Jun 2026 02:32:58 +0000 Subject: [PATCH 4/5] feat(Task): adding catch block to log any errors --- src/core/task/Task.ts | 9 ++++++--- src/core/task/__tests__/Task.spec.ts | 1 + 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index 8f59bf5094..fa81697431 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1319,8 +1319,9 @@ export class Task extends EventEmitter implements TaskLike { if (message) { this.interactiveAsk = message this.emit(RooCodeEventName.TaskInteractive, this.taskId) - /* v8 ignore next -- fire-and-forget webview notification inside setTimeout; rejection is benign */ - void provider?.postMessageToWebview({ type: "interactionRequired" }) + void provider?.postMessageToWebview({ type: "interactionRequired" }).catch((error) => { + console.error("[Task#ask] postMessageToWebview interactionRequired failed:", error) + }) } }, statusMutationTimeout), ) @@ -1460,7 +1461,9 @@ export class Task extends EventEmitter implements TaskLike { ) if (lastToolAskIndex !== -1) { this.clineMessages[lastToolAskIndex].isAnswered = true - void this.updateClineMessage(this.clineMessages[lastToolAskIndex]) + void this.updateClineMessage(this.clineMessages[lastToolAskIndex]).catch((error) => { + console.error("[Task#handleWebviewAskResponse] updateClineMessage failed:", error) + }) this.saveClineMessages().catch((error) => { console.error("Failed to save answered tool-ask state:", error) }) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index 05ee34f253..f973e5f87c 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2368,6 +2368,7 @@ describe("Cline", () => { afterEach(() => { consoleErrorSpy.mockRestore() + vi.restoreAllMocks() }) it("logs (instead of crashing) when startTask rejects from the constructor", async () => { From 6686bbf31b382a495df35d06c9c2bc7007ab5026 Mon Sep 17 00:00:00 2001 From: Elliott de Launay Date: Tue, 30 Jun 2026 02:57:44 +0000 Subject: [PATCH 5/5] test(Task): coverage --- .../presentAssistantMessage.ts | 5 +- src/core/task/Task.ts | 1 + src/core/task/__tests__/Task.spec.ts | 69 +++++++++++++++++++ 3 files changed, 72 insertions(+), 3 deletions(-) diff --git a/src/core/assistant-message/presentAssistantMessage.ts b/src/core/assistant-message/presentAssistantMessage.ts index cc675dd948..f71b5cc1bd 100644 --- a/src/core/assistant-message/presentAssistantMessage.ts +++ b/src/core/assistant-message/presentAssistantMessage.ts @@ -959,8 +959,7 @@ export async function presentAssistantMessage(cline: Task) { if (cline.currentStreamingContentIndex < cline.assistantMessageContent.length) { // There are already more content blocks to stream, so we'll call // this function ourselves. - presentAssistantMessage(cline) - return + return presentAssistantMessage(cline) } else { // CRITICAL FIX: If we're out of bounds and the stream is complete, set userMessageContentReady // This handles the case where assistantMessageContent is empty or becomes empty after processing @@ -972,7 +971,7 @@ export async function presentAssistantMessage(cline: Task) { // Block is partial, but the read stream may have finished. if (cline.presentAssistantMessageHasPendingUpdates) { - presentAssistantMessage(cline) + return presentAssistantMessage(cline) } } diff --git a/src/core/task/Task.ts b/src/core/task/Task.ts index fa81697431..6e8d70f349 100644 --- a/src/core/task/Task.ts +++ b/src/core/task/Task.ts @@ -1319,6 +1319,7 @@ export class Task extends EventEmitter implements TaskLike { if (message) { this.interactiveAsk = message this.emit(RooCodeEventName.TaskInteractive, this.taskId) + /* v8 ignore next 3 -- fires inside 2s timer after ask() resolves; not reachable in unit tests */ void provider?.postMessageToWebview({ type: "interactionRequired" }).catch((error) => { console.error("[Task#ask] postMessageToWebview interactionRequired failed:", error) }) diff --git a/src/core/task/__tests__/Task.spec.ts b/src/core/task/__tests__/Task.spec.ts index f973e5f87c..27ba5ce8ff 100644 --- a/src/core/task/__tests__/Task.spec.ts +++ b/src/core/task/__tests__/Task.spec.ts @@ -2648,6 +2648,75 @@ describe("Cline", () => { updateSpy.mockRestore() saveSpy.mockRestore() }) + + it("logs (instead of crashing) when updateClineMessage rejects from the ask() ignore-partial path", async () => { + // Pins the .catch arm on the fire-and-forget updateClineMessage call + // in ask() when a new partial ask arrives while the previous partial + // is still pending (AskIgnoredError path). + const boom = new Error("updateClineMessage boom") + const updateSpy = vi.spyOn(Task.prototype as any, "updateClineMessage").mockImplementation(async () => { + throw boom + }) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Seed a prior partial ask so the isUpdatingPreviousPartial branch fires. + task.clineMessages.push({ + ts: Date.now() - 1, + type: "ask", + ask: "tool", + text: "partial", + partial: true, + }) + + // Sending a new partial of the same type triggers updateClineMessage + // then throws AskIgnoredError — catch it so the test doesn't fail. + await task.ask("tool", "updated partial", true).catch(() => {}) + await flushMicrotasks() + + expect(updateSpy).toHaveBeenCalled() + expect(consoleErrorSpy).toHaveBeenCalledWith("[Task#ask] updateClineMessage failed:", boom) + }) + + it("logs (instead of crashing) when updateClineMessage rejects from handleWebviewAskResponse", async () => { + // Pins the .catch arm on the fire-and-forget updateClineMessage call + // in handleWebviewAskResponse when marking a tool ask as answered. + const boom = new Error("updateClineMessage boom") + const updateSpy = vi.spyOn(Task.prototype as any, "updateClineMessage").mockImplementation(async () => { + throw boom + }) + vi.spyOn(Task.prototype as any, "saveClineMessages").mockResolvedValue(undefined) + + const task = new Task({ + provider: mockProvider, + apiConfiguration: mockApiConfig, + task: "test task", + startTask: false, + }) + + // Seed an unanswered tool ask so the lastToolAskIndex branch fires. + task.clineMessages.push({ + ts: Date.now() - 1, + type: "ask", + ask: "tool", + text: "tool call", + partial: false, + }) + + task.handleWebviewAskResponse("yesButtonClicked") + await flushMicrotasks() + + expect(updateSpy).toHaveBeenCalled() + expect(consoleErrorSpy).toHaveBeenCalledWith( + "[Task#handleWebviewAskResponse] updateClineMessage failed:", + boom, + ) + }) }) })