chai: expect.eql() returns true when deeply comparing different Error objects

I’ve run into an issue which I think is a bug when using deeply equals to compare two Error objects. Even though the message of the errors is different the expectation still returns that they’re equal.

Here’s an example using Mocha with expect().to.eql.

it('this test should fail', () => {
    const expectedError = new Error('An error');
    const error = new Error('A different error');
    expect(error).to.eql(expectedError);
});

Mocha passes this test even though I’d expect this to fail as the two errors have different message strings.

Is this a bug? I couldn’t find any similar problems when searching the issues.

Thanks, Marc

About this issue

Original URL
State: closed
Created 8 years ago
Comments: 18 (12 by maintainers)

Commits related to this issue

fix: Fix `handle error instance` test case Error objects are compared by reference now in deep-eql: https://github.com/chaijs/deep-eql See Chai issue #608: expect.eql() returns true when deeply comp... — committed to amobiz/mocha-cases by amobiz 6 years ago
fix: Fix `handle error instance` test case Error objects are compared by reference now in deep-eql: https://github.com/chaijs/deep-eql See Chai issue #608: expect.eql() returns true when deeply comp... — committed to amobiz/mocha-cases by amobiz 6 years ago

Most upvoted comments

Just FYI - this is already fixed in an outstanding PR on deep-eql; see https://github.com/chaijs/deep-eql/pull/14/files#diff-910eb6f57886ca16c136101fb1699231R268

keithamus on Jul 2, 2016

Hey @MarcL thanks for the issue.

This definitely looks like a bug - we want your assertion to fail. The reason why it’s not is a little… nuanced though:

deep-eql (our deep equality algo) checks all enumerable keys (effectively using `Object.keys) (see here)
Error, for some reason, sets Error.message to be non-enumerable by default (see here).

This kind of leaves us in a tricky position, because we could solve this with several different solutions, but each one has it’s own pitfalls:

We could switch to Object.getOwnPropertyNames, but this would be a breaking change for lots of people - potentially causing more issues. This also may cause issues because stack is a enumerable property and trying to do equality on this will always fail because the line/column numbers will be different for every error. I am sure we had a discussion about enumerability and deep equality somewhere but I can’t find the issue now.
We could specifically detect when you’re comparing Errors and specifically check for message - but this could be a slippery slope - how many types do we treat specially? How do we document to users that these behaviours are happening?
We could add a new assertion - something like expect(error).to.be.same.error(otherError) but of course this wont solve the fact that eql wont work and so its not really fixing your issue.

I’m not sure what the right solution is to this. As such - I’ve labeled it as more-discussion-needed

keithamus on Jul 27, 2017