Mangled exception handling with `chai-as-promised` leads to mocha silently exiting without reporting an error

[mmomtchev]

I don't know if this is supported or not, but it causes a very vicious silent error which causes the rest of the unit tests to not run and mocha to immediately exit with 0, reporting a global success - which means that automated unit testing doesn't report any error

It is somewhat random, depending on the Node version - I have a repo that can reproduce it with 100% probability on Node 14 on Ubuntu 20.04

Normally ds.root.arrays.getAsync('time') returns a Promise that resolves with a gdal.MDArray It is a promisified function of a function that calls a callback In this particular case getAsync() is throwing synchronously from C++ and then proceeding to invoke its callback which causes the exception to be propagated in a very unusual way going higher up the chain.

it('should have "getAsync()" method', (done) => {
  assert.eventually.instanceOf(ds.root.arrays.getAsync('time'), gdal.MDArray).then(done)
})

[mmomtchev]

I understood the underlying problem that was causing it, I will try to make a simple repro

[mmomtchev]

Ok, I leave it to you how to handle this - it is an error that is impossible to produce using only JS.

Basically getAsync is a C++ method with a callback. It encounters an error and then places an exception on the stack, but then doesn't handle correctly the error condition and calls the JS callback - which in this case is provided by Node.js promisify. The exception stays on the stack and it is triggered in a very unusual way and sometimes propagates up one level more than it should - depending on the exact V8 version.

[sukrosono]

Hi thanks for the issue, please include the repro. It might the internal team want to dive right away when this is not intended. Or this just specific to node v.14. You also could provide more detail about your environment, like chai and mocha version.

[mmomtchev]

@sukrosono Thanks for your reply, I can probably try to make a small Node.js addon that does this, all that is needed is a C++ function that accepts an (e, r) callback and does:

Nan::ThrowError("error");
callback->Call(2, { Nan::Null(), Nan::Null() }, async_resource);

and then use the above construct in chai/mocha with a done - mocha will immediately exit and report a success.

The problem is that, from mocha/chai standpoint, this is an abnormal V8 behavior since, by returning with a pending exception, the user program is able to find a way out of its enclosing try...catch block - which is shouldn't be possible according to the JS specifications.

[mmomtchev]

@sukrosono I was able to reproduce the problem without any C++ shenanigans

npm i mocha chai chai-as-promised

mocha.test.js

const chai = require('chai')
const assert = chai.assert
const chaiAsPromised = require('chai-as-promised')
chai.use(chaiAsPromised)

function asyncIterator() {
  let i = 0
  const count = 10

  return {
    next: () => Promise.resolve(count)
      .then((count) => {
        if (i++ < count) {
          return Promise.resolve(i).then((value) => ({ done: false, value }))
        }
        return { done: true, value: null }
      })
  }
}

it('asyncIterator', (done) => {
  const w = []
  const it = asyncIterator()
  const recurse = (r) => {
    if (r.done) {
      assert(w.length == 0) // This will throw
      done()
    } else {
      w.push(r.value)
      it.next().then(recurse)
    }
  }
  it.next().then(recurse)
})

.mocharc.json

{
  "timeout": 0
}

If you run this test, mocha will silently exit with a 0 exit code, indicating success, upon hitting the exception. If the file contain other tests, they will be silently ignored.

[mmomtchev]

@sukrosono I think I found the problem: https://github.com/nodejs/node/issues/29355