Skip to content

Commit 2490eeb

Browse files
author
CodeKing
committed
chore: enforce no-floating-promises (Zoo-Code-Org#253) [imported from upstream 8849f1a]
1 parent e250b6b commit 2490eeb

4 files changed

Lines changed: 471 additions & 30 deletions

File tree

src/core/assistant-message/presentAssistantMessage.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,8 +959,7 @@ export async function presentAssistantMessage(cline: Task) {
959959
if (cline.currentStreamingContentIndex < cline.assistantMessageContent.length) {
960960
// There are already more content blocks to stream, so we'll call
961961
// this function ourselves.
962-
presentAssistantMessage(cline)
963-
return
962+
return presentAssistantMessage(cline)
964963
} else {
965964
// CRITICAL FIX: If we're out of bounds and the stream is complete, set userMessageContentReady
966965
// This handles the case where assistantMessageContent is empty or becomes empty after processing
@@ -972,7 +971,7 @@ export async function presentAssistantMessage(cline: Task) {
972971

973972
// Block is partial, but the read stream may have finished.
974973
if (cline.presentAssistantMessageHasPendingUpdates) {
975-
presentAssistantMessage(cline)
974+
return presentAssistantMessage(cline)
976975
}
977976
}
978977

src/core/task/Task.ts

Lines changed: 95 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,29 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
356356
*/
357357
assistantMessageSavedToHistory = false
358358

359+
/**
360+
* Fire-and-forget wrapper around `presentAssistantMessage` that swallows the
361+
* expected cancellation rejection (the presenter throws when `this.abort` is set)
362+
* and logs any other failure. Keeping it non-blocking preserves the streaming
363+
* presenter's self-locking semantics while preventing unhandled promise rejections
364+
* from crashing the extension host.
365+
*/
366+
private presentAssistantMessageSafe(): void {
367+
void presentAssistantMessage(this).catch((error) => {
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")) {
376+
return
377+
}
378+
console.error(`[Task#presentAssistantMessage] task ${this.taskId}.${this.instanceId} failed:`, error)
379+
})
380+
}
381+
359382
/**
360383
* Push a tool_result block to userMessageContent, preventing duplicates.
361384
* Duplicate tool_use_ids cause API errors.
@@ -522,7 +545,15 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
522545
this.messageQueueStateChangedHandler = () => {
523546
this.emit(RooCodeEventName.TaskUserMessage, this.taskId)
524547
this.emit(RooCodeEventName.QueuedMessagesUpdated, this.taskId, this.messageQueueService.messages)
525-
this.providerRef.deref()?.postStateToWebviewWithoutTaskHistory()
548+
void this.providerRef
549+
.deref()
550+
?.postStateToWebviewWithoutTaskHistory()
551+
.catch((error) => {
552+
console.error(
553+
"[Task#messageQueueStateChangedHandler] postStateToWebviewWithoutTaskHistory failed:",
554+
error,
555+
)
556+
})
526557
}
527558

528559
this.messageQueueService.on("stateChanged", this.messageQueueStateChangedHandler)
@@ -567,9 +598,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
567598
if (startTask) {
568599
this._started = true
569600
if (task || images) {
570-
this.startTask(task, images)
601+
void this.startTask(task, images).catch((error) => {
602+
console.error("[Task#constructor] startTask failed:", error)
603+
})
571604
} else if (historyItem) {
572-
this.resumeTaskFromHistory()
605+
void this.resumeTaskFromHistory().catch((error) => {
606+
console.error("[Task#constructor] resumeTaskFromHistory failed:", error)
607+
})
573608
} else {
574609
throw new Error("Either historyItem or task/images must be provided")
575610
}
@@ -1162,7 +1197,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
11621197
// data or one whole message at a time so ignore partial for
11631198
// saves, and only post parts of partial message instead of
11641199
// whole array in new listener.
1165-
this.updateClineMessage(lastMessage)
1200+
// Fire-and-forget: the webview post is internally guarded, but
1201+
// the `RooCodeEventName.Message` emit can synchronously throw
1202+
// if any consumer-attached listener does, which would surface
1203+
// here as an unhandled rejection. Log it instead.
1204+
this.updateClineMessage(lastMessage).catch((error) => {
1205+
console.error("[Task#ask] updateClineMessage failed:", error)
1206+
})
11661207
// console.log("Task#ask: current ask promise was ignored (#1)")
11671208
throw new AskIgnoredError("updating existing partial")
11681209
} else {
@@ -1203,7 +1244,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
12031244
lastMessage.isAnswered = true
12041245
}
12051246
await this.saveClineMessages()
1206-
this.updateClineMessage(lastMessage)
1247+
// Fire-and-forget: see updateClineMessage call above for the
1248+
// rationale on the .catch arm.
1249+
this.updateClineMessage(lastMessage).catch((error) => {
1250+
console.error("[Task#ask] updateClineMessage failed:", error)
1251+
})
12071252
} else {
12081253
// This is a new and complete message, so add it like normal.
12091254
this.askResponse = undefined
@@ -1274,7 +1319,10 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
12741319
if (message) {
12751320
this.interactiveAsk = message
12761321
this.emit(RooCodeEventName.TaskInteractive, this.taskId)
1277-
provider?.postMessageToWebview({ type: "interactionRequired" })
1322+
/* v8 ignore next 3 -- fires inside 2s timer after ask() resolves; not reachable in unit tests */
1323+
void provider?.postMessageToWebview({ type: "interactionRequired" }).catch((error) => {
1324+
console.error("[Task#ask] postMessageToWebview interactionRequired failed:", error)
1325+
})
12781326
}
12791327
}, statusMutationTimeout),
12801328
)
@@ -1414,7 +1462,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
14141462
)
14151463
if (lastToolAskIndex !== -1) {
14161464
this.clineMessages[lastToolAskIndex].isAnswered = true
1417-
void this.updateClineMessage(this.clineMessages[lastToolAskIndex])
1465+
void this.updateClineMessage(this.clineMessages[lastToolAskIndex]).catch((error) => {
1466+
console.error("[Task#handleWebviewAskResponse] updateClineMessage failed:", error)
1467+
})
14181468
this.saveClineMessages().catch((error) => {
14191469
console.error("Failed to save answered tool-ask state:", error)
14201470
})
@@ -1662,7 +1712,13 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
16621712
lastMessage.images = images
16631713
lastMessage.partial = partial
16641714
lastMessage.progressStatus = progressStatus
1665-
this.updateClineMessage(lastMessage)
1715+
// Fire-and-forget: webview post is internally guarded, but the
1716+
// `RooCodeEventName.Message` emit can synchronously throw via a
1717+
// consumer-attached listener. Surface that as a log, not an
1718+
// unhandled rejection.
1719+
this.updateClineMessage(lastMessage).catch((error) => {
1720+
console.error("[Task#say] updateClineMessage failed:", error)
1721+
})
16661722
} else {
16671723
// This is a new partial message, so add it with partial state.
16681724
const sayTs = Date.now()
@@ -1701,7 +1757,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
17011757
await this.saveClineMessages()
17021758

17031759
// More performant than an entire `postStateToWebview`.
1704-
this.updateClineMessage(lastMessage)
1760+
// Fire-and-forget: see updateClineMessage call above for the
1761+
// rationale on the .catch arm.
1762+
this.updateClineMessage(lastMessage).catch((error) => {
1763+
console.error("[Task#say] updateClineMessage failed:", error)
1764+
})
17051765
} else {
17061766
// This is a new and complete message, so add it like normal.
17071767
const sayTs = Date.now()
@@ -1810,7 +1870,9 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
18101870
const { task, images } = this.metadata
18111871

18121872
if (task || images) {
1813-
this.startTask(task ?? undefined, images ?? undefined)
1873+
void this.startTask(task ?? undefined, images ?? undefined).catch((error) => {
1874+
console.error("[Task#start] startTask failed:", error)
1875+
})
18141876
}
18151877
}
18161878

@@ -2351,7 +2413,11 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
23512413

23522414
private async initiateTaskLoop(userContent: Anthropic.Messages.ContentBlockParam[]): Promise<void> {
23532415
// Kicks off the checkpoints initialization process in the background.
2354-
getCheckpointService(this)
2416+
// `getCheckpointService` wraps its full body in a try/catch and returns
2417+
// `undefined` on failure (see src/core/checkpoints/index.ts), so the
2418+
// returned promise cannot reject. `void` is sufficient — no `.catch`
2419+
// arm needed.
2420+
void getCheckpointService(this)
23552421

23562422
let nextUserContent = userContent
23572423
let includeFileDetails = true
@@ -2798,7 +2864,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
27982864
// Add to content and present
27992865
this.assistantMessageContent.push(partialToolUse)
28002866
this.userMessageContentReady = false
2801-
presentAssistantMessage(this)
2867+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
2868+
this.presentAssistantMessageSafe()
28022869
} else if (event.type === "tool_call_delta") {
28032870
// Process chunk using streaming JSON parser
28042871
const partialToolUse = NativeToolCallParser.processStreamingChunk(
@@ -2817,7 +2884,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
28172884
this.assistantMessageContent[toolUseIndex] = partialToolUse
28182885

28192886
// Present updated tool use
2820-
presentAssistantMessage(this)
2887+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
2888+
this.presentAssistantMessageSafe()
28212889
}
28222890
}
28232891
} else if (event.type === "tool_call_end") {
@@ -2843,7 +2911,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
28432911
this.userMessageContentReady = false
28442912

28452913
// Present the finalized tool call
2846-
presentAssistantMessage(this)
2914+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
2915+
this.presentAssistantMessageSafe()
28472916
} else if (toolUseIndex !== undefined) {
28482917
// finalizeStreamingToolCall returned null (malformed JSON or missing args)
28492918
// Mark the tool as non-partial so it's presented as complete, but execution
@@ -2862,7 +2931,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
28622931
this.userMessageContentReady = false
28632932

28642933
// Present the tool call - validation will handle missing params
2865-
presentAssistantMessage(this)
2934+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
2935+
this.presentAssistantMessageSafe()
28662936
}
28672937
}
28682938
}
@@ -2895,7 +2965,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
28952965

28962966
// Present the tool call to user - presentAssistantMessage will execute
28972967
// tools sequentially and accumulate all results in userMessageContent
2898-
presentAssistantMessage(this)
2968+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
2969+
this.presentAssistantMessageSafe()
28992970
break
29002971
}
29012972
case "text": {
@@ -2914,7 +2985,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
29142985
})
29152986
this.userMessageContentReady = false
29162987
}
2917-
presentAssistantMessage(this)
2988+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
2989+
this.presentAssistantMessageSafe()
29182990
break
29192991
}
29202992
}
@@ -3241,7 +3313,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
32413313
this.userMessageContentReady = false
32423314

32433315
// Present the finalized tool call
3244-
presentAssistantMessage(this)
3316+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
3317+
this.presentAssistantMessageSafe()
32453318
} else if (toolUseIndex !== undefined) {
32463319
// finalizeStreamingToolCall returned null (malformed JSON or missing args)
32473320
// We still need to mark the tool as non-partial so it gets executed
@@ -3260,7 +3333,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
32603333
this.userMessageContentReady = false
32613334

32623335
// Present the tool call - validation will handle missing params
3263-
presentAssistantMessage(this)
3336+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
3337+
this.presentAssistantMessageSafe()
32643338
}
32653339
}
32663340
}
@@ -3459,7 +3533,8 @@ export class Task extends EventEmitter<TaskEvents> implements TaskLike {
34593533
// If there is content to update then it will complete and
34603534
// update `this.userMessageContentReady` to true, which we
34613535
// `pWaitFor` before making the next request.
3462-
presentAssistantMessage(this)
3536+
/* v8 ignore next -- streaming presenter; .catch lives in presentAssistantMessageSafe (covered) */
3537+
this.presentAssistantMessageSafe()
34633538
}
34643539

34653540
if (hasTextContent || hasToolUses) {

0 commit comments

Comments
 (0)