Rewrote tests and simplified implementation

Author: kamilogorek

lib/client.js

@@ -230,25 +230,13 @@ extend(Raven.prototype, {
       so we only parse a `req` property if the `request` property is absent/empty (and hence we won't clobber)
       parseUser returns a partial kwargs object with a `request` property and possibly a `user` property
     */
-    var initialRequest = utils.prepareInitialRequest(
-      this._globalContext.request,
-      domainContext.request,
-      kwargs.request
-    );
-    kwargs.request = extend(
-      initialRequest,
+    kwargs.request = this._createRequestObject(
       this._globalContext.request,
       domainContext.request,
       kwargs.request
     );
     if (Object.keys(kwargs.request).length === 0) {
-      var initialReq = utils.prepareInitialRequest(
-        this._globalContext.req,
-        domainContext.req,
-        kwargs.req
-      );
-      var req = extend(
-        initialReq,
+      var req = this._createRequestObject(
         this._globalContext.req,
         domainContext.req,
         kwargs.req
@@ -529,6 +517,36 @@ extend(Raven.prototype, {
       currCtx.breadcrumbs.shift();
     }
     this.setContext(currCtx);
+  },
+
+  _createRequestObject: function() {
+    /**
+     * When using proxy, some of the attributes of req/request objects are non-enumerable.
+     * To make sure, that they are still available to us after we consolidate our sources
+     * (eg. globalContext.request + domainContext.request + kwargs.request),
+     * we manually pull them out from original objects.
+     *
+     * We don't use Object.assign/extend as it's only merging over objects own properties,
+     * and we don't want to go through all of the properties as well, as we simply don't
+     * need all of them.
+     *
+     * So far the only missing piece is `ip`, but we can specify what properties should
+     * be pulled by extending `nonEnumerables` array.
+     **/
+    var sources = Array.from(arguments).filter(function(source) {
+      return Object.prototype.toString.call(source) === '[object Object]';
+    });
+    sources = [{}].concat(sources);
+    var request = extend.apply(null, sources);
+    var nonEnumberables = ['ip'];
+
+    nonEnumberables.forEach(function(key) {
+      sources.forEach(function(source) {
+        if (source[key]) request[key] = source[key];
+      });
+    });
+
+    return request;
   }
 });
 

lib/utils.js

@@ -56,22 +56,6 @@ module.exports.getAuthHeader = function getAuthHeader(timestamp, apiKey, apiSecr
   return header.join(', ');
 };
 
-module.exports.prepareInitialRequest = function prepareInitialRequest() {
-  var request = {};
-  var nonEnumberables = ['ip'];
-  var sources = Array.from(arguments).filter(function(source) {
-    return Object.prototype.toString.call(source) === '[object Object]';
-  });
-
-  nonEnumberables.forEach(function(key) {
-    sources.forEach(function(source) {
-      if (source[key]) request[key] = source[key];
-    });
-  });
-
-  return request;
-};
-
 module.exports.parseDSN = function parseDSN(dsn) {
   if (!dsn) {
     // Let a falsey value return false explicitly

test/raven.client.js

@@ -1369,6 +1369,102 @@ describe('raven.Client', function() {
       });
     });
   });
+
+  describe('#_createRequestObject', function() {
+    it('should merge together all sources', function() {
+      var req = client._createRequestObject(
+        {
+          foo: 123
+        },
+        {
+          bar: 42
+        }
+      );
+      var expected = {
+        foo: 123,
+        bar: 42
+      };
+      req.should.eql(expected);
+    });
+
+    it('should preserve extend-like order', function() {
+      var req = client._createRequestObject(
+        {
+          foo: 111
+        },
+        {
+          foo: 222
+        },
+        {
+          foo: 333
+        }
+      );
+      var expected = {
+        foo: 333
+      };
+      req.should.eql(expected);
+    });
+
+    it('should filter incorrect sources', function() {
+      var req = client._createRequestObject(
+        {
+          foo: 111
+        },
+        [42],
+        null,
+        'hello',
+        {
+          foo: 222
+        }
+      );
+      var expected = {
+        foo: 222
+      };
+      req.should.eql(expected);
+    });
+
+    it('should extract specified non-enumerables', function() {
+      var foo = {};
+      Object.defineProperty(foo, 'ip', {
+        value: '127.0.0.1',
+        enumerable: false
+      });
+      var bar = {
+        foo: 222
+      };
+      var req = client._createRequestObject(foo, bar);
+      var expected = {
+        foo: 222,
+        ip: '127.0.0.1'
+      };
+      req.should.eql(expected);
+    });
+
+    it('should skip all remaining non-enumerables', function() {
+      var foo = {};
+      Object.defineProperty(foo, 'ip', {
+        value: '127.0.0.1',
+        enumerable: false
+      });
+      Object.defineProperty(foo, 'pickle', {
+        value: 'rick',
+        enumerable: false
+      });
+      var bar = {
+        dont: 'skip'
+      };
+      Object.defineProperty(bar, 'evil', {
+        value: 'morty',
+        enumerable: false
+      });
+      var req = client._createRequestObject(foo, bar);
+      var expected = {
+        ip: '127.0.0.1',
+        dont: 'skip'
+      };
+      req.should.eql(expected);
+    });
+  });
 });
 
 describe('raven requestHandler/errorHandler middleware', function() {

test/raven.utils.js

@@ -98,66 +98,6 @@ describe('raven.utils', function() {
     });
   });
 
-  describe('#prepareInitialRequest', function() {
-    it('should extract specific non-enumerables and return an object containing them', function() {
-      var req = raven.utils.prepareInitialRequest({
-        ip: '127.0.0.1'
-      });
-      var expected = {
-        ip: '127.0.0.1'
-      };
-      req.should.eql(expected);
-    });
-
-    it('should skip non-listed properties', function() {
-      var req = raven.utils.prepareInitialRequest({
-        ip: '127.0.0.1',
-        pickle: 'Rick',
-        evil: 'Morty'
-      });
-      var expected = {
-        ip: '127.0.0.1'
-      };
-      req.should.eql(expected);
-    });
-
-    it('should preserve extend-like order', function() {
-      var req = raven.utils.prepareInitialRequest(
-        {
-          ip: '127.0.0.1'
-        },
-        {
-          ip: '123.123.123.123'
-        },
-        {
-          ip: '42.42.42.42'
-        }
-      );
-      var expected = {
-        ip: '42.42.42.42'
-      };
-      req.should.eql(expected);
-    });
-
-    it('should filter incorrect sources', function() {
-      var req = raven.utils.prepareInitialRequest(
-        {
-          ip: '127.0.0.1'
-        },
-        [42],
-        null,
-        'hello',
-        {
-          ip: '123.123.123.123'
-        }
-      );
-      var expected = {
-        ip: '123.123.123.123'
-      };
-      req.should.eql(expected);
-    });
-  });
-
   describe('#parseAuthHeader()', function() {
     it('should parse all parameters', function() {
       var timestamp = 12345,