Skip to content

Conversation

WillieRuemmele
Copy link
Member

What does this PR do?

when the fast-xml-parser library throws an error due to invalid xml, we will parse that error, and add the file that caused the error.

given a custom labels xml file

<?xml version="1.0" encoding="UTF-8"?>
</CustomLabels xmlns="http://soap.sforce.com/2006/04/metadata">
  <labels>
    <fullName>force_app_Label_1</fullName>
    <language>en_US</language>
    <protected>true</protected>
    <shortDescription>force-app Label 1</shortDescription>
    <value>force-app</value>
  </labels>
</CustomLabels>

note the opening CustomLabels tag is actually a closing tag

What issues does this PR fix or reference?

#1261, @W-10119453@

Functionality Before

TypeError: Cannot read property 'addChild' of undefined

Functionality After

 Component conversion failed: error parsing dreamhouse-lwc/force-app/main/default/labels/CustomLabels.labels-meta.xml due to:
  message: Closing tag 'CustomLabels' can't have attributes or invalid starting..
  line: 2.
  code: InvalidTag.

@WillieRuemmele WillieRuemmele requested a review from a team December 28, 2021 23:33
@WillieRuemmele WillieRuemmele requested a review from a team as a code owner December 28, 2021 23:33
try {
return this.parse<T>(contents);
} catch (e) {
// only attempt validating once there's an error to avoid the performance hit of validating every file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

return this.parse<T>(contents);
} catch (e) {
// only attempt validating once there's an error to avoid the performance hit of validating every file
const validation = validate(contents);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think having them both use a sharedprivate parseAndValidateXML() would be more DRY.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had that for a while - but it was causing testing issues because parse was stubbed, but I got it working 👍

@mshanemc
Copy link
Contributor

mshanemc commented Jan 3, 2022

QA notes

this was pretty hard to QA (SDR doesn't care about most xml or parse it on a source:convert or deploy, including apexClasses or object/field stuff).

my VSCode had an xml extension that would remove stuff inside a self-closing tag. I finally got the new, improved error to happen via

<?xml version="1.0" encoding="UTF-8"?>
<CustomLabels xmlns="http://soap.sforce.com/2006/04/metadata">
  <labels>
    <fullName>force_app_Label_1</fullName>
    <language>en_US</language>
    <protected>true</protected>
    <shortDescription>force-app Label 1</shortDescription>
    <value>force-app</value>
  </labels>
  <CustomLabels>
    <SelfClos

✅ resulted in

../../plugin-source/bin/run force:source:convert -d force-app --outputdir mdapiOut
ERROR running force:source:convert:  Component conversion failed: error parsing /Users/shane.mclaughlin/eng/repros/mdapiDeployReport/force-app/main/default/labels/CustomLabels.labels-meta.xml due to:
  message: Invalid '[    "CustomLabels",    "CustomLabels",    "SelfClos"]' found.
  line: 1
  code: InvalidXml

which is way better than the original error.

It's still possibly to convert/deploy a lot of invalid xml (well, the metadata API throws the errors back).

🤷🏻‍♂️ It only throws the error on the first failure--you can't get all the xml errors in your project

@mshanemc mshanemc closed this Jan 3, 2022
@mshanemc mshanemc reopened this Jan 3, 2022
@mshanemc mshanemc merged commit 407af67 into main Jan 3, 2022
@mshanemc mshanemc deleted the wr/invalidXmlParse branch January 3, 2022 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants