-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: resolve gray screen of death issue during long chats #7166
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Enhanced ErrorBoundary with recovery mechanism and auto-retry - Added proper error state management and recovery buttons - Improved memory management in ChatView with better LRUCache settings - Added periodic memory cleanup for very long chat sessions - Implemented error handling for Virtuoso virtual scrolling component - Added fallback UI for scroll rendering failures - Optimized viewport settings to reduce memory usage - Added error boundaries around critical rendering components This fix addresses the gray screen issue that occurs during extended chat sessions by: 1. Preventing the error boundary from showing a gray overlay 2. Adding automatic recovery mechanisms 3. Improving memory management to prevent crashes 4. Providing user-friendly recovery options when errors occur
| componentStack: enhancedError.sourceMappedComponentStack || componentStack, | ||
| timestamp: Date.now(), | ||
| errorType: enhancedError.name, | ||
| errorCount: this.state.errorCount + 1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Auto-retry check relies on 'this.state.errorCount' immediately after incrementing it with setState. Since setState is asynchronous, the value may be stale. Consider capturing the updated error count (e.g. by using the updater’s returned value or storing it in a local variable) to ensure correct retry logic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed my own code and found 6 things wrong with it. Classic Tuesday.
| }) | ||
|
|
||
| // Auto-retry after 5 seconds if this is the first error | ||
| if (this.state.errorCount === 0 && !this.retryTimeoutId) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? The auto-retry logic checks if errorCount === 0 but we've already incremented it to 1 by this point (line 51-53). This means auto-retry will never trigger. Should this be checking errorCount === 1 instead?
| if (this.state.errorCount === 0 && !this.retryTimeoutId) { | |
| if (this.state.errorCount === 1 && !this.retryTimeoutId) { |
| {this.state.errorCount === 1 && ( | ||
| <div className="mb-4 p-3 bg-vscode-notifications-background border border-vscode-notifications-border rounded"> | ||
| <p className="text-vscode-notifications-foreground"> | ||
| The application will attempt to recover automatically in a few seconds... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These UI strings should use i18n translations for consistency with the rest of the application. Consider adding translation keys for:
- "The application will attempt to recover automatically in a few seconds..."
- "Try Again"
- "Reload Window"
- "(Click to expand)"
| }) | ||
| oldCache.clear() | ||
| } | ||
| }, 60000) // Run every minute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 60-second interval for memory cleanup might be too conservative for users actively experiencing the gray screen issue. Could we make this more aggressive (30 seconds) or configurable?
| }, 60000) // Run every minute | |
| }, 30000) // Run every 30 seconds for more aggressive cleanup |
| return false | ||
| // Limit processing for very large message arrays to prevent performance issues | ||
| const messagesToProcess = | ||
| modifiedMessages.length > 5000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider extracting these magic numbers to named constants for better maintainability:
- 5000 →
MAX_MESSAGES_TO_PROCESS - 150 →
CACHE_CLEANUP_THRESHOLD - 100 →
CACHE_RECENT_ENTRIES_TO_KEEP
This would make the memory management strategy more explicit and easier to tune.
| <div className="flex-1 flex flex-col items-center justify-center p-4"> | ||
| <div className="text-center max-w-md"> | ||
| <h3 className="text-lg font-semibold mb-2 text-vscode-editor-foreground"> | ||
| Unable to display messages |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These error messages are hardcoded in English. For consistency with the app's i18n support, consider adding translation keys for:
- "Unable to display messages"
- "The chat view encountered an issue while rendering messages. This can happen with very long conversations."
- "Try Again"
- "Reload Window"
- "Retry attempts:"
| } | ||
|
|
||
| // Simple error boundary for Virtuoso | ||
| class ErrorBoundary extends React.Component< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we consolidate the two error boundary implementations? The simple ErrorBoundary class (lines 2146-2173) duplicates logic from the main ErrorBoundary component. Consider creating a shared base implementation or using the main ErrorBoundary with different props.
|
I don't know why it's closed, this bug still happen with me |
Summary
This PR fixes the "Gray Screen of Death" issue (#7165) that occurs during long chat sessions in the Roo Code extension.
Problem
Users experience a gray screen that covers the entire chat window during extended conversations, making the extension unusable. The workaround of opening in a new window suggests this is a rendering/memory issue rather than a complete crash.
Solution
The fix implements multiple layers of protection and recovery:
1. Enhanced Error Boundary
2. Memory Management Improvements
3. Virtuoso Scrolling Optimizations
4. Error Handling
Testing
Related Issues
Fixes #7165
Impact
This should significantly improve the stability of the extension during long chat sessions and provide users with recovery options if issues do occur, eliminating the need to manually open in a new window.
Important
Fixes the 'Gray Screen of Death' in long chat sessions by enhancing error handling, memory management, and scrolling performance.
ErrorBoundaryinErrorBoundary.tsxwith auto-retry, error count tracking, and recovery buttons.ErrorBoundaryVirtuosoinChatView.tsxfor handling scrolling errors with retry logic.LRUCachecapacity and TTL inChatView.tsx.ChatView.tsx.ChatView.tsx.ChatView.tsx.ChatView.tsx.ErrorBoundary.tsx.This description was created by
for 60910c4. You can customize this summary. It will automatically update as commits are pushed.