cannot stub newly created net.Socket

[benfleis]

Attempts at stub()ing a Socket have failed. I debugged a little bit, and it appears to walk quite a way through the property list before failing. I will spend a bit more effort on it, but thought it worth filing at this point.

Sample code:

var sinon = require('sinon');
var net = require('net');
var s = new net.Socket();
sinon.stub(s);

Failure:

$ node socket_stub_fails.js

TypeError: Cannot read property 'address' of undefined
    at Socket.remoteAddress (net.js:452:29)
    at Object.stub (/.../node_modules/sinon/lib/sinon/stub.js:47:34)
    at Object.<anonymous> (/.../socket_stub_fails.js:4:7)
    at Module._compile (module.js:449:26)
    at Object.Module._extensions..js (module.js:467:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.runMain (module.js:492:10)
    at process.startup.processNextTick.process._tickCallback (node.js:244:9)

[benfleis]

Okay, more info. The offending property is 'remoteAddress', which is a "getter", defined as:

Socket.prototype.__defineGetter__('remoteAddress', function() {                                                                                                                                         
  return this._getpeername().address;
});

My read is that because it's defined as a Getter, it automatically executes the behavior, which we don't want in this case. Instead it needs to be special cased, and that behavior dropped. I don't know how to detect getters. Still looking.

[cjohansen]

Sinon doesn't properly support getters and setters. I guess ideally the code should feature detect Object.getOwnPropertyDescriptor and use that with Object.defineProperty when available. Man, I hate getters and setters :(

[benfleis]

I just tried that, and it doesn't work. net.Socket.remoteAddress is defined this way:

Socket.prototype.__defineGetter__('remoteAddress', function() {                                                                                                                                         
  return this._getpeername().address;
});

and my code tried this:

        if (!property && !!object && typeof object == "object") {
            for (var prop in object) {
                var propDesc = Object.getOwnPropertyDescriptor(object, prop);
                if (prop === 'remoteAddress')                                                                                                                                                           
                    debugger;
                if ((propDesc && propDesc.get) || typeof object[prop] === "function") {
                    stub(object, prop);
                }
            }

            return object;
        }

propDesc was undefined.

Other ideas?

[cjohansen]

Is it an inherited property then?

My suggestion is to not rely on the Socket constructor at all. After all, if you intend to stub all properties on it, why not just build a stub like this?

var socket = { remoteAddress: "0.0.0.0", on: sinon.spy(), ... };

[benfleis]

That's what I've already begun. But life is better if I can simply sinon.stub(obj) and roll on :)

[benfleis]

Ok. I have a diff principle. I suspect it needs to also handle the getPropertyDescriptor case as well, and don't know where setters fit into this. (I've been coding in Node on and off for a couple months, and lack depth in understanding, frankly.) In any case, this works for net.Socket.

This is patched against HEAD, not 1.3.4. But I'm running it against 1.3.4, as mentioned. Thoughts?

diff --git a/sinon.js b/sinon.js                                                                                                                                                                        
index 27184e3..88db212 100644
--- a/sinon.js
+++ b/sinon.js
@@ -59,7 +59,8 @@ var sinon = (function (buster) {
                 throw new TypeError("Method wrapper should be function");
             }
·
-            var wrappedMethod = object[property];
+            var getter = object.__lookupGetter__(property);
+            var wrappedMethod = getter || object[property];
·
             if (!isFunction(wrappedMethod)) {
                 throw new TypeError("Attempted to wrap " + (typeof wrappedMethod) + " property " +
@@ -78,6 +79,14 @@ var sinon = (function (buster) {
             // IE 8 does not support hasOwnProperty on the window object.
             var owned = hasOwn.call(object, property);
             object[property] = method;
+
+            // handle getters appropriately, by replacing with a getter.
+            // could||should delete+replace it with a normal property.
+            if (getter)
+              object.__defineGetter__(property, method);
+            else
+              object[property] = method;
+
             method.displayName = property;
·
             method.restore = function () {
diff --git a/sinon/stub.js b/sinon/stub.js
index 15d90f3..76fb669 100644
--- a/sinon/stub.js
+++ b/sinon/stub.js
@@ -44,7 +44,8 @@
·
         if (!property && !!object && typeof object == "object") {
             for (var prop in object) {
-                if (typeof object[prop] === "function") {
+                var getter = object.__lookupGetter__(prop);
+                if (getter || typeof object[prop] === "function") {
                     stub(object, prop);
                 }
             }

[juanghurtado]

Any news on Sinon spies and stubs supporting getters and setters?

[ouhouhsami]

I would like to have it, also for the 'value' of an ES5 defineProperty ! ... So, any news or ideas on how to use Sinon stubs with these properties ?
Cheers

[cjohansen]

I've made graceful failure for getters a release goal for Sinon 2.0: #600