diff --git a/src/core/diff/strategies/__tests__/issue-4852-extended.spec.ts b/src/core/diff/strategies/__tests__/issue-4852-extended.spec.ts index 4584ff3c8b..7fb356da0f 100644 --- a/src/core/diff/strategies/__tests__/issue-4852-extended.spec.ts +++ b/src/core/diff/strategies/__tests__/issue-4852-extended.spec.ts @@ -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 = ` test.txt @@ -48,16 +28,15 @@ describe("Issue #4852 - addChild error investigation", () => { ` - // 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", () => { ` - // 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)` diff --git a/src/core/tools/__tests__/multiApplyDiffTool.test.ts b/src/core/tools/__tests__/multiApplyDiffTool.test.ts index 5c8c6d899f..af654d37e9 100644 --- a/src/core/tools/__tests__/multiApplyDiffTool.test.ts +++ b/src/core/tools/__tests__/multiApplyDiffTool.test.ts @@ -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 = `test.txttest` - // 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 diff --git a/src/core/tools/multiApplyDiffTool.ts b/src/core/tools/multiApplyDiffTool.ts index 8494f3dd39..e119b2adbf 100644 --- a/src/core/tools/multiApplyDiffTool.ts +++ b/src/core/tools/multiApplyDiffTool.ts @@ -174,6 +174,7 @@ export async function applyDiffTool( 1. The XML structure is malformed or incomplete 2. Missing required , , or tags 3. Invalid characters or encoding in the XML +4. The XML structure is too complex for the parser Expected structure: @@ -187,7 +188,7 @@ Expected structure: 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") diff --git a/src/utils/xml.ts b/src/utils/xml.ts index de57a0df8b..79056b4588 100644 --- a/src/utils/xml.ts +++ b/src/utils/xml.ts @@ -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(