`assert.deepEqual` loops forever with circular refs

[kuraga]

Version: v4.4.3

Platform: Linux 3.13.0-24-generic #47-Ubuntu SMP Fri May 2 23:30:00 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

Code:

var assert = require('assert');

var b = {};
b.b = b;

var c = {};
c.b = c;

assert.deepEqual(b, c);

Expected: no exceptions.

Actual: RangeError: Maximum call stack size exceeded

See also:

• assert.deepEqual() does not stop on circular references node-v0.x-archive#207
• https://github.com/nodejs/node/blob/v4.4.3/test/parallel/test-assert.js#L369-L381
• https://github.com/nodejs/node/blob/v4.4.3/test/parallel/test-assert.js#L387

[Trott]

Also affects deepStrictEqual().

[Trott]

Judging from the test, this is sort of by design. The original behavior was (apparently) to loop forever but now it throws an error instead.

Should be fixable by keeping track of seen references. Not sure about the semver implications or if the Locked status of the API is an issue here or not. (I would consider this a bugfix myself, but others may disagree.)

[addaleax]

@Trott I’ve looked a bit at the original issue linked above, and I think Ryan Dahl actually just forgot to pull in the patch which does what you suggest (nodejs/node-v0.x-archive#207 (comment)). Would say it’s a bugfix, too.

[Trott]

@addaleax Cool! I think I've got a fix almost ready to go, but if you're already working on it, I'm happy to drop it and go do something else.

[addaleax]

@Trott No, sorry, didn’t mean to imply that I was coding anything right now – go ahead! 😄