Mocha: done() invoked with non-Error

[stevenvachon]

Using done() as a callback for other libraries that add their own argument data results in errors thrown.

something().then(done);

The only way around it is:

something().then( function(){ done() } );

It'd be great if Mocha accepted and ignored non-Error arguments to avoid the extra code.

[hallas]

In my opinion unnecessary magic behavior. If you have one case with this, wrap it like you show in your example, if you have multiple, create a helper function or overload the library function.

The strength of the err, res signature is that you can count on it, and libraries should conform to that standard when possible.

[stevenvachon]

Ok, fair enough.

[enthal]

But there has never been a requirement for Error class when the err object is not thrown as an exception. It's just a parameter. If mocha is throwing the object itself for flow control, then wrapping is mocha's concern, not the client code.

[Nate-Wilkins]

I agree with @enthal considering I'm trying to queue promises up and execute a done callback like so:

Q.all(promises).then(done);

But when done is supplied an array of undefined errors it throws because it's testing to see if the first argument is !undefined instead of checking if it's an error or rather is an array that contains errors.

Concluding: Does mocha currently support promises? And if not is it at some point?

[boneskull]

@enthal The first parameter to the err, res pattern is implied to be an error. So, I disagree that it's "just" a parameter; it has a type.

@Nate-Wilkins It occurs to me we need better documentation about how to do this, but yes, it supports promises.

// note lack of `done` parameter
it('should support promises', function() {
  return Q.all(promises)
    .then(function(res) { 
      // make some assertions
    });
});

[Nate-Wilkins]

@boneskull ingenious thanks!

[jtara1]

So what's the convention if the error isn't a type of Error? I'm using mocha to test my rest api and it returns some json object in all responses.

So I guess I'll just do this, but it feels weird

describe('have james, a user, leave the game', () => {
  it('should remove james from the game then delete the empty game', function(done) {
    game.leave(user.id)
      .then(json => {
        assert(true);
        done();
      })
      .catch(err => {
        done(new Error(err.message));
      })
  });
});

It'd probably be better to throw in a check for err instanceof Error then make a new error object if it's not an Error

This particular test func should be using the expect func and to and throw method from mocha instead of what's there.

[outsideris]

@jtara1 Why doesn't game.leave() throw Error instance? And how do you throw your error?

[jtara1]

@outsideris yea that method should be throwing objects of type Error. It's not right now. It's making http requests so errors don't come in the form of exceptions. They're just another request, but with a status code of 300-599 with a response body that holds info on the exception.

[boneskull]

@jtara1 boom might help you.

to and throw are (likely) Chai methods.

Also, try this sort of thing instead of combining Promises with done():

describe('have james, a user, leave the game', () => {
  it('should remove james from the game then delete the empty game', function() {
    return game.leave(user.id)
      .then(json => {
        assert(true);
      })
      .catch(err => {
        throw new Error(err.message);
      });
  });
});

[iqianxing]

I wrote some code to test this problem.

var request = require('request');

function httpCallback(data, callback) {

  if (!data.ip || !data.port) {
    getProvider(data, function (provider) {
      data.ip = provider.ip || "192.24.7.021";
      data.port = provider.port || 80;
      httpCallback(data, callback);
    });

    return;
  }

  var reqData = {
    body: data,
    json: true,
  }

  request.post('https://github.githubassets.com/apple-touch-icon-114x114.png', reqData, function (err, response, body) {
    if (callback && callback.length) {
      callback && callback(body);
    } else {
      callback && callback();
    }
  });

  function getProvider(opts, callback) {

    request.get("https://github.com/manifest.json", {
      json: true,
    }, function (error, response, body) {
      if (callback && callback.length) {
        var provider = body;
        callback && callback(provider);
      } else {
        callback && callback();
      }
    });
  }
}
if (module.parent && /mocha\.js/.test(module.parent.id)) {
  describe("# Mocha callback", function () {
    this.timeout(0);

    it("## Http callback", function (done) {
      var data = {
        ip: '192.24.7.021',
      }
      httpCallback(data, function(){
        done();
      });
    });

    it("## Http callback", function (done) {
      var data = {
        ip: '192.24.7.021',
      }
      httpCallback(data, done);
    });
  })
}

Run mocha command, the test output: