-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: prevent crashes when parsing Rust/Swift files (#6532) #6533
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -395,18 +395,41 @@ async function parseFile( | |
|
|
||
| try { | ||
| // Parse the file content into an Abstract Syntax Tree (AST) | ||
| const tree = parser.parse(fileContent) | ||
| // Add timeout to prevent hanging on large or complex files | ||
| const parseTimeout = new Promise<any>((resolve) => { | ||
| setTimeout(() => { | ||
| console.warn(`Parsing timeout for ${filePath} - file may be too large or complex`) | ||
| resolve(null) | ||
| }, 10000) // 10 second timeout | ||
| }) | ||
|
|
||
| const parsePromise = new Promise<any>((resolve) => { | ||
| try { | ||
| const tree = parser.parse(fileContent) | ||
| resolve(tree) | ||
| } catch (err) { | ||
| console.error(`Parser error for ${filePath}: ${err instanceof Error ? err.message : err}`) | ||
| resolve(null) | ||
| } | ||
| }) | ||
|
|
||
| const tree = await Promise.race([parsePromise, parseTimeout]) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this approach intentional? Using Promise.race here means the parser might continue running in the background even after the timeout resolves. This could lead to resource consumption. Consider using AbortController or a similar mechanism to actually cancel the parsing operation when the timeout occurs. |
||
|
|
||
| if (!tree || !tree.rootNode) { | ||
| console.warn(`Failed to parse ${filePath} - skipping`) | ||
| return null | ||
| } | ||
|
|
||
| // Apply the query to the AST and get the captures | ||
| const captures = tree ? query.captures(tree.rootNode) : [] | ||
| const captures = query.captures(tree.rootNode) || [] | ||
|
|
||
| // Split the file content into individual lines | ||
| const lines = fileContent.split("\n") | ||
|
|
||
| // Process the captures | ||
| return processCaptures(captures, lines, extLang) | ||
| } catch (error) { | ||
| console.log(`Error parsing file: ${error}\n`) | ||
| console.error(`Error parsing file ${filePath}: ${error instanceof Error ? error.message : error}`) | ||
| // Return null on parsing error to avoid showing error messages in the output | ||
| return null | ||
| } | ||
|
|
||
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.
Using
anytype here reduces type safety. Could we define proper types for the tree structure to maintain type safety throughout the Promise chain?