Skip to content

Commit 311ea08

Browse files
roomoteroomote
committed
fix: improve stability and error handling to address issue #6045
- Add ErrorBoundary component to prevent UI crashes - Improve stream error handling in Task.ts with better recovery - Enhance abort/cancellation flow with timeouts to prevent hanging - Add long-running operation indicator for better user feedback - Fix timeout handling in ClineProvider to prevent indefinite waits - Add proper cleanup in dispose() method for streaming states These changes address the stability issues reported in #6045 by: 1. Preventing crashes from propagating through error boundaries 2. Handling stream failures gracefully with user notification 3. Ensuring operations don't hang indefinitely with proper timeouts 4. Providing visual feedback for long-running operations
1 parent b1bc085 commit 311ea08

File tree

6 files changed

+334
-41
lines changed

6 files changed

+334
-41
lines changed

‎src/core/task/Task.ts‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,13 @@ export class Task extends EventEmitter<ClineEvents> {
10721072
} catch (error) {
10731073
console.error("Error reverting diff changes:", error)
10741074
}
1075+
1076+
// Reset all streaming-related state
1077+
this.isStreaming = false
1078+
this.isWaitingForFirstChunk = false
1079+
this.didCompleteReadingStream = true
1080+
this.presentAssistantMessageLocked = false
1081+
this.presentAssistantMessageHasPendingUpdates = false
10751082
}
10761083

10771084
public async abortTask(isAbandoned = false) {
@@ -1091,6 +1098,10 @@ export class Task extends EventEmitter<ClineEvents> {
10911098
console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error)
10921099
// Don't rethrow - we want abort to always succeed
10931100
}
1101+
1102+
// Ensure streaming state is reset even if dispose fails
1103+
this.isStreaming = false
1104+
this.isWaitingForFirstChunk = false
10941105
// Save the countdown message in the automatic retry or other content.
10951106
try {
10961107
// Save the countdown message in the automatic retry or other content.
@@ -1359,6 +1370,24 @@ export class Task extends EventEmitter<ClineEvents> {
13591370
let reasoningMessage = ""
13601371
this.isStreaming = true
13611372

1373+
// Add a timeout for the entire streaming operation
1374+
const streamTimeout = setTimeout(
1375+
async () => {
1376+
if (this.isStreaming && !this.abort) {
1377+
console.error(`Stream timeout for task ${this.taskId} - aborting stream`)
1378+
await this.say(
1379+
"error",
1380+
"The API request timed out. This might be due to network issues or server problems. The task will be cancelled.",
1381+
undefined,
1382+
false,
1383+
)
1384+
// Trigger abort to clean up properly
1385+
this.abort = true
1386+
}
1387+
},
1388+
5 * 60 * 1000,
1389+
) // 5 minute timeout
1390+
13621391
try {
13631392
for await (const chunk of stream) {
13641393
if (!chunk) {
@@ -1451,19 +1480,25 @@ export class Task extends EventEmitter<ClineEvents> {
14511480
? undefined
14521481
: (error.message ?? JSON.stringify(serializeError(error), null, 2))
14531482

1454-
// Now call abortTask after determining the cancel reason
1455-
await this.abortTask()
1483+
try {
1484+
// Now call abortTask after determining the cancel reason
1485+
await this.abortTask()
14561486

1457-
await abortStream(cancelReason, streamingFailedMessage)
1487+
await abortStream(cancelReason, streamingFailedMessage)
14581488

1459-
const history = await provider?.getTaskWithId(this.taskId)
1489+
const history = await provider?.getTaskWithId(this.taskId)
14601490

1461-
if (history) {
1462-
await provider?.initClineWithHistoryItem(history.historyItem)
1491+
if (history) {
1492+
await provider?.initClineWithHistoryItem(history.historyItem)
1493+
}
1494+
} catch (abortError) {
1495+
console.error(`Error during stream failure recovery for task ${this.taskId}:`, abortError)
1496+
// Continue execution even if abort fails to prevent hanging
14631497
}
14641498
}
14651499
} finally {
14661500
this.isStreaming = false
1501+
clearTimeout(streamTimeout)
14671502
}
14681503

14691504
if (inputTokens > 0 || outputTokens > 0 || cacheWriteTokens > 0 || cacheReadTokens > 0) {

‎src/core/webview/ClineProvider.ts‎

Lines changed: 64 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -192,13 +192,20 @@ export class ClineProvider
192192
console.log(`[subtasks] removing task ${cline.taskId}.${cline.instanceId} from stack`)
193193

194194
try {
195-
// Abort the running task and set isAbandoned to true so
196-
// all running promises will exit as well.
197-
await cline.abortTask(true)
195+
// Set a timeout for the abort operation
196+
const abortPromise = cline.abortTask(true)
197+
const timeoutPromise = new Promise((_, reject) =>
198+
setTimeout(() => reject(new Error("Abort timeout")), 3000),
199+
)
200+
201+
// Race between abort completion and timeout
202+
await Promise.race([abortPromise, timeoutPromise])
198203
} catch (e) {
199204
this.log(
200205
`[subtasks] encountered error while aborting task ${cline.taskId}.${cline.instanceId}: ${e.message}`,
201206
)
207+
// Force abandon the task if abort fails
208+
cline.abandoned = true
202209
}
203210

204211
// Make sure no reference kept, once promises end it will be
@@ -971,38 +978,69 @@ export class ClineProvider
971978

972979
console.log(`[subtasks] cancelling task ${cline.taskId}.${cline.instanceId}`)
973980

974-
const { historyItem } = await this.getTaskWithId(cline.taskId)
975-
// Preserve parent and root task information for history item.
976-
const rootTask = cline.rootTask
977-
const parentTask = cline.parentTask
981+
let historyItem: HistoryItem | undefined
982+
let rootTask: Task | undefined
983+
let parentTask: Task | undefined
984+
985+
try {
986+
const taskData = await this.getTaskWithId(cline.taskId)
987+
historyItem = taskData.historyItem
988+
// Preserve parent and root task information for history item.
989+
rootTask = cline.rootTask
990+
parentTask = cline.parentTask
991+
} catch (error) {
992+
console.error(`Failed to get task history for ${cline.taskId}:`, error)
993+
// Continue with cancellation even if we can't get history
994+
}
978995

996+
// Start the abort process
979997
cline.abortTask()
980998

981-
await pWaitFor(
982-
() =>
983-
this.getCurrentCline()! === undefined ||
984-
this.getCurrentCline()!.isStreaming === false ||
985-
this.getCurrentCline()!.didFinishAbortingStream ||
986-
// If only the first chunk is processed, then there's no
987-
// need to wait for graceful abort (closes edits, browser,
988-
// etc).
989-
this.getCurrentCline()!.isWaitingForFirstChunk,
990-
{
991-
timeout: 3_000,
992-
},
993-
).catch(() => {
994-
console.error("Failed to abort task")
995-
})
999+
// Wait for abort to complete with a timeout
1000+
try {
1001+
await pWaitFor(
1002+
() => {
1003+
const currentCline = this.getCurrentCline()
1004+
return (
1005+
!currentCline ||
1006+
currentCline.taskId !== cline.taskId ||
1007+
!currentCline.isStreaming ||
1008+
currentCline.didFinishAbortingStream ||
1009+
currentCline.isWaitingForFirstChunk
1010+
)
1011+
},
1012+
{
1013+
timeout: 5_000, // Increased timeout for better reliability
1014+
interval: 100,
1015+
},
1016+
)
1017+
} catch (error) {
1018+
console.error("Timeout waiting for task abort, forcing cleanup")
1019+
// Force cleanup if timeout occurs
1020+
const currentCline = this.getCurrentCline()
1021+
if (currentCline && currentCline.taskId === cline.taskId) {
1022+
currentCline.abandoned = true
1023+
// Force remove from stack
1024+
await this.removeClineFromStack()
1025+
}
1026+
}
9961027

997-
if (this.getCurrentCline()) {
1028+
// Final check and cleanup
1029+
const currentCline = this.getCurrentCline()
1030+
if (currentCline && currentCline.taskId === cline.taskId) {
9981031
// 'abandoned' will prevent this Cline instance from affecting
9991032
// future Cline instances. This may happen if its hanging on a
10001033
// streaming request.
1001-
this.getCurrentCline()!.abandoned = true
1034+
currentCline.abandoned = true
10021035
}
10031036

1004-
// Clears task again, so we need to abortTask manually above.
1005-
await this.initClineWithHistoryItem({ ...historyItem, rootTask, parentTask })
1037+
// Only reinitialize with history if we have it
1038+
if (historyItem) {
1039+
await this.initClineWithHistoryItem({ ...historyItem, rootTask, parentTask })
1040+
} else {
1041+
// Just clear the task if no history available
1042+
await this.removeClineFromStack()
1043+
}
10061044
}
10071045

10081046
async updateCustomInstructions(instructions?: string) {

‎webview-ui/src/App.tsx‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
55
import { ExtensionMessage } from "@roo/ExtensionMessage"
66
import TranslationProvider from "./i18n/TranslationContext"
77
import { MarketplaceViewStateManager } from "./components/marketplace/MarketplaceViewStateManager"
8+
import { ErrorBoundary } from "./components/ErrorBoundary"
89

910
import { vscode } from "./utils/vscode"
1011
import { telemetryClient } from "./utils/TelemetryClient"
@@ -283,15 +284,17 @@ const App = () => {
283284
const queryClient = new QueryClient()
284285

285286
const AppWithProviders = () => (
286-
<ExtensionStateContextProvider>
287-
<TranslationProvider>
288-
<QueryClientProvider client={queryClient}>
289-
<TooltipProvider delayDuration={STANDARD_TOOLTIP_DELAY}>
290-
<App />
291-
</TooltipProvider>
292-
</QueryClientProvider>
293-
</TranslationProvider>
294-
</ExtensionStateContextProvider>
287+
<ErrorBoundary>
288+
<ExtensionStateContextProvider>
289+
<TranslationProvider>
290+
<QueryClientProvider client={queryClient}>
291+
<TooltipProvider delayDuration={STANDARD_TOOLTIP_DELAY}>
292+
<App />
293+
</TooltipProvider>
294+
</QueryClientProvider>
295+
</TranslationProvider>
296+
</ExtensionStateContextProvider>
297+
</ErrorBoundary>
295298
)
296299

297300
export default AppWithProviders

‎webview-ui/src/components/ErrorBoundary.tsx‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
import React, { Component, ErrorInfo, ReactNode } from "react"
2+
import { VSCodeButton } from "@vscode/webview-ui-toolkit/react"
3+
4+
interface Props {
5+
children: ReactNode
6+
fallback?: ReactNode
7+
}
8+
9+
interface State {
10+
hasError: boolean
11+
error: Error | null
12+
errorInfo: ErrorInfo | null
13+
}
14+
15+
export class ErrorBoundary extends Component<Props, State> {
16+
constructor(props: Props) {
17+
super(props)
18+
this.state = {
19+
hasError: false,
20+
error: null,
21+
errorInfo: null,
22+
}
23+
}
24+
25+
static getDerivedStateFromError(error: Error): State {
26+
// Update state so the next render will show the fallback UI
27+
return {
28+
hasError: true,
29+
error,
30+
errorInfo: null,
31+
}
32+
}
33+
34+
componentDidCatch(error: Error, errorInfo: ErrorInfo) {
35+
// Log error details for debugging
36+
console.error("ErrorBoundary caught an error:", error, errorInfo)
37+
38+
// Update state with error details
39+
this.setState({
40+
error,
41+
errorInfo,
42+
})
43+
}
44+
45+
handleReset = () => {
46+
this.setState({
47+
hasError: false,
48+
error: null,
49+
errorInfo: null,
50+
})
51+
}
52+
53+
render() {
54+
if (this.state.hasError) {
55+
// Custom fallback UI
56+
if (this.props.fallback) {
57+
return this.props.fallback
58+
}
59+
60+
// Default error UI
61+
return (
62+
<div className="error-boundary-container p-4 m-4 bg-vscode-inputValidation-errorBackground border border-vscode-inputValidation-errorBorder rounded">
63+
<h2 className="text-lg font-semibold mb-2 text-vscode-errorForeground">Something went wrong</h2>
64+
<p className="text-vscode-foreground mb-4">
65+
An unexpected error occurred. The application may continue to work, but some features might be
66+
affected.
67+
</p>
68+
69+
{this.state.error && (
70+
<details className="mb-4">
71+
<summary className="cursor-pointer text-vscode-textLink-foreground hover:underline">
72+
Error details
73+
</summary>
74+
<pre className="mt-2 p-2 bg-vscode-editor-background rounded text-xs overflow-auto">
75+
{this.state.error.message}
76+
{this.state.error.stack && "\n\n" + this.state.error.stack}
77+
</pre>
78+
</details>
79+
)}
80+
81+
<div className="flex gap-2">
82+
<VSCodeButton onClick={this.handleReset}>Try Again</VSCodeButton>
83+
<VSCodeButton appearance="secondary" onClick={() => window.location.reload()}>
84+
Reload Window
85+
</VSCodeButton>
86+
</div>
87+
</div>
88+
)
89+
}
90+
91+
return this.props.children
92+
}
93+
}
94+
95+
// Higher-order component for easier usage
96+
export function withErrorBoundary<P extends object>(
97+
Component: React.ComponentType<P>,
98+
fallback?: ReactNode,
99+
): React.ComponentType<P> {
100+
return (props: P) => (
101+
<ErrorBoundary fallback={fallback}>
102+
<Component {...props} />
103+
</ErrorBoundary>
104+
)
105+
}

‎webview-ui/src/components/chat/ChatRow.tsx‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ import { CommandExecutionError } from "./CommandExecutionError"
4646
import { AutoApprovedRequestLimitWarning } from "./AutoApprovedRequestLimitWarning"
4747
import { CondenseContextErrorRow, CondensingContextRow, ContextCondenseRow } from "./ContextCondenseRow"
4848
import CodebaseSearchResultsDisplay from "./CodebaseSearchResultsDisplay"
49+
import { LongRunningOperationIndicator } from "../common/LongRunningOperationIndicator"
4950

5051
interface ChatRowProps {
5152
message: ClineMessage
@@ -987,6 +988,9 @@ export const ChatRowContent = ({
987988
/>
988989
)
989990
case "api_req_started":
991+
// Show long-running operation indicator for streaming API requests
992+
const isApiStreaming = isLast && !cost && !apiRequestFailedMessage && !apiReqCancelReason
993+
990994
return (
991995
<>
992996
<div
@@ -1037,6 +1041,16 @@ export const ChatRowContent = ({
10371041
</>
10381042
)}
10391043

1044+
{isApiStreaming && (
1045+
<LongRunningOperationIndicator
1046+
isRunning={true}
1047+
elapsedTime={Date.now() - message.ts}
1048+
onCancel={() => {
1049+
vscode.postMessage({ type: "cancelTask" })
1050+
}}
1051+
/>
1052+
)}
1053+
10401054
{isExpanded && (
10411055
<div style={{ marginTop: "10px" }}>
10421056
<CodeAccordian

0 commit comments

Comments
 (0)