Skip to content

Commit 77b76a8

Browse files
authored
Handle cancel/resume abort races without crashing (#11422)
1 parent b51af98 commit 77b76a8

File tree

2 files changed

+113
-3
lines changed

2 files changed

+113
-3
lines changed

src/core/task/Task.ts

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -749,15 +749,49 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
749749
if (startTask) {
750750
this._started = true
751751
if (task || images) {
752-
this.startTask(task, images)
752+
this.runLifecycleTaskInBackground(this.startTask(task, images), "startTask")
753753
} else if (historyItem) {
754-
this.resumeTaskFromHistory()
754+
this.runLifecycleTaskInBackground(this.resumeTaskFromHistory(), "resumeTaskFromHistory")
755755
} else {
756756
throw new Error("Either historyItem or task/images must be provided")
757757
}
758758
}
759759
}
760760

761+
private runLifecycleTaskInBackground(taskPromise: Promise<void>, operation: "startTask" | "resumeTaskFromHistory") {
762+
void taskPromise.catch((error) => {
763+
if (this.shouldIgnoreBackgroundLifecycleError(error)) {
764+
return
765+
}
766+
767+
console.error(
768+
`[Task#${operation}] task ${this.taskId}.${this.instanceId} failed: ${
769+
error instanceof Error ? error.message : String(error)
770+
}`,
771+
)
772+
})
773+
}
774+
775+
private shouldIgnoreBackgroundLifecycleError(error: unknown): boolean {
776+
if (error instanceof AskIgnoredError) {
777+
return true
778+
}
779+
780+
if (this.abandoned === true || this.abort === true || this.abortReason === "user_cancelled") {
781+
return true
782+
}
783+
784+
if (!(error instanceof Error)) {
785+
return false
786+
}
787+
788+
const abortedByCurrentTask =
789+
error.message.includes(`[RooCode#ask] task ${this.taskId}.${this.instanceId} aborted`) ||
790+
error.message.includes(`[RooCode#say] task ${this.taskId}.${this.instanceId} aborted`)
791+
792+
return abortedByCurrentTask
793+
}
794+
761795
/**
762796
* Initialize the task mode from the provider state.
763797
* This method handles async initialization with proper error handling.
@@ -2067,7 +2101,7 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
20672101
const { task, images } = this.metadata
20682102

20692103
if (task || images) {
2070-
this.startTask(task ?? undefined, images ?? undefined)
2104+
this.runLifecycleTaskInBackground(this.startTask(task ?? undefined, images ?? undefined), "startTask")
20712105
}
20722106
}
20732107

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,82 @@ describe("Cline", () => {
394394
new Task({ provider: mockProvider, apiConfiguration: mockApiConfig })
395395
}).toThrow("Either historyItem or task/images must be provided")
396396
})
397+
398+
it("should ignore cancelled background resumeTaskFromHistory errors", async () => {
399+
const resumeSpy = vi
400+
.spyOn(Task.prototype as any, "resumeTaskFromHistory")
401+
.mockImplementationOnce(async function (this: Task) {
402+
this.abort = true
403+
throw new Error("resume aborted")
404+
})
405+
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
406+
407+
new Task({
408+
provider: mockProvider,
409+
apiConfiguration: mockApiConfig,
410+
historyItem: {
411+
id: "history-task-id",
412+
number: 1,
413+
ts: Date.now(),
414+
task: "historical task",
415+
tokensIn: 0,
416+
tokensOut: 0,
417+
cacheWrites: 0,
418+
cacheReads: 0,
419+
totalCost: 0,
420+
} as any,
421+
startTask: true,
422+
})
423+
424+
await Promise.resolve()
425+
await Promise.resolve()
426+
427+
const lifecycleErrors = consoleErrorSpy.mock.calls.filter(
428+
([message]) => typeof message === "string" && message.includes("[Task#resumeTaskFromHistory]"),
429+
)
430+
expect(lifecycleErrors).toHaveLength(0)
431+
432+
resumeSpy.mockRestore()
433+
consoleErrorSpy.mockRestore()
434+
})
435+
436+
it("should log unexpected background resumeTaskFromHistory errors", async () => {
437+
const resumeSpy = vi
438+
.spyOn(Task.prototype as any, "resumeTaskFromHistory")
439+
.mockRejectedValueOnce(new Error("unexpected resume failure"))
440+
const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
441+
442+
new Task({
443+
provider: mockProvider,
444+
apiConfiguration: mockApiConfig,
445+
historyItem: {
446+
id: "history-task-id",
447+
number: 1,
448+
ts: Date.now(),
449+
task: "historical task",
450+
tokensIn: 0,
451+
tokensOut: 0,
452+
cacheWrites: 0,
453+
cacheReads: 0,
454+
totalCost: 0,
455+
} as any,
456+
startTask: true,
457+
})
458+
459+
await Promise.resolve()
460+
await Promise.resolve()
461+
462+
const lifecycleErrors = consoleErrorSpy.mock.calls.filter(
463+
([message]) =>
464+
typeof message === "string" &&
465+
message.includes("[Task#resumeTaskFromHistory]") &&
466+
message.includes("unexpected resume failure"),
467+
)
468+
expect(lifecycleErrors).toHaveLength(1)
469+
470+
resumeSpy.mockRestore()
471+
consoleErrorSpy.mockRestore()
472+
})
397473
})
398474

399475
describe("getEnvironmentDetails", () => {

0 commit comments

Comments
 (0)