Test fail without reason + #2906

[robininfo-edsx]

OS: Linux (Ubuntu)
Node: v8.1.4
NPM: 5.4.0
Mocha: 3.5.0

How to reproduce:

const fs = require('fs');

function write() {
  return new Promise(async (resolve) => {
    try {
      await new Promise((resolve, reject) =>
        fs.createWriteStream('/dev/full', {flags: 'a'})
          .write('lo', 'utf8', (err) => (err) ? reject(err) : resolve())
      );
      resolve();
    } catch (e) {
      resolve(e);
    }
  });
}
describe('Logger', () => {
  it('Try to log with /dev/full', async () => {
    console.log('!!!!');
    const err = await write();
    console.log(err.message);
    console.log('????');
  });
});

Output of mocha:

  Logger
!!!!
    1) Try to log with /dev/full
????
ENOSPC: no space left on device, write
    ✓ Try to log with /dev/full


  1 passing (12ms)
  1 failing

  1) Logger Try to log with /dev/full:
     Uncaught Error: ENOSPC: no space left on device, write
  




  1 passing (14ms)
  1 failing

  1) Logger Try to log with /dev/full:
     Uncaught Error: ENOSPC: no space left on device, write
  

[robininfo-edsx]

Strange thing if you remove describe like that

const fs = require('fs');

function write() {
  return new Promise(async (resolve) => {
    try {
      await new Promise((resolve, reject) =>
        fs.createWriteStream('/dev/full', {flags: 'a'})
          .write('lo', 'utf8', (err) => reject(err))
      );
      resolve();
    } catch (e) {
      resolve(e);
    }
  });
}
it('Try to log with /dev/full', async () => {
  console.log('!!!!');
  const err = await write();
  console.log('????');
  console.log(err.message);
});

You obtain a different result with an uncaught promisse rejection

!!!!
  1) Try to log with /dev/full
????
ENOSPC: no space left on device, write
(node:21060) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): TypeError: Cannot set property 'state' of undefined
(node:21060) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

  0 passing (13ms)
  1 failing

  1)  Try to log with /dev/full:
     Uncaught Error: ENOSPC: no space left on device, write
  

[robininfo-edsx]

For the last one (it not encapsulated) this fail there:

mocha/lib/runner.js

Line 576 in 2bc9c4d

test.state = 'passed';

Edit: It only fail when it is not in describe

[ScottFreeCode]

Hey @robininfo-edsx,

I was initially trying to figure out that nested promise construction and async/await with try-catch, but after looking twice at the output, would I be correct in saying this is just a more complex case of #2906? If so, I'd like to close this one and focus the discussion there -- I'll see if I can verify that adding describe turns the simpler example you left there into another instance of the issue, and possibly simplify it even further to get fs out of the picture altogether.

[robininfo-edsx]

@ScottFreeCode Seems to be that 👍

[robininfo-edsx]

Hmm finally not sure it's only that as my test fail but shouldn't fail 😢
Nothing in my code should fail there I don't see any point where the code can fail

[ScottFreeCode]

Maybe change from using the err parameter of the write callback to using the stream's on("error" handler? From https://nodejs.org/dist/latest-v8.x/docs/api/stream.html#stream_class_stream_writable:

If an error occurs, the callback may or may not be called with the error as its first argument. To reliably detect write errors, add a listener for the 'error' event.

I'm not sure what event emitters do when they have an error to emit and no "error" event handler has been attached, but if they throw an uncaught exception asynchronously in that case, that might be how the exception escapes your promise structure and ends up failing the Mocha test.

---

It may also help to simplify the promise/async code. There's a pretty good article on how to do that here: https://pouchdb.com/2015/05/18/we-have-a-problem-with-promises.html It's not aimed at your particular case and it predates async/await, but since async/await is just syntactic sugar for a function that returns a promise then chain understanding how to simplify the underlying promises can help simplify async/await patterns too. (There's also a misprint in one of the headings about catch and then(null, but if you read the section you'll get the correct info.)

[robininfo-edsx]

It's effectively that but well Node's documentation does mention that if an error event handler is not attached errors will be thrown (I was expecting that the function just failed quietly)
I think I may need to open an issue about this part of documentation on Node project
So yes this is exactly the same but with a very specific case of synchronous error that can't be handled in a try/catch.
After that it's exactly the same bug as #2906