Two different Errors are considered equal #47

Activity

1. khirayama commented on Apr 18, 2018

   

   khirayama

   on Apr 18, 2018

   Last edited by khirayama

   Contributor

   More actions

   @agirorn #58
   I'm not a committer or a contributor but I tried to implement that. And I felt it is difficult 😢

   I want to know your opinion 😄
   "When you get some errors on same file and same line, but these have different messages."
   Or
   "When you get some errors on different file and different line, but these have same messages."

   Thank you 👍
2. khirayama commented on Apr 18, 2018

   

   khirayama

   on Apr 18, 2018

   Contributor

   More actions

   FYI @RusinovAnton
3. agirorn commented on Oct 29, 2018

   

   agirorn

   on Oct 29, 2018

   ContributorAuthor

   More actions

   I think that errors are only equal in the following conditions:

   1. If they are the same object

   2. If they both of them have the error.code property and the code is equal on both of them and they both have the same error.message

   3. If they neither of them has the error.code property and both of them have the same error.message including both massages being undefined or null
4. ljharb commented on Jul 29, 2019

   

   ljharb

   on Jul 29, 2019

   Member

   More actions

   More that they need to have the same properties, enumerable or not - which includes .stack.
5. agirorn commented on Jul 30, 2019

   

   agirorn

   on Jul 30, 2019

   ContributorAuthor

   More actions

   @ljharb you might be on to something here. I just did some minor experiments and have found out that node 12 (Object.keys) and object-keys do not return all the keys for all properties on an instance of the Error class.

   > Object.keys(new Error('a'))
   []
   
   > const keys = require('object-keys');
   > keys(new Error('a'))
                                                                                  │[]
   > class T { constructor() { this.key = 'VALUE'; }}
   > Object.keys(new T())
   [ 'key' ]

   So maybe it is just a javascript issue and then the question is should this model try to fix this strange behavior of javascript or should the user of this module just feed in the correct object that can be checked for equality?

   const equal = require('deep-equal');
   const a = new Error('a')
   const b = new Error('a')
   
   const A =  {
     message: a.message,
     className: a.constructor.name
   };
   
   const B = {
     message: b.message,
     className: b.constructor.name
   };
   
   console.dir([
     A,
     B,
     equal(A, B, { strict: true })
   ])
   // [
   //   { message: 'a', className: 'Error' },
   //   { message: 'b', className: 'Error' },
   //  false
   // ]

   As far as I'm concerned this issue can be closed.
6. ljharb commented on Jul 30, 2019

   

   ljharb

   on Jul 30, 2019

   Member

   More actions

   That's because Object.keys returns only enumerable string properties; you need Object.getOwnPropertyNames + Object.getOwnPropertySymbols, or Reflect.ownKeys, to get all including non-enumerables.

   The real question is, what does assert.deepEqual do? If it treats Errors as equal like this issue requests, so should this package - if not, it also should not.
7. agirorn commented on Jul 31, 2019

   

   agirorn

   on Jul 31, 2019

   ContributorAuthor

   More actions

   node 12 assert.deepEqual treats them ass being different.

   > assert.deepEqual(new Error('a'), new Error('a'))
   undefined
   > assert.deepEqual(new Error('a'), new Error('b'))
   Thrown:
   AssertionError [ERR_ASSERTION]: Expected values to be loosely deep-equal:
   
   [Error: a]
   
   should loosely deep-equal
   
   [Error: b]
       at repl:1:8
       at Script.runInThisContext (vm.js:126:20)
       at REPLServer.defaultEval (repl.js:384:29)
       at bound (domain.js:415:14)
       at REPLServer.runBound [as eval] (domain.js:428:12)
       at REPLServer.onLine (repl.js:700:10)
       at REPLServer.emit (events.js:208:15)
       at REPLServer.EventEmitter.emit (domain.js:471:20)
       at REPLServer.Interface._onLine (readline.js:316:10)
       at REPLServer.Interface._line (readline.js:693:8) {
     generatedMessage: true,
     code: 'ERR_ASSERTION',
     actual: [Error: a],
     expected: [Error: b],
     operator: 'deepEqual'
   }
   > 
   
8. ljharb commented on Jul 31, 2019

   

   ljharb

   on Jul 31, 2019

   Member

   More actions

   I also see these:

   assert.deepStrictEqual(new Error('a'), new Error('a')); // passes
   assert.deepStrictEqual(new Error('a'), new Error('b')); // throws
   
   assert.deepEqual(new Error(), new TypeError()); // throws
   assert.deepStrictEqual(new Error(), new TypeError()); // throws
   
   assert.deepEqual(new Error(), Object.setPrototypeOf(new TypeError(), Error.prototype)); // passes
   assert.deepStrictEqual(new Error(), Object.setPrototypeOf(new TypeError(), Error.prototype)); // passes
   
   assert.deepEqual(new Error(), Object.assign(new Error(), { a: 1 })); // throws
   assert.deepStrictEqual(new Error(), Object.assign(new Error(), { a: 1 })); // throws

   Which suggests that it's comparing the .message, and the [[Prototype]] (like a normal object), as well as the own properties (like a normal object).

   I then, however, compared this:

   var e1 = new Error('a');
   var e2 = { __proto__: Error.prototype };
   Object.defineProperties(e2, Object.getOwnPropertyDescriptors(e1));
   
   assert.deepEqual(Reflect.ownKeys(e1), Reflect.ownKeys(e2)); // passes
   assert.deepStrictEqual(Reflect.ownKeys(e1), Reflect.ownKeys(e2)); // passes
   
   assert.deepEqual(Object.getPrototypeOf(e1), Object.getPrototypeOf(e2)); // passes
   assert.deepStrictEqual(Object.getPrototypeOf(e1), Object.getPrototypeOf(e2)); // passes
   
   assert.equal(e1 instanceof Error, e2 instanceof Error); // passes
   
   assert.deepEqual(e1, e2); // throws
   assert.deepStrictEqual(e1, e2); // throws

   and separately, non-enumerable properties are not compared on non-errors:

   var o1 = { message: 'a' };
   var o2 = { message: 'a' };
   Object.defineProperty(o2, 'message', { enumerable: false });
   assert.deepEqual(o1, o2); // throws
   assert.deepStrictEqual(o1, o2); // throws
   
   Object.defineProperty(o1, 'message', { enumerable: false });
   assert.deepEqual(o1, o2); // passes
   assert.deepStrictEqual(o1, o2); // passes
   
   o2.message = 'b';
   assert.deepEqual(o1, o2); // passes
   assert.deepStrictEqual(o1, o2); // passes

   which suggests that there is, unfortunately, Error-specific checking - presumably some kind of brand check that we'll be unable to do at runtime, and that non-enumerable properties are only checked on Errors and not on normal objects.
9. agirorn commented on Aug 1, 2019

   

   agirorn

   on Aug 1, 2019

   ContributorAuthor

   More actions

   Are those not all the cases that PR #48 covers for this module?
   Do you think I should add more tests cases to #48 to verify that?
10. ljharb commented on Aug 3, 2019

    

    ljharb

    on Aug 3, 2019

    Member

    More actions

    The issue is that there's no runtime way of brand-checking Error objects, and instanceof Error is not sufficient.
11. ljharb

    closed this as completedin #48on Aug 10, 2019