Uncaught error when tag contains multiple invalid attributes

[knolleary]

If an XML tag contains more than one invalid attribute, such as <one two three>, and parseString is used in async mode, it throws an uncaught error. If the tag contains only one invalid attribute , it doesn't throw the error.

This appears to have been broken by this commit: b4ccc87

Here is a testcase:

var xml2js = require('xml2js');

var parseString = xml2js.parseString;
parseString('<invalid xml string>', {async:true}, function (err, result) {
    if (err) {
        console.log("Caught",err);
    }
    else {
        console.log('ok');
    }
});

Expected result - the callback receives the error:

Caught [Error: Attribute without value
Line: 0
Column: 14
Char: s]

Actual result - the callback receives the error and an uncaught error is throw:

Caught [Error: Attribute without value
Line: 0
Column: 14
Char: s]

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: Attribute without value
Line: 0
Column: 20
Char: >
    at error (/private/tmp/a/node-red/node_modules/xml2js/node_modules/sax/lib/sax.js:652:8)
    at strictFail (/private/tmp/a/node-red/node_modules/xml2js/node_modules/sax/lib/sax.js:675:22)
    at Object.write (/private/tmp/a/node-red/node_modules/xml2js/node_modules/sax/lib/sax.js:1224:11)
    at Parser.exports.Parser.Parser.processAsync (/private/tmp/a/node-red/node_modules/xml2js/lib/xml2js.js:256:41)
    at Object._onImmediate (/private/tmp/a/node-red/node_modules/xml2js/lib/xml2js.js:7:59)
    at processImmediate [as _immediateCallback] (timers.js:354:15)

[tflanagan]

As you discovered @knolleary, this only happens when there is more than one error in any chunk of XML being parsed. As this chunk has two Attribute without value errors, it emits two errors.

If you had an XML string of <one two three four>, it would emit three errors, the second uncaught one would prevent you from seeing it, but that's what would happen.

Commenting out this line of code seems to solve the issue of not catching the subsequent errors. But they are still being emitted.

This change passes all tests. I wonder how this would affect things in the wild.

https://github.com/Leonidas-from-XIV/node-xml2js/blob/master/src/xml2js.coffee#L383

[Leonidas-from-XIV]

This change passes all tests. I wonder how this would affect things in the wild.

Good question. I've only added it as people like to reuse the parser object for multiple parses (which I recommend against in any case), but I would be open to removing it and seeing how many issues are reported.

[tflanagan]

fixed in #240