-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: resolve URL loading timeout issues in @ mentions #5160
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
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Problem URLs provided through @ mentions were failing to load with "Navigation timeout of 10000 ms exceeded" errors, preventing users from fetching web content for AI assistance. ## Solution - **Increased timeout limits**: Extended UrlContentFetcher timeout from 10s to 30s and BrowserSession timeout from 7s to 15s for better reliability with slow-loading websites - **Added fallback retry logic**: If networkidle2 fails, automatically retry with domcontentloaded only (20s timeout) to handle JavaScript-heavy sites - **Improved error handling**: Enhanced error messages with specific guidance for timeout, DNS, network, and HTTP status errors - **Enhanced browser compatibility**: Added Chrome launch arguments (--no-sandbox, --disable-web-security, etc.) and set proper viewport/headers to reduce blocking by websites ## Files Changed - `src/services/browser/UrlContentFetcher.ts`: Increased timeout, added retry logic, enhanced browser setup - `src/services/browser/BrowserSession.ts`: Increased navigation timeouts for consistency - `src/core/mentions/index.ts`: Improved error handling with user-friendly messages ## Impact Users can now successfully fetch content from slower websites and receive clearer error messages when issues occur, significantly improving the @ mention URL feature reliability. Fixes RooCodeInc#1430
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Fixes inconsistent error handling where errorMessage was safely extracted but error.message was still accessed directly in conditionals, which could cause runtime errors if error.message was undefined.
- Move timeout values to named constants for better maintainability - Increase timeouts: URL fetch (30s), fallback (20s), browser navigation (15s) - Improve error handling with cleaner code structure - Add internationalized error messages for all supported languages - Fix unsafe error.message access issue noted in PR review
- Removed --disable-web-security flag that bypasses CORS and CSP - Removed --no-sandbox flag that disables Chrome's security sandbox - Removed --disable-setuid-sandbox flag that weakens process isolation These flags were not necessary for resolving the URL loading timeout issues and introduced significant security risks. The timeout issues are better addressed through the increased timeouts and retry logic already in the PR.
- Fix error handling to check if error is an Error instance before accessing .message - Improve retry logic to only retry for timeout/network errors - Add comprehensive test coverage for URL fetching and error handling
- Fixed cheerio mock to return a callable function with html method - Fixed turndown mock to use a proper class constructor - Fixed error handling in parseMentions for non-Error objects - Added retry logic for unknown error types in UrlContentFetcher
- Use path.join() in test expectation to match the implementation - Fixes test failure on Windows due to backslash vs forward slash differences
…Fetcher - Replace manual error type checking with serialize-error package - Simplifies error handling logic and ensures all error types are handled consistently - Remove unnecessary console.warn as serialize-error handles edge cases - Update tests to match new behavior
… and AI integration
daniel-lxs
approved these changes
Jun 30, 2025
mrubens
approved these changes
Jun 30, 2025
hannesrudolph
pushed a commit
that referenced
this pull request
Jul 3, 2025
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> Co-authored-by: Daniel Riccio <[email protected]>
utarn
pushed a commit
to modelharbor/ModelHarbor-Agent
that referenced
this pull request
Jul 4, 2025
Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com> Co-authored-by: Daniel Riccio <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
lgtm
This PR has been approved by a maintainer
PR - Needs Review
size:XL
This PR changes 500-999 lines, ignoring generated files.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related GitHub Issue
Closes: #1430
Description
Problem
URLs provided through @ mentions were failing to load with "Navigation timeout of 10000 ms exceeded" errors, preventing users from fetching web content for AI assistance.
Solution
Files Changed
src/services/browser/UrlContentFetcher.ts: Increased timeout, added retry logic, enhanced browser setupsrc/services/browser/BrowserSession.ts: Increased navigation timeouts for consistencysrc/core/mentions/index.ts: Improved error handling with user-friendly messagesImpact
Users can now successfully fetch content from slower websites and receive clearer error messages when issues occur, significantly improving the @ mention URL feature reliability.
Test Procedure
Mention an URL and ask for a summary, for example. I tested with "Give me a summary of @#1430".
Before my changes, I got the same result as the issue, with the timeout error.
After the changes, it loaded successfully and the model was able to provide an answer. I tested with models that aren't able to access URL content too, to make sure they were getting the info from our fetch URL tool.
Pre-Submission Checklist
Screenshots / Videos
Documentation Updates
Additional Notes
Also added better error handling for common types of failed fetch URL errors.
Get in Touch
@MuriloFP
Important
Fix URL loading timeout issues by increasing timeouts, adding retry logic, and improving error handling in
UrlContentFetcher,BrowserSession, andindex.ts.UrlContentFetchertimeout from 10s to 30s andBrowserSessiontimeout from 7s to 15s.UrlContentFetcherto fallback todomcontentloadedifnetworkidle2fails.parseMentions()inindex.tswith specific messages for timeout, DNS, network, and HTTP errors.UrlContentFetcherto improve compatibility.UrlContentFetcherto reduce website blocking.UrlContentFetcher.ts: Increased timeout, added retry logic, enhanced browser setup.BrowserSession.ts: Increased navigation timeouts for consistency.index.ts: Improved error handling with user-friendly messages.This description was created by
for 40156b6. You can customize this summary. It will automatically update as commits are pushed.