async=true has different error handling than async=false

[samskeller]

I'm using node-xml2js via the express-xml-bodyparser module, which just allows an express web application to use the node-xml2js parser via some middleware for XML requests to convert them to JSON objects. I noticed that when I send an XML body that has two different syntax errors in it, a global error handling function within my express application will catch an error being raised during the parsing of XML, but after that error handling finishes, another error is thrown in xml2js which causes my express application to fail. An example of a bad XML body would be:

x<foo>test</foo>
 <bar>test</bar>
</data>

where there <data> root node is missing, and there's a non-whitespace character before what the parser considers to be the root node.

The error that crashes the application looks like this:

/var/core/app/node_modules/xml2js/lib/xml2js.js:515
          throw err;
          ^

Error: Text data outside of root node.
Line: 1
Column: 15
Char: i
    at error (/var/core/app/node_modules/xml2js/node_modules/sax/lib/sax.js:651:10)
    at strictFail (/var/core/app/node_modules/xml2js/node_modules/sax/lib/sax.js:677:7)
    at SAXParser.write (/var/core/app/node_modules/xml2js/node_modules/sax/lib/sax.js:1035:15)
    at Parser.exports.Parser.Parser.parseString (/var/core/app/node_modules/xml2js/lib/xml2js.js:508:31)
    at Parser.parseString (/var/core/app/node_modules/xml2js/lib/xml2js.js:7:59)
    at IncomingMessage.<anonymous> (/var/core/app/node_modules/express-xml-bodyparser/lib/types/xml.js:99:14)
    at emitNone (events.js:105:13)
    at IncomingMessage.emit (events.js:207:7)
    at endReadableNT (_stream_readable.js:1047:12)
    at _combinedTickCallback (internal/process/next_tick.js:102:11)
    at process._tickDomainCallback (internal/process/next_tick.js:198:9)

However, my application doesn't crash when I have the async=true option set -- everything works fine there. From digging into the node-xml2js code, it looks like the error handling with asynchronous parsing is different than it is when async=false. Here's the relevant code from Parser.prototype.parseString after an error is caught with this.saxParser.write(str).close();: https://github.com/Leonidas-from-XIV/node-xml2js/blob/1ab44ea837eff59305bd11f0e1a1e542e7c3e79f/lib/parser.js#L324-L330

However, the error handling when async=true, from Parser.prototype.processAsync looks like this: https://github.com/Leonidas-from-XIV/node-xml2js/blob/1ab44ea837eff59305bd11f0e1a1e542e7c3e79f/lib/parser.js#L85-L89

You can see that the this.saxParser.ended is not taken into account, and there's no throw err; line. I believe this is the reason why, when I have async=true in my configuration, I don't see the same hard error crashing my application as I do when I have async=false in my configuration.

Let me know if I can provide more information! Thanks!

[ghost]

Took me some time to figure out why my assert tests stalls, I has something todo with the error handling in this module.

When async === true, the asserts tests stalls, when 'false' the AssertionErrors are thrown as expected

Example: async: false, works as expected

const assert = require('assert')
const x2j = require('xml2js')
var parser = new x2j.Parser({
  async: false
})

parser.parseString('<xml><foo></foo></xml>', (e, o) => {
  assert.equal(true, false)
})
process.stdin.resume()

example asyn: true, assert test stalls

const assert = require('assert')
const x2j = require('xml2js')
var parser = new x2j.Parser({
  async: true
})

parser.parseString('<xml><foo></foo></xml>', (e, o) => {
  assert.equal(true, false)
})
process.stdin.resume()