Stack overflow on deepEqual

ferrao commented 7 years ago

When asserting the equality with Code.expect.equal() I am getting on Hoek.deepEqual:

  Maximum call stack size exceeded

  at Array.indexOf (native)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:278:14)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:360:27)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:306:26)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:360:27)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:360:27)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:360:27)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:360:27)
  at Object.exports.deepEqual (/Users/ferrao/Dev/Workspace/academia-codigo/noire/noire-server/node_modules/code/node_modules/hoek/lib/index.js:360:27)

hueniverse commented 6 years ago

That is not a valid statement. Can't do much with this information.

Marsup commented 6 years ago

I'm betting on cyclic objects...

hueniverse commented 6 years ago

We have circular ref protection. Same as clone.

ferrao commented 6 years ago

The objects are cyclic, but due to the circular ref protection this issue did not happened in lab 14.0.1. When I upgraded to to lab 14.3.1, with the exact same objects I got the stack overflow.

4.x.x is the Hoek dependency on both, so I am guessing somewhere in 4.x.x something changed in Hoek.

Sorry for the lack of detail, I will find the exact Hoek version where this issue surfaced and see what I can do to help.

hueniverse commented 6 years ago

I need a failing test...

ferrao commented 6 years ago

Got it!

The problem manifested itself doing an equal assertion on two route configuration objects.

The only thing out of the ordinary these route objects have are Joi validations, which Hoek could handle fine before, but at some point is not able to do so anymore, perhaps due to some changes in Joi itself.

You can verify this behaviour with a simple expect(Joi).to.equals(Joi);, which now causes a stack overflow.

kanongil commented 6 years ago

Failing test:

    it('handles circular dependency in array', () => {

        const a = { x: [] };
        a.x.push(a);

        const b = Hoek.clone(a);
        expect(Hoek.deepEqual(a, b)).to.be.true();
    });

I'm working on a fix.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

hapijs / hoek

Stack overflow on deepEqual #219