`util_equals` will throw on `.equals` check for Buffer instances

[ggoodman]

Handling of Buffer instances may need to be special cased because otherwise the first line of the return statement will throw a TypeError whenever a Buffer is being evaluated.

function util_equals (a, b, stackA, stackB) {
  if ((Object.is && Object.is(a, b)) || _is(a, b)) {
    return true;
  }
  if (!(a && b)) return false;

  return (typeof a.equals === 'function' && a.equals(b)) ||
         (typeof b.equals === 'function' && b.equals(a)) ||
         _equals(a, b, stackA, stackB) ||
         false;
}

Reproduction:

var havelock = require('havelock');
var buf = new Buffer('string', 'utf8');
var atom = havelock.atom('abcdef');
var buf = atom.derive(function (str) {
    return new Buffer(str, 'utf8');
});

buf.react(function (buf) {
  console.log('Call me, maybe?'); // Sorry, not getting that call
});

This will not throw if using a Buffer Atom directly. Only via derivations will the issue get triggered.

[ds300]

To be explicit, the problem is that the Buffer#equals method throws a TypeError if the formal parameter is anything other than a Buffer.

I'm tempted to not take any action on this because: I'm pretty sure that's unusual behaviour for a .equals method, Buffers are node-specific, and Havelock already exposes the ability to override equality precisely because of such issues.

I'll leave this open for discussion for now though. Maybe there should be a node-only curated substitution for util_equals which takes care of this and whatever other idiosyncrasies pop up in the node stdlib?

[ggoodman]

How would you propose overloading Buffer for this kind of one-off change in #equals? Custom object derived from Buffer with an overloaded comparison method?

[ds300]

The options are currently:

1. Wrapping Buffer by inheritance or delegation, while supplying a sensible .equals method

2. re-initiliazing havelock with a custom equality function which either:

   a. surrounds the whole equality check in a try block, returning false if an error is thrown.
   b. handles buffers with a special case

1 potentially involves a lot of work and could get really messy if your wrapped types need to interact with other people's code.

2.a involves negligible extra work but means that real errors in .equals methods will be ignored. Plus there would be extra computational overhead and possibly an icky feeling in your gut as a result of using try/catch for flow control.

2.b involves minimal extra work and has no potential to get messy or cause icky feelings.

So I think 2.b is the clear winner here. Especially if Havelock exposed util_equals as defaultEquals or something so other people can use it when writing their custom equality functions.