Simplify context methods to fall back on instance-global context (#226)

* Simplify context methods to fall back on instance-global context

* Fix setContext, add merging hierarchy w/tests

* Simplify errorHandler middleware, call next immediately instead of waiting for capture

* Deprecate setUser/Tags/ExtraContext

* Rename/fix updateContext -> mergeContext

Author: LewisJEllis

lib/client.js

@@ -93,6 +93,11 @@ extend(Raven.prototype, {
       eventId = this.generateEventId();
     }
 
+    var domainContext = domain.active && domain.active.sentryContext || {};
+    kwargs.user = extend({}, this._globalContext.user, domainContext.user, kwargs.user);
+    kwargs.tags = extend({}, this._globalContext.tags, domainContext.tags, kwargs.tags);
+    kwargs.extra = extend({}, this._globalContext.extra, domainContext.extra, kwargs.extra);
+
     kwargs.modules = utils.getModules();
     kwargs.server_name = kwargs.server_name || this.name;
 
@@ -101,19 +106,12 @@ extend(Raven.prototype, {
     }
 
     kwargs.environment = kwargs.environment || this.environment;
-    kwargs.extra = extend({}, this._globalContext.extra, kwargs.extra);
-    kwargs.tags = extend({}, this._globalContext.tags, kwargs.tags);
-
     kwargs.logger = kwargs.logger || this.loggerName;
     kwargs.event_id = eventId;
     kwargs.timestamp = new Date().toISOString().split('.')[0];
     kwargs.project = this.dsn.project_id;
     kwargs.platform = 'node';
 
-    if (this._globalContext.user) {
-      kwargs.user = this._globalContext.user || kwargs.user;
-    }
-
     // Only include release information if it is set
     if (this.release) {
       kwargs.release = this.release;
@@ -287,28 +285,21 @@ extend(Raven.prototype, {
   },
 
   setContext: function setContext(ctx) {
-    if (!domain.active) {
-      utils.consoleAlert('attempt to setContext outside context scope');
-    } else {
+    if (domain.active) {
       domain.active.sentryContext = ctx;
+    } else {
+      this._globalContext = ctx;
     }
+    return this;
   },
 
-  // todo consider this naming; maybe "mergeContext" instead?
-  updateContext: function updateContext(ctx) {
-    if (!domain.active) {
-      utils.consoleAlert('attempt to updateContext outside context scope');
-    } else {
-      domain.active.sentryContext = extend({}, domain.active.sentryContext, ctx);
-    }
+  mergeContext: function mergeContext(ctx) {
+    extend(this.getContext(), ctx);
+    return this;
   },
 
   getContext: function getContext() {
-    if (!domain.active) {
-      utils.consoleAlert('attempt to getContext outside context scope');
-      return null;
-    }
-    return domain.active.sentryContext;
+    return domain.active ? domain.active.sentryContext : this._globalContext;
   },
 
   /*
@@ -317,9 +308,8 @@ extend(Raven.prototype, {
    * @param {object} user An object representing user data [optional]
    * @return {Raven}
    */
-  setUserContext: function setUserContext(user) {
-    utils.consoleAlert('setUserContext has been deprecated and will be removed in v2.0');
-    this._globalContext.user = user;
+  setUserContext: function setUserContext() {
+    utils.consoleAlert('setUserContext has been deprecated and will be removed in v2.0; use setContext instead');
     return this;
   },
 
@@ -329,9 +319,8 @@ extend(Raven.prototype, {
    * @param {object} extra An object representing extra data [optional]
    * @return {Raven}
    */
-  setExtraContext: function setExtraContext(extra) {
-    utils.consoleAlert('setExtraContext has been deprecated and will be removed in v2.0');
-    this._globalContext.extra = extend({}, this._globalContext.extra, extra);
+  setExtraContext: function setExtraContext() {
+    utils.consoleAlert('setExtraContext has been deprecated and will be removed in v2.0; use setContext instead');
     return this;
   },
 
@@ -341,9 +330,8 @@ extend(Raven.prototype, {
    * @param {object} tags An object representing tags [optional]
    * @return {Raven}
    */
-  setTagsContext: function setTagsContext(tags) {
-    utils.consoleAlert('setTagsContext has been deprecated and will be removed in v2.0');
-    this._globalContext.tags = extend({}, this._globalContext.tags, tags);
+  setTagsContext: function setTagsContext() {
+    utils.consoleAlert('setTagsContext has been deprecated and will be removed in v2.0; use setContext instead');
     return this;
   },
 
@@ -398,13 +386,9 @@ extend(Raven.prototype, {
       if (status < 500) return next(err);
 
       var kwargs = parsers.parseRequest(req);
-      if (domain.active && domain.active.sentryContext) {
-        kwargs = extend(kwargs, domain.active.sentryContext);
-      }
-      return self.captureException(err, kwargs, function (sendErr, eventId) {
-        res.sentry = eventId;
-        next(err);
-      });
+      var eventId = self.captureException(err, kwargs);
+      res.sentry = eventId;
+      return next(err);
     };
   }
 });

test/raven.client.js

@@ -748,9 +748,11 @@ describe('raven.Client', function () {
       release: 'version1'
     });
 
-    client.setUserContext({
-      email: '[email protected]',
-      id: '123'
+    client.setContext({
+      user: {
+        email: '[email protected]',
+        id: '123'
+      }
     });
 
     client.on('logged', function () {
@@ -788,52 +790,54 @@ describe('raven.Client', function () {
     });
   });
 
-  describe('#setUserContext()', function () {
-    it('should add the user object to the globalContext', function () {
-      var user = {
-        email: '[email protected]', // <-- my fave user
-        id: '123'
-      };
-
-      client.setUserContext(user);
-
-      client._globalContext.user.should.equal(user);
+  describe('#setContext', function () {
+    afterEach(function () {
+      process.domain && process.domain.exit();
     });
-  });
 
-  describe('#setExtraContext()', function () {
-    it('should merge the extra data object into the globalContext', function () {
-      // when no pre-existing context
-      client.setExtraContext({
-        bar: 'baz'
-      });
-
-      client._globalContext.extra.should.have.property('bar', 'baz');
+    it('should merge contexts in correct hierarchy', function (done) {
+      var scope = nock('https://app.getsentry.com')
+        .filteringRequestBody(/.*/, '*')
+        .post('/api/269/store/', '*')
+        .reply(200, function (uri, body) {
+          zlib.inflate(new Buffer(body, 'base64'), function (err, dec) {
+            if (err) return done(err);
+            var msg = JSON.parse(dec.toString());
 
-      client.setExtraContext({ // should merge onto previous
-        foo: 'bar'
-      });
+            msg.user.should.eql({
+              a: 1,
+              b: 2,
+              c: 3
+            });
 
-      client._globalContext.extra.should.have.property('foo', 'bar');
-      client._globalContext.extra.should.have.property('bar', 'baz');
-    });
-  });
+            done();
+          });
+          return 'OK';
+        });
 
-  describe('#setTagsContext()', function () {
-    it('should merge the extra data object into the globalContext', function () {
-      // when no pre-existing context
-      client.setTagsContext({
-        browser: 'Chrome'
+      client.setContext({
+        user: {
+          a: 1,
+          b: 1,
+          c: 1
+        }
       });
 
-      client._globalContext.tags.should.have.property('browser', 'Chrome');
-
-      client.setTagsContext({ // should merge onto previous
-        platform: 'OS X'
+      client.context(function () {
+        client.setContext({
+          user: {
+            b: 2,
+            c: 2
+          }
+        });
+        client.captureException(new Error('foo'), {
+          user: {
+            c: 3
+          }
+        }, function () {
+          scope.done();
+        });
       });
-
-      client._globalContext.tags.should.have.property('browser', 'Chrome');
-      client._globalContext.tags.should.have.property('platform', 'OS X');
     });
   });