Skip to content

Commit 4f494bf

Browse files
committed
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
1 parent 8669ab9 commit 4f494bf

File tree

4 files changed

+21
-43
lines changed

4 files changed

+21
-43
lines changed

src/core/diff/strategies/__tests__/issue-4852-extended.spec.ts

Lines changed: 10 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,24 @@
11
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
22
import { parseXml } from "../../../../utils/xml"
33

4-
describe("Issue #4852 - addChild error investigation", () => {
4+
describe("Issue #4852 - fast-xml-parser error on complex XML", () => {
55
let consoleErrorSpy: any
66
let consoleWarnSpy: any
77

88
beforeEach(() => {
99
// Spy on console methods
1010
consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(() => {})
1111
consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {})
12-
13-
// Reset any global state
14-
if ((global as any).xml2js) {
15-
delete (global as any).xml2js
16-
}
1712
})
1813

1914
afterEach(() => {
2015
// Restore console methods
2116
consoleErrorSpy.mockRestore()
2217
consoleWarnSpy.mockRestore()
23-
24-
// Clean up global state
25-
if ((global as any).xml2js) {
26-
delete (global as any).xml2js
27-
}
2818
})
2919

30-
describe("External xml2js interference detection", () => {
31-
it("should detect xml2js presence and handle it silently", () => {
32-
// Simulate xml2js being loaded by another extension
33-
;(global as any).xml2js = {
34-
Parser: function () {
35-
this.parseString = function (xml: string, callback: (error: Error | null, result?: any) => void) {
36-
// Simulate xml2js behavior
37-
callback(new Error("Cannot read properties of undefined (reading 'addChild')"))
38-
}
39-
},
40-
}
41-
20+
describe("Fast-xml-parser error detection", () => {
21+
it("should detect parser errors and handle them gracefully", () => {
4222
const testXml = `<args>
4323
<file>
4424
<path>test.txt</path>
@@ -48,16 +28,15 @@ describe("Issue #4852 - addChild error investigation", () => {
4828
</file>
4929
</args>`
5030

51-
// Our parseXml should detect xml2js presence and handle it silently
52-
// (no console warnings as per code review requirements)
31+
// Our parseXml should handle parser errors gracefully
5332
let result
5433
try {
5534
result = parseXml(testXml)
5635
} catch (error) {
57-
// Expected to potentially fail
36+
// Expected to potentially fail on complex structures
5837
}
5938

60-
// Verify that no warnings were logged (as per code review requirements)
39+
// Verify that no warnings were logged
6140
expect(consoleWarnSpy).not.toHaveBeenCalled()
6241

6342
// The parser should still work correctly
@@ -76,7 +55,7 @@ describe("Issue #4852 - addChild error investigation", () => {
7655
</file>
7756
</args>`
7857

79-
// Test that our code can detect addChild errors
58+
// Test that our code can detect addChild errors from fast-xml-parser
8059
const mockError = new Error("Cannot read properties of undefined (reading 'addChild')")
8160

8261
// Check that the error message contains addChild
@@ -90,9 +69,9 @@ describe("Issue #4852 - addChild error investigation", () => {
9069
if (hasAddChild) {
9170
// This is what would be logged
9271
const expectedLog =
93-
'[XML_PARSER_ERROR] Detected "addChild" error - this is from xml2js, not fast-xml-parser'
94-
expect(expectedLog).toContain("xml2js")
72+
'[XML_PARSER_ERROR] Detected "addChild" error from fast-xml-parser on complex XML structure'
9573
expect(expectedLog).toContain("fast-xml-parser")
74+
expect(expectedLog).toContain("complex XML")
9675
}
9776
})
9877
})
@@ -319,7 +298,7 @@ function newFunction() {
319298
// Create a mock error with stack trace
320299
const mockError = new Error("Cannot read properties of undefined (reading 'addChild')")
321300
mockError.stack = `Error: Cannot read properties of undefined (reading 'addChild')
322-
at XMLParser.parse (node_modules/xml2js/lib/parser.js:123:45)
301+
at XMLParser.parse (node_modules/fast-xml-parser/src/xmlparser/XMLParser.js:123:45)
323302
at parseXml (src/utils/xml.ts:15:20)
324303
at multiApplyDiffTool (src/core/tools/multiApplyDiffTool.ts:111:30)`
325304

src/core/tools/__tests__/multiApplyDiffTool.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,11 +135,11 @@ describe("multiApplyDiffTool", () => {
135135
expect(() => parseXml(malformedXml)).toThrow("XML parsing failed")
136136
})
137137

138-
it("should detect and warn about xml2js addChild errors", async () => {
138+
it("should detect and warn about fast-xml-parser addChild errors", async () => {
139139
const consoleSpy = vi.spyOn(console, "error")
140140
const xml = `<args><file><path>test.txt</path><diff><content>test</content></diff></file></args>`
141141

142-
// Mock parseXml to throw an addChild error (simulating xml2js interference)
142+
// Mock parseXml to throw an addChild error (simulating fast-xml-parser error on complex XML)
143143
vi.mocked(parseXml).mockImplementation(() => {
144144
const error = new Error("Cannot read properties of undefined (reading 'addChild')")
145145
throw error

src/core/tools/multiApplyDiffTool.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ export async function applyDiffTool(
174174
1. The XML structure is malformed or incomplete
175175
2. Missing required <file>, <path>, or <diff> tags
176176
3. Invalid characters or encoding in the XML
177+
4. The XML structure is too complex for the parser
177178
178179
Expected structure:
179180
<args>
@@ -187,7 +188,7 @@ Expected structure:
187188
</args>
188189
189190
Original error: ${errorMessage}
190-
${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." : ""}`
191+
${hasAddChild ? "\n⚠️ NOTE: The parser encountered an error with complex XML structure. The fallback parser will be used if available." : ""}`
191192

192193
cline.consecutiveMistakeCount++
193194
cline.recordToolError("apply_diff")

src/utils/xml.ts

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,9 @@ import { XMLParser } from "fast-xml-parser"
33
/**
44
* Encapsulated XML parser with fallback mechanism
55
*
6-
* This dual-parser system handles interference from external XML parsers (like xml2js)
7-
* that may be loaded globally by other VSCode extensions. When the primary parser
8-
* (fast-xml-parser) fails due to external interference, it automatically falls back
9-
* to a regex-based parser.
6+
* This dual-parser system handles parsing errors that may occur when fast-xml-parser
7+
* encounters complex or deeply nested XML structures. When the primary parser
8+
* (fast-xml-parser) fails, it automatically falls back to a regex-based parser.
109
*/
1110
class XmlParserWithFallback {
1211
private readonly MAX_XML_SIZE = 10 * 1024 * 1024 // 10MB limit for fallback parser
@@ -128,16 +127,15 @@ class XmlParserWithFallback {
128127
}
129128

130129
// Try fallback parser for any parsing error
131-
// This handles both xml2js interference (addChild errors) and other parse failures
130+
// This handles parsing failures on complex XML structures
132131
try {
133132
const result = this.fallbackXmlParse(xmlString)
134133
return result
135134
} catch (fallbackError) {
136135
const fallbackErrorMsg = fallbackError instanceof Error ? fallbackError.message : "Unknown error"
137-
// Provide context about which error was from xml2js interference
138-
const isXml2jsError = errorMessage.includes("addChild")
139-
const errorContext = isXml2jsError
140-
? "XML parsing failed due to external parser interference (xml2js)."
136+
// Provide context about the parsing failure
137+
const errorContext = errorMessage.includes("addChild")
138+
? "XML parsing failed due to fast-xml-parser error on complex structure."
141139
: "XML parsing failed."
142140

143141
throw new Error(

0 commit comments

Comments
 (0)