assert.equal (and friends) throw incorrect error on circular refs

[bengl]

When assert.equal fails, I would expect to get an AssertionError. Instead, when one of the first two arguments has a circular reference in it, I get a TypeError. Here is a contrived example from a node#v0.10.33 repl:

> x = {}
{}
> x.x = x
{ x: [Circular] }
> assert.equal(x, {})
TypeError: Converting circular structure to JSON
    at Object.stringify (native)
    at getMessage (assert.js:75:24)
    at new AssertionError (assert.js:45:37)
    at fail (assert.js:92:9)
    at Function.equal (assert.js:121:27)
    at repl:1:9
    at REPLServer.self.eval (repl.js:110:21)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.emit (events.js:95:17)
    at Interface._onLine (readline.js:202:10)

Clearly this is happening because JSON.stringify fails on x with its circular references. Maybe something like util.inspect can be used instead, in order to replace circular references with [Circular]?

I've tested the following on both 0.10.33 and 0.11.14 and they all behave similarly for the x defined above.

assert.equal(x, {})
assert.equal({}, x)
assert.deepEqual(x, {})
assert.deepEqual({}, x)
assert.strictEqual(x, {})
assert.strictEqual({}, x)

[micnic]

I would propose my solution for deep equal: https://gist.github.com/micnic/2ed0cc5b01ab5413271f
It is inspired a bit from lodash and underscore .isEqual() function.

isEqual(x, {}); // false
isEqual(x, { x: x }); // true

[cjihrig]

In this case, the comparison has already failed, and the bad call to JSON.stringify() occurs while trying to create an error message. So, I don't think changing the comparison is the right thing to do for this issue (but could be worth bringing up on #7161).

I personally think that using util.inspect() makes sense. I put together cjihrig/node@e904589, which does exactly this. If the maintainers are interested, I can PR it.

[chrisdickinson]

I'm +1 on @cjihrig's proposed fix; @tjfontaine thoughts?

[nathanmacinnes]

I think using util.inspect as @chrisdickinson has implemented is a perfectly good fix, but if any maintainers think a more minimal fix (which alters existing behaviour less) is better, my PR is another option to consider.

[misterdjules]

Fixed by bcff90e, thank you @cjihrig and all!