Regression: Error when matching message body

[MacMcIrish]

What is the expected behavior?
To not crash on message body matching if body doesn't match.

What is the actual behavior?
Crashing on message body matching if message body has same count of keys, but different keys.

Possible solution
Go back to using deep-equal npm package. There was a PR here that introduced custom deepEqual logic that is not good enough.
If two objects have the same count of keys but not the same keys, it tries to extract the keys on undefined and crashes, making debugging hard to impossible.

How to reproduce the issue

Runkit: Link

Having problem producing a test case? Try and ask the community for help. If the test case cannot be reproduced, the Nock community might not be able to help you.

Does the bug have a test case?

Versions

Software	Version(s)
Nock	11.3.2
Node	10.15.3

[simlu]

This regression is unfortunately very impacting for us. Would be great to get a quick response! Thank you very much for all the hard work!

[paulmelnikow]

Thanks for the test case!

[paulmelnikow]

Can you confirm the fix in #1714?

[MacMcIrish]

@paulmelnikow I can confirm that #1714 will fix the issue for now. Thank you for the quick response!

Should bringing deep-equal (which is a very popular and maintained package) back into the project be considered? There may be more edge cases not considered with the custom utility.

[paulmelnikow]

On one hand I agree it's better to pull in a library rather than solve problems over and over again. On the other side, a lot of what makes deep-equal complicated seems to be its focus on performance, which is not a big consideration in nock.

This particular bug was very shallow, though I'd say we should consider reverting to deep-equal if we find more subtle edge-case bugs.

/cc @mastermatt, I'd be curious your thoughts.

[nockbot]

🎉 This issue has been resolved in version 11.3.4 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀