mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-07 08:26:51 +00:00
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
This commit is contained in:
parent
b1bc085aa6
commit
311ea08d0e
6 changed files with 334 additions and 41 deletions
|
|
@ -1072,6 +1072,13 @@ export class Task extends EventEmitter<ClineEvents> {
|
|||
} catch (error) {
|
||||
console.error("Error reverting diff changes:", error)
|
||||
}
|
||||
|
||||
// Reset all streaming-related state
|
||||
this.isStreaming = false
|
||||
this.isWaitingForFirstChunk = false
|
||||
this.didCompleteReadingStream = true
|
||||
this.presentAssistantMessageLocked = false
|
||||
this.presentAssistantMessageHasPendingUpdates = false
|
||||
}
|
||||
|
||||
public async abortTask(isAbandoned = false) {
|
||||
|
|
@ -1091,6 +1098,10 @@ export class Task extends EventEmitter<ClineEvents> {
|
|||
console.error(`Error during task ${this.taskId}.${this.instanceId} disposal:`, error)
|
||||
// Don't rethrow - we want abort to always succeed
|
||||
}
|
||||
|
||||
// Ensure streaming state is reset even if dispose fails
|
||||
this.isStreaming = false
|
||||
this.isWaitingForFirstChunk = false
|
||||
// Save the countdown message in the automatic retry or other content.
|
||||
try {
|
||||
// Save the countdown message in the automatic retry or other content.
|
||||
|
|
@ -1359,6 +1370,24 @@ export class Task extends EventEmitter<ClineEvents> {
|
|||
let reasoningMessage = ""
|
||||
this.isStreaming = true
|
||||
|
||||
// Add a timeout for the entire streaming operation
|
||||
const streamTimeout = setTimeout(
|
||||
async () => {
|
||||
if (this.isStreaming && !this.abort) {
|
||||
console.error(`Stream timeout for task ${this.taskId} - aborting stream`)
|
||||
await this.say(
|
||||
"error",
|
||||
"The API request timed out. This might be due to network issues or server problems. The task will be cancelled.",
|
||||
undefined,
|
||||
false,
|
||||
)
|
||||
// Trigger abort to clean up properly
|
||||
this.abort = true
|
||||
}
|
||||
},
|
||||
5 * 60 * 1000,
|
||||
) // 5 minute timeout
|
||||
|
||||
try {
|
||||
for await (const chunk of stream) {
|
||||
if (!chunk) {
|
||||
|
|
@ -1451,19 +1480,25 @@ export class Task extends EventEmitter<ClineEvents> {
|
|||
? undefined
|
||||
: (error.message ?? JSON.stringify(serializeError(error), null, 2))
|
||||
|
||||
// Now call abortTask after determining the cancel reason
|
||||
await this.abortTask()
|
||||
try {
|
||||
// Now call abortTask after determining the cancel reason
|
||||
await this.abortTask()
|
||||
|
||||
await abortStream(cancelReason, streamingFailedMessage)
|
||||
await abortStream(cancelReason, streamingFailedMessage)
|
||||
|
||||
const history = await provider?.getTaskWithId(this.taskId)
|
||||
const history = await provider?.getTaskWithId(this.taskId)
|
||||
|
||||
if (history) {
|
||||
await provider?.initClineWithHistoryItem(history.historyItem)
|
||||
if (history) {
|
||||
await provider?.initClineWithHistoryItem(history.historyItem)
|
||||
}
|
||||
} catch (abortError) {
|
||||
console.error(`Error during stream failure recovery for task ${this.taskId}:`, abortError)
|
||||
// Continue execution even if abort fails to prevent hanging
|
||||
}
|
||||
}
|
||||
} finally {
|
||||
this.isStreaming = false
|
||||
clearTimeout(streamTimeout)
|
||||
}
|
||||
|
||||
if (inputTokens > 0 || outputTokens > 0 || cacheWriteTokens > 0 || cacheReadTokens > 0) {
|
||||
|
|
|
|||
|
|
@ -192,13 +192,20 @@ export class ClineProvider
|
|||
console.log(`[subtasks] removing task ${cline.taskId}.${cline.instanceId} from stack`)
|
||||
|
||||
try {
|
||||
// Abort the running task and set isAbandoned to true so
|
||||
// all running promises will exit as well.
|
||||
await cline.abortTask(true)
|
||||
// Set a timeout for the abort operation
|
||||
const abortPromise = cline.abortTask(true)
|
||||
const timeoutPromise = new Promise((_, reject) =>
|
||||
setTimeout(() => reject(new Error("Abort timeout")), 3000),
|
||||
)
|
||||
|
||||
// Race between abort completion and timeout
|
||||
await Promise.race([abortPromise, timeoutPromise])
|
||||
} catch (e) {
|
||||
this.log(
|
||||
`[subtasks] encountered error while aborting task ${cline.taskId}.${cline.instanceId}: ${e.message}`,
|
||||
)
|
||||
// Force abandon the task if abort fails
|
||||
cline.abandoned = true
|
||||
}
|
||||
|
||||
// Make sure no reference kept, once promises end it will be
|
||||
|
|
@ -971,38 +978,69 @@ export class ClineProvider
|
|||
|
||||
console.log(`[subtasks] cancelling task ${cline.taskId}.${cline.instanceId}`)
|
||||
|
||||
const { historyItem } = await this.getTaskWithId(cline.taskId)
|
||||
// Preserve parent and root task information for history item.
|
||||
const rootTask = cline.rootTask
|
||||
const parentTask = cline.parentTask
|
||||
let historyItem: HistoryItem | undefined
|
||||
let rootTask: Task | undefined
|
||||
let parentTask: Task | undefined
|
||||
|
||||
try {
|
||||
const taskData = await this.getTaskWithId(cline.taskId)
|
||||
historyItem = taskData.historyItem
|
||||
// Preserve parent and root task information for history item.
|
||||
rootTask = cline.rootTask
|
||||
parentTask = cline.parentTask
|
||||
} catch (error) {
|
||||
console.error(`Failed to get task history for ${cline.taskId}:`, error)
|
||||
// Continue with cancellation even if we can't get history
|
||||
}
|
||||
|
||||
// Start the abort process
|
||||
cline.abortTask()
|
||||
|
||||
await pWaitFor(
|
||||
() =>
|
||||
this.getCurrentCline()! === undefined ||
|
||||
this.getCurrentCline()!.isStreaming === false ||
|
||||
this.getCurrentCline()!.didFinishAbortingStream ||
|
||||
// If only the first chunk is processed, then there's no
|
||||
// need to wait for graceful abort (closes edits, browser,
|
||||
// etc).
|
||||
this.getCurrentCline()!.isWaitingForFirstChunk,
|
||||
{
|
||||
timeout: 3_000,
|
||||
},
|
||||
).catch(() => {
|
||||
console.error("Failed to abort task")
|
||||
})
|
||||
// Wait for abort to complete with a timeout
|
||||
try {
|
||||
await pWaitFor(
|
||||
() => {
|
||||
const currentCline = this.getCurrentCline()
|
||||
return (
|
||||
!currentCline ||
|
||||
currentCline.taskId !== cline.taskId ||
|
||||
!currentCline.isStreaming ||
|
||||
currentCline.didFinishAbortingStream ||
|
||||
currentCline.isWaitingForFirstChunk
|
||||
)
|
||||
},
|
||||
{
|
||||
timeout: 5_000, // Increased timeout for better reliability
|
||||
interval: 100,
|
||||
},
|
||||
)
|
||||
} catch (error) {
|
||||
console.error("Timeout waiting for task abort, forcing cleanup")
|
||||
// Force cleanup if timeout occurs
|
||||
const currentCline = this.getCurrentCline()
|
||||
if (currentCline && currentCline.taskId === cline.taskId) {
|
||||
currentCline.abandoned = true
|
||||
// Force remove from stack
|
||||
await this.removeClineFromStack()
|
||||
}
|
||||
}
|
||||
|
||||
if (this.getCurrentCline()) {
|
||||
// Final check and cleanup
|
||||
const currentCline = this.getCurrentCline()
|
||||
if (currentCline && currentCline.taskId === cline.taskId) {
|
||||
// 'abandoned' will prevent this Cline instance from affecting
|
||||
// future Cline instances. This may happen if its hanging on a
|
||||
// streaming request.
|
||||
this.getCurrentCline()!.abandoned = true
|
||||
currentCline.abandoned = true
|
||||
}
|
||||
|
||||
// Clears task again, so we need to abortTask manually above.
|
||||
await this.initClineWithHistoryItem({ ...historyItem, rootTask, parentTask })
|
||||
// Only reinitialize with history if we have it
|
||||
if (historyItem) {
|
||||
await this.initClineWithHistoryItem({ ...historyItem, rootTask, parentTask })
|
||||
} else {
|
||||
// Just clear the task if no history available
|
||||
await this.removeClineFromStack()
|
||||
}
|
||||
}
|
||||
|
||||
async updateCustomInstructions(instructions?: string) {
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
|
|||
import { ExtensionMessage } from "@roo/ExtensionMessage"
|
||||
import TranslationProvider from "./i18n/TranslationContext"
|
||||
import { MarketplaceViewStateManager } from "./components/marketplace/MarketplaceViewStateManager"
|
||||
import { ErrorBoundary } from "./components/ErrorBoundary"
|
||||
|
||||
import { vscode } from "./utils/vscode"
|
||||
import { telemetryClient } from "./utils/TelemetryClient"
|
||||
|
|
@ -283,15 +284,17 @@ const App = () => {
|
|||
const queryClient = new QueryClient()
|
||||
|
||||
const AppWithProviders = () => (
|
||||
<ExtensionStateContextProvider>
|
||||
<TranslationProvider>
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<TooltipProvider delayDuration={STANDARD_TOOLTIP_DELAY}>
|
||||
<App />
|
||||
</TooltipProvider>
|
||||
</QueryClientProvider>
|
||||
</TranslationProvider>
|
||||
</ExtensionStateContextProvider>
|
||||
<ErrorBoundary>
|
||||
<ExtensionStateContextProvider>
|
||||
<TranslationProvider>
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<TooltipProvider delayDuration={STANDARD_TOOLTIP_DELAY}>
|
||||
<App />
|
||||
</TooltipProvider>
|
||||
</QueryClientProvider>
|
||||
</TranslationProvider>
|
||||
</ExtensionStateContextProvider>
|
||||
</ErrorBoundary>
|
||||
)
|
||||
|
||||
export default AppWithProviders
|
||||
|
|
|
|||
105
webview-ui/src/components/ErrorBoundary.tsx
Normal file
105
webview-ui/src/components/ErrorBoundary.tsx
Normal file
|
|
@ -0,0 +1,105 @@
|
|||
import React, { Component, ErrorInfo, ReactNode } from "react"
|
||||
import { VSCodeButton } from "@vscode/webview-ui-toolkit/react"
|
||||
|
||||
interface Props {
|
||||
children: ReactNode
|
||||
fallback?: ReactNode
|
||||
}
|
||||
|
||||
interface State {
|
||||
hasError: boolean
|
||||
error: Error | null
|
||||
errorInfo: ErrorInfo | null
|
||||
}
|
||||
|
||||
export class ErrorBoundary extends Component<Props, State> {
|
||||
constructor(props: Props) {
|
||||
super(props)
|
||||
this.state = {
|
||||
hasError: false,
|
||||
error: null,
|
||||
errorInfo: null,
|
||||
}
|
||||
}
|
||||
|
||||
static getDerivedStateFromError(error: Error): State {
|
||||
// Update state so the next render will show the fallback UI
|
||||
return {
|
||||
hasError: true,
|
||||
error,
|
||||
errorInfo: null,
|
||||
}
|
||||
}
|
||||
|
||||
componentDidCatch(error: Error, errorInfo: ErrorInfo) {
|
||||
// Log error details for debugging
|
||||
console.error("ErrorBoundary caught an error:", error, errorInfo)
|
||||
|
||||
// Update state with error details
|
||||
this.setState({
|
||||
error,
|
||||
errorInfo,
|
||||
})
|
||||
}
|
||||
|
||||
handleReset = () => {
|
||||
this.setState({
|
||||
hasError: false,
|
||||
error: null,
|
||||
errorInfo: null,
|
||||
})
|
||||
}
|
||||
|
||||
render() {
|
||||
if (this.state.hasError) {
|
||||
// Custom fallback UI
|
||||
if (this.props.fallback) {
|
||||
return this.props.fallback
|
||||
}
|
||||
|
||||
// Default error UI
|
||||
return (
|
||||
<div className="error-boundary-container p-4 m-4 bg-vscode-inputValidation-errorBackground border border-vscode-inputValidation-errorBorder rounded">
|
||||
<h2 className="text-lg font-semibold mb-2 text-vscode-errorForeground">Something went wrong</h2>
|
||||
<p className="text-vscode-foreground mb-4">
|
||||
An unexpected error occurred. The application may continue to work, but some features might be
|
||||
affected.
|
||||
</p>
|
||||
|
||||
{this.state.error && (
|
||||
<details className="mb-4">
|
||||
<summary className="cursor-pointer text-vscode-textLink-foreground hover:underline">
|
||||
Error details
|
||||
</summary>
|
||||
<pre className="mt-2 p-2 bg-vscode-editor-background rounded text-xs overflow-auto">
|
||||
{this.state.error.message}
|
||||
{this.state.error.stack && "\n\n" + this.state.error.stack}
|
||||
</pre>
|
||||
</details>
|
||||
)}
|
||||
|
||||
<div className="flex gap-2">
|
||||
<VSCodeButton onClick={this.handleReset}>Try Again</VSCodeButton>
|
||||
<VSCodeButton appearance="secondary" onClick={() => window.location.reload()}>
|
||||
Reload Window
|
||||
</VSCodeButton>
|
||||
</div>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
return this.props.children
|
||||
}
|
||||
}
|
||||
|
||||
// Higher-order component for easier usage
|
||||
export function withErrorBoundary<P extends object>(
|
||||
Component: React.ComponentType<P>,
|
||||
fallback?: ReactNode,
|
||||
): React.ComponentType<P> {
|
||||
return (props: P) => (
|
||||
<ErrorBoundary fallback={fallback}>
|
||||
<Component {...props} />
|
||||
</ErrorBoundary>
|
||||
)
|
||||
}
|
||||
|
|
@ -46,6 +46,7 @@ import { CommandExecutionError } from "./CommandExecutionError"
|
|||
import { AutoApprovedRequestLimitWarning } from "./AutoApprovedRequestLimitWarning"
|
||||
import { CondenseContextErrorRow, CondensingContextRow, ContextCondenseRow } from "./ContextCondenseRow"
|
||||
import CodebaseSearchResultsDisplay from "./CodebaseSearchResultsDisplay"
|
||||
import { LongRunningOperationIndicator } from "../common/LongRunningOperationIndicator"
|
||||
|
||||
interface ChatRowProps {
|
||||
message: ClineMessage
|
||||
|
|
@ -987,6 +988,9 @@ export const ChatRowContent = ({
|
|||
/>
|
||||
)
|
||||
case "api_req_started":
|
||||
// Show long-running operation indicator for streaming API requests
|
||||
const isApiStreaming = isLast && !cost && !apiRequestFailedMessage && !apiReqCancelReason
|
||||
|
||||
return (
|
||||
<>
|
||||
<div
|
||||
|
|
@ -1037,6 +1041,16 @@ export const ChatRowContent = ({
|
|||
</>
|
||||
)}
|
||||
|
||||
{isApiStreaming && (
|
||||
<LongRunningOperationIndicator
|
||||
isRunning={true}
|
||||
elapsedTime={Date.now() - message.ts}
|
||||
onCancel={() => {
|
||||
vscode.postMessage({ type: "cancelTask" })
|
||||
}}
|
||||
/>
|
||||
)}
|
||||
|
||||
{isExpanded && (
|
||||
<div style={{ marginTop: "10px" }}>
|
||||
<CodeAccordian
|
||||
|
|
|
|||
|
|
@ -0,0 +1,98 @@
|
|||
import React, { useEffect, useState } from "react"
|
||||
import { VSCodeProgressRing } from "@vscode/webview-ui-toolkit/react"
|
||||
|
||||
interface LongRunningOperationIndicatorProps {
|
||||
isRunning: boolean
|
||||
operationName?: string
|
||||
elapsedTime?: number
|
||||
estimatedTime?: number
|
||||
showCancel?: boolean
|
||||
onCancel?: () => void
|
||||
}
|
||||
|
||||
export const LongRunningOperationIndicator: React.FC<LongRunningOperationIndicatorProps> = ({
|
||||
isRunning,
|
||||
operationName = "Operation",
|
||||
elapsedTime,
|
||||
estimatedTime,
|
||||
showCancel = true,
|
||||
onCancel,
|
||||
}) => {
|
||||
const [seconds, setSeconds] = useState(0)
|
||||
|
||||
useEffect(() => {
|
||||
if (!isRunning) {
|
||||
setSeconds(0)
|
||||
return
|
||||
}
|
||||
|
||||
const interval = setInterval(() => {
|
||||
setSeconds((prev) => prev + 1)
|
||||
}, 1000)
|
||||
|
||||
return () => clearInterval(interval)
|
||||
}, [isRunning])
|
||||
|
||||
if (!isRunning) {
|
||||
return null
|
||||
}
|
||||
|
||||
const formatTime = (totalSeconds: number): string => {
|
||||
const minutes = Math.floor(totalSeconds / 60)
|
||||
const secs = totalSeconds % 60
|
||||
if (minutes > 0) {
|
||||
return `${minutes}m ${secs}s`
|
||||
}
|
||||
return `${secs}s`
|
||||
}
|
||||
|
||||
const displayTime = elapsedTime !== undefined ? elapsedTime : seconds
|
||||
|
||||
return (
|
||||
<div className="long-running-operation-indicator flex items-center gap-3 p-3 my-2 bg-vscode-editor-background border border-vscode-panel-border rounded">
|
||||
<VSCodeProgressRing />
|
||||
<div className="flex-1">
|
||||
<div className="text-vscode-foreground font-medium">{operationName} in progress...</div>
|
||||
<div className="text-vscode-descriptionForeground text-sm mt-1">
|
||||
Elapsed: {formatTime(displayTime)}
|
||||
{estimatedTime && <span className="ml-3">Estimated: {formatTime(estimatedTime)}</span>}
|
||||
</div>
|
||||
</div>
|
||||
{showCancel && onCancel && (
|
||||
<button
|
||||
onClick={onCancel}
|
||||
className="px-3 py-1 text-sm bg-vscode-button-background text-vscode-button-foreground hover:bg-vscode-button-hoverBackground rounded"
|
||||
aria-label="Cancel operation">
|
||||
Cancel
|
||||
</button>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
}
|
||||
|
||||
// Hook to track long-running operations
|
||||
export const useLongRunningOperation = (threshold: number = 3000) => {
|
||||
const [isLongRunning, setIsLongRunning] = useState(false)
|
||||
const [startTime, setStartTime] = useState<number | null>(null)
|
||||
|
||||
const start = () => {
|
||||
setStartTime(Date.now())
|
||||
setTimeout(() => {
|
||||
setIsLongRunning(true)
|
||||
}, threshold)
|
||||
}
|
||||
|
||||
const stop = () => {
|
||||
setIsLongRunning(false)
|
||||
setStartTime(null)
|
||||
}
|
||||
|
||||
const elapsedTime = startTime ? Math.floor((Date.now() - startTime) / 1000) : 0
|
||||
|
||||
return {
|
||||
isLongRunning,
|
||||
elapsedTime,
|
||||
start,
|
||||
stop,
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue