mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-09-05 08:10:14 +00:00
fix: remove misleading xml2js references
- Updated PR description to clarify fast-xml-parser throws the error, not xml2js - Removed all xml2js interference references from code and comments - Updated tests to reflect the actual issue (fast-xml-parser error on complex XML) - The issue is about fast-xml-parser limitations, not external extension interference
This commit is contained in:
parent
8669ab9ff1
commit
4f494bf007
4 changed files with 21 additions and 43 deletions
|
|
@ -1,7 +1,7 @@
|
|||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
|
||||
import { parseXml } from "../../../../utils/xml"
|
||||
|
||||
describe("Issue #4852 - addChild error investigation", () => {
|
||||
describe("Issue #4852 - fast-xml-parser error on complex XML", () => {
|
||||
let consoleErrorSpy: any
|
||||
let consoleWarnSpy: any
|
||||
|
||||
|
|
@ -9,36 +9,16 @@ describe("Issue #4852 - addChild error investigation", () => {
|
|||
// Spy on console methods
|
||||
consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
|
||||
consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
|
||||
|
||||
// Reset any global state
|
||||
if ((global as any).xml2js) {
|
||||
delete (global as any).xml2js
|
||||
}
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
// Restore console methods
|
||||
consoleErrorSpy.mockRestore()
|
||||
consoleWarnSpy.mockRestore()
|
||||
|
||||
// Clean up global state
|
||||
if ((global as any).xml2js) {
|
||||
delete (global as any).xml2js
|
||||
}
|
||||
})
|
||||
|
||||
describe("External xml2js interference detection", () => {
|
||||
it("should detect xml2js presence and handle it silently", () => {
|
||||
// Simulate xml2js being loaded by another extension
|
||||
;(global as any).xml2js = {
|
||||
Parser: function () {
|
||||
this.parseString = function (xml: string, callback: (error: Error | null, result?: any) => void) {
|
||||
// Simulate xml2js behavior
|
||||
callback(new Error("Cannot read properties of undefined (reading 'addChild')"))
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
describe("Fast-xml-parser error detection", () => {
|
||||
it("should detect parser errors and handle them gracefully", () => {
|
||||
const testXml = `<args>
|
||||
<file>
|
||||
<path>test.txt</path>
|
||||
|
|
@ -48,16 +28,15 @@ describe("Issue #4852 - addChild error investigation", () => {
|
|||
</file>
|
||||
</args>`
|
||||
|
||||
// Our parseXml should detect xml2js presence and handle it silently
|
||||
// (no console warnings as per code review requirements)
|
||||
// Our parseXml should handle parser errors gracefully
|
||||
let result
|
||||
try {
|
||||
result = parseXml(testXml)
|
||||
} catch (error) {
|
||||
// Expected to potentially fail
|
||||
// Expected to potentially fail on complex structures
|
||||
}
|
||||
|
||||
// Verify that no warnings were logged (as per code review requirements)
|
||||
// Verify that no warnings were logged
|
||||
expect(consoleWarnSpy).not.toHaveBeenCalled()
|
||||
|
||||
// The parser should still work correctly
|
||||
|
|
@ -76,7 +55,7 @@ describe("Issue #4852 - addChild error investigation", () => {
|
|||
</file>
|
||||
</args>`
|
||||
|
||||
// Test that our code can detect addChild errors
|
||||
// Test that our code can detect addChild errors from fast-xml-parser
|
||||
const mockError = new Error("Cannot read properties of undefined (reading 'addChild')")
|
||||
|
||||
// Check that the error message contains addChild
|
||||
|
|
@ -90,9 +69,9 @@ describe("Issue #4852 - addChild error investigation", () => {
|
|||
if (hasAddChild) {
|
||||
// This is what would be logged
|
||||
const expectedLog =
|
||||
'[XML_PARSER_ERROR] Detected "addChild" error - this is from xml2js, not fast-xml-parser'
|
||||
expect(expectedLog).toContain("xml2js")
|
||||
'[XML_PARSER_ERROR] Detected "addChild" error from fast-xml-parser on complex XML structure'
|
||||
expect(expectedLog).toContain("fast-xml-parser")
|
||||
expect(expectedLog).toContain("complex XML")
|
||||
}
|
||||
})
|
||||
})
|
||||
|
|
@ -319,7 +298,7 @@ function newFunction() {
|
|||
// Create a mock error with stack trace
|
||||
const mockError = new Error("Cannot read properties of undefined (reading 'addChild')")
|
||||
mockError.stack = `Error: Cannot read properties of undefined (reading 'addChild')
|
||||
at XMLParser.parse (node_modules/xml2js/lib/parser.js:123:45)
|
||||
at XMLParser.parse (node_modules/fast-xml-parser/src/xmlparser/XMLParser.js:123:45)
|
||||
at parseXml (src/utils/xml.ts:15:20)
|
||||
at multiApplyDiffTool (src/core/tools/multiApplyDiffTool.ts:111:30)`
|
||||
|
||||
|
|
|
|||
|
|
@ -135,11 +135,11 @@ describe("multiApplyDiffTool", () => {
|
|||
expect(() => parseXml(malformedXml)).toThrow("XML parsing failed")
|
||||
})
|
||||
|
||||
it("should detect and warn about xml2js addChild errors", async () => {
|
||||
it("should detect and warn about fast-xml-parser addChild errors", async () => {
|
||||
const consoleSpy = vi.spyOn(console, "error")
|
||||
const xml = `<args><file><path>test.txt</path><diff><content>test</content></diff></file></args>`
|
||||
|
||||
// Mock parseXml to throw an addChild error (simulating xml2js interference)
|
||||
// Mock parseXml to throw an addChild error (simulating fast-xml-parser error on complex XML)
|
||||
vi.mocked(parseXml).mockImplementation(() => {
|
||||
const error = new Error("Cannot read properties of undefined (reading 'addChild')")
|
||||
throw error
|
||||
|
|
|
|||
|
|
@ -174,6 +174,7 @@ export async function applyDiffTool(
|
|||
1. The XML structure is malformed or incomplete
|
||||
2. Missing required <file>, <path>, or <diff> tags
|
||||
3. Invalid characters or encoding in the XML
|
||||
4. The XML structure is too complex for the parser
|
||||
|
||||
Expected structure:
|
||||
<args>
|
||||
|
|
@ -187,7 +188,7 @@ Expected structure:
|
|||
</args>
|
||||
|
||||
Original error: ${errorMessage}
|
||||
${hasAddChild ? "\n⚠️ NOTE: There may be a conflict with another extension. If you have XML-related extensions installed in VSCode, try disabling them and retrying." : ""}`
|
||||
${hasAddChild ? "\n⚠️ NOTE: The parser encountered an error with complex XML structure. The fallback parser will be used if available." : ""}`
|
||||
|
||||
cline.consecutiveMistakeCount++
|
||||
cline.recordToolError("apply_diff")
|
||||
|
|
|
|||
|
|
@ -3,10 +3,9 @@ import { XMLParser } from "fast-xml-parser"
|
|||
/**
|
||||
* Encapsulated XML parser with fallback mechanism
|
||||
*
|
||||
* This dual-parser system handles interference from external XML parsers (like xml2js)
|
||||
* that may be loaded globally by other VSCode extensions. When the primary parser
|
||||
* (fast-xml-parser) fails due to external interference, it automatically falls back
|
||||
* to a regex-based parser.
|
||||
* This dual-parser system handles parsing errors that may occur when fast-xml-parser
|
||||
* encounters complex or deeply nested XML structures. When the primary parser
|
||||
* (fast-xml-parser) fails, it automatically falls back to a regex-based parser.
|
||||
*/
|
||||
class XmlParserWithFallback {
|
||||
private readonly MAX_XML_SIZE = 10 * 1024 * 1024 // 10MB limit for fallback parser
|
||||
|
|
@ -128,16 +127,15 @@ class XmlParserWithFallback {
|
|||
}
|
||||
|
||||
// Try fallback parser for any parsing error
|
||||
// This handles both xml2js interference (addChild errors) and other parse failures
|
||||
// This handles parsing failures on complex XML structures
|
||||
try {
|
||||
const result = this.fallbackXmlParse(xmlString)
|
||||
return result
|
||||
} catch (fallbackError) {
|
||||
const fallbackErrorMsg = fallbackError instanceof Error ? fallbackError.message : "Unknown error"
|
||||
// Provide context about which error was from xml2js interference
|
||||
const isXml2jsError = errorMessage.includes("addChild")
|
||||
const errorContext = isXml2jsError
|
||||
? "XML parsing failed due to external parser interference (xml2js)."
|
||||
// Provide context about the parsing failure
|
||||
const errorContext = errorMessage.includes("addChild")
|
||||
? "XML parsing failed due to fast-xml-parser error on complex structure."
|
||||
: "XML parsing failed."
|
||||
|
||||
throw new Error(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue