Merge pull request #681 from bugsnag/v7-client-state

V7: Move context and breadcrumbs to private properties

CHANGELOG.md

@@ -27,6 +27,8 @@
 - Remove `client.app` property [#677](https://github.com/bugsnag/bugsnag-js/pull/677)
 - Rename and make private the `Session` method `trackError()` -> `_track()` [#675](https://github.com/bugsnag/bugsnag-js/pull/675)
 - Update `Event` to support multiple errors [#680](https://github.com/bugsnag/bugsnag-js/pull/680)
+- Move breadcrumbs to a private property on `client._breadcrumbs` [#681](https://github.com/bugsnag/bugsnag-js/pull/681)
+- Move context to a private property on `Client`, and get/set via `getContext()/setContext()` [#681](https://github.com/bugsnag/bugsnag-js/pull/681)
 
 ## 6.5.0 (2019-12-16)
 

packages/core/client.js

@@ -19,21 +19,18 @@ class Client {
     this._config = {}
     this._schema = schema
 
-    // // i/o
+    // i/o
     this._delivery = { sendSession: noop, sendEvent: noop }
     this._logger = { debug: noop, info: noop, warn: noop, error: noop }
 
     // plugins
     this._plugins = {}
 
+    // state
+    this._breadcrumbs = []
     this._session = null
-
     this._metadata = {}
-
-    this.breadcrumbs = []
-
-    // setable props
-    this.context = undefined
+    this._context = undefined
     this._user = {}
 
     // callbacks:
@@ -82,6 +79,14 @@ class Client {
     return metadataDelegate.clear(this._metadata, section, key)
   }
 
+  getContext () {
+    return this._context
+  }
+
+  setContext (c) {
+    this._context = c
+  }
+
   _extractConfiguration (partialSchema = this._schema) {
     const conf = config.mergeDefaults(this._opts, partialSchema)
     const validity = config.validate(conf, partialSchema)
@@ -91,6 +96,7 @@ class Client {
     // update and elevate some special options if they were passed in at this point
     if (conf.metadata) this._metadata = conf.metadata
     if (conf.user) this._user = conf.user
+    if (conf.context) this._context = conf.context
     if (conf.logger) this._logger = conf.logger
 
     // add callbacks
@@ -207,9 +213,9 @@ class Client {
     }
 
     // push the valid crumb onto the queue and maintain the length
-    this.breadcrumbs.push(crumb)
-    if (this.breadcrumbs.length > this._config.maxBreadcrumbs) {
-      this.breadcrumbs = this.breadcrumbs.slice(this.breadcrumbs.length - this._config.maxBreadcrumbs)
+    this._breadcrumbs.push(crumb)
+    if (this._breadcrumbs.length > this._config.maxBreadcrumbs) {
+      this._breadcrumbs = this._breadcrumbs.slice(this._breadcrumbs.length - this._config.maxBreadcrumbs)
     }
   }
 
@@ -225,10 +231,10 @@ class Client {
       version: this._config.appVersion,
       type: this._config.appType
     }
-    event.context = event.context || this.context || undefined
+    event.context = event.context || this._context
     event._metadata = { ...event._metadata, ...this._metadata }
     event._user = { ...event._user, ...this._user }
-    event.breadcrumbs = this.breadcrumbs.slice(0)
+    event.breadcrumbs = this._breadcrumbs.slice()
 
     if (this._session) {
       this._session._track(event)

packages/core/config.js

@@ -88,9 +88,14 @@ module.exports.schema = {
       return includes(BREADCRUMB_TYPES, maybeType)
     }, true))
   },
+  context: {
+    defaultValue: () => undefined,
+    message: 'should be a string',
+    validate: value => value === undefined || typeof value === 'string'
+  },
   user: {
     defaultValue: () => null,
-    message: '(object) user should be an object',
+    message: 'should be an object',
     validate: (value) => typeof value === 'object'
   },
   metadata: {

packages/core/lib/clone-client.js

@@ -1,15 +1,14 @@
 module.exports = (client) => {
   const clone = new client.Client({}, {}, client._notifier)
 
-  // changes to these properties should be reflected in the original client
   clone._config = client._config
-  clone.context = client.context
 
   // changes to these properties should not be reflected in the original client,
   // so ensure they are are (shallow) cloned
-  clone.breadcrumbs = client.breadcrumbs.slice()
+  clone._breadcrumbs = client._breadcrumbs.slice()
   clone._metadata = { ...client._metadata }
   clone._user = { ...client._user }
+  clone._context = client._context
 
   clone._cbs = {
     e: client._cbs.e.slice(),

packages/core/test/client.test.js

@@ -244,10 +244,10 @@ describe('@bugsnag/core/client', () => {
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
       client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
       client.notify(new Error('foobar'))
-      expect(client.breadcrumbs.length).toBe(1)
-      expect(client.breadcrumbs[0].type).toBe('error')
-      expect(client.breadcrumbs[0].message).toBe('Error')
-      expect(client.breadcrumbs[0].metadata.stacktrace).toBe(undefined)
+      expect(client._breadcrumbs.length).toBe(1)
+      expect(client._breadcrumbs[0].type).toBe('error')
+      expect(client._breadcrumbs[0].message).toBe('Error')
+      expect(client._breadcrumbs[0].metadata.stacktrace).toBe(undefined)
       // the error shouldn't appear as a breadcrumb for itself
       expect(payloads[0].events[0].breadcrumbs.length).toBe(0)
     })
@@ -338,23 +338,23 @@ describe('@bugsnag/core/client', () => {
     it('creates a manual breadcrumb when a list of arguments are supplied', () => {
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
       client.leaveBreadcrumb('french stick')
-      expect(client.breadcrumbs.length).toBe(1)
-      expect(client.breadcrumbs[0].type).toBe('manual')
-      expect(client.breadcrumbs[0].message).toBe('french stick')
-      expect(client.breadcrumbs[0].metadata).toEqual({})
+      expect(client._breadcrumbs.length).toBe(1)
+      expect(client._breadcrumbs[0].type).toBe('manual')
+      expect(client._breadcrumbs[0].message).toBe('french stick')
+      expect(client._breadcrumbs[0].metadata).toEqual({})
     })
 
     it('caps the length of breadcrumbs at the configured limit', () => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', maxBreadcrumbs: 3 })
       client.leaveBreadcrumb('malted rye')
-      expect(client.breadcrumbs.length).toBe(1)
+      expect(client._breadcrumbs.length).toBe(1)
       client.leaveBreadcrumb('medium sliced white hovis')
-      expect(client.breadcrumbs.length).toBe(2)
+      expect(client._breadcrumbs.length).toBe(2)
       client.leaveBreadcrumb('pumperninkel')
-      expect(client.breadcrumbs.length).toBe(3)
+      expect(client._breadcrumbs.length).toBe(3)
       client.leaveBreadcrumb('seedy farmhouse')
-      expect(client.breadcrumbs.length).toBe(3)
-      expect(client.breadcrumbs.map(b => b.message)).toEqual([
+      expect(client._breadcrumbs.length).toBe(3)
+      expect(client._breadcrumbs.map(b => b.message)).toEqual([
         'medium sliced white hovis',
         'pumperninkel',
         'seedy farmhouse'
@@ -367,18 +367,18 @@ describe('@bugsnag/core/client', () => {
       client.leaveBreadcrumb(null, { data: 'is useful' })
       client.leaveBreadcrumb(null, {}, null)
       client.leaveBreadcrumb(null, { t: 10 }, null, 4)
-      expect(client.breadcrumbs.length).toBe(0)
+      expect(client._breadcrumbs.length).toBe(0)
     })
 
     it('allows maxBreadcrumbs to be set to 0', () => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', maxBreadcrumbs: 0 })
       client.leaveBreadcrumb('toast')
-      expect(client.breadcrumbs.length).toBe(0)
+      expect(client._breadcrumbs.length).toBe(0)
       client.leaveBreadcrumb('toast')
       client.leaveBreadcrumb('toast')
       client.leaveBreadcrumb('toast')
       client.leaveBreadcrumb('toast')
-      expect(client.breadcrumbs.length).toBe(0)
+      expect(client._breadcrumbs.length).toBe(0)
     })
 
     it('doesn’t store the breadcrumb if an onBreadcrumb callback returns false', () => {
@@ -392,7 +392,7 @@ describe('@bugsnag/core/client', () => {
       })
       client.leaveBreadcrumb('message')
       expect(calls).toBe(1)
-      expect(client.breadcrumbs.length).toBe(0)
+      expect(client._breadcrumbs.length).toBe(0)
     })
 
     it('tolerates errors in onBreadcrumb callbacks', () => {
@@ -406,7 +406,7 @@ describe('@bugsnag/core/client', () => {
       })
       client.leaveBreadcrumb('message')
       expect(calls).toBe(1)
-      expect(client.breadcrumbs.length).toBe(1)
+      expect(client._breadcrumbs.length).toBe(1)
     })
 
     it('ignores breadcrumb types that aren’t in the enabled list', () => {
@@ -416,8 +416,8 @@ describe('@bugsnag/core/client', () => {
       })
       client.leaveBreadcrumb('brrrrr')
       client.leaveBreadcrumb('GET /jim', {}, 'request')
-      expect(client.breadcrumbs.length).toBe(1)
-      expect(client.breadcrumbs[0].message).toBe('brrrrr')
+      expect(client._breadcrumbs.length).toBe(1)
+      expect(client._breadcrumbs[0].message).toBe('brrrrr')
     })
   })
 
@@ -549,6 +549,18 @@ describe('@bugsnag/core/client', () => {
     })
   })
 
+  describe('get/setContext()', () => {
+    it('modifies and retreives context', () => {
+      const c = new Client({ apiKey: 'API_KEY' })
+      c.setContext('str')
+      expect(c.getContext()).toBe('str')
+    })
+    it('can be set via config', () => {
+      const c = new Client({ apiKey: 'API_KEY', context: 'str' })
+      expect(c.getContext()).toBe('str')
+    })
+  })
+
   describe('add/get/clearMetadata()', () => {
     it('modifies and retrieves metadata', () => {
       const client = new Client({ apiKey: 'API_KEY' })

packages/core/types/test/test.js

@@ -0,0 +1,96 @@
+"use strict";
+var __importDefault = (this && this.__importDefault) || function (mod) {
+    return (mod && mod.__esModule) ? mod : { "default": mod };
+};
+Object.defineProperty(exports, "__esModule", { value: true });
+var __1 = __importDefault(require("../.."));
+require("jasmine");
+// the client's constructor isn't public in TS so this drops down to JS to create one for the tests
+function createClient(opts) {
+    var c = new __1.default.Client(opts, undefined, { name: 'Type Tests', version: 'nope', url: 'https://github.com/bugsnag/bugsnag-js' });
+    c._setDelivery(function () { return ({
+        sendSession: function (p, cb) { cb(); },
+        sendEvent: function (p, cb) { cb(); }
+    }); });
+    c._sessionDelegate = { startSession: function () { return c; }, pauseSession: function () { }, resumeSession: function () { } };
+    return c;
+}
+describe('Type definitions', function () {
+    it('has all the classes matching the types available at runtime', function () {
+        expect(__1.default.Client).toBeDefined();
+        expect(__1.default.Breadcrumb).toBeDefined();
+        expect(__1.default.Event).toBeDefined();
+        expect(__1.default.Session).toBeDefined();
+        var client = createClient({ apiKey: 'API_KEY' });
+        expect(client.Breadcrumb).toBeDefined();
+        expect(client.Event).toBeDefined();
+        expect(client.Session).toBeDefined();
+    });
+    it('works for reporting errors', function (done) {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.notify(new Error('uh oh'), function (event) {
+            expect(event.apiKey).toBe(undefined);
+            expect(event.context).toBe(undefined);
+            expect(event.errors[0].errorMessage).toBe('uh oh');
+            expect(event.errors[0].stacktrace.length > 0).toBe(true);
+            event.addMetadata('abc', { def: 'ghi' });
+            event.addMetadata('jkl', 'mno', 'pqr');
+            event.clearMetadata('jkl');
+            var val = event.getMetadata('jkl');
+            expect(val).toBe(undefined);
+        }, function (err, event) {
+            expect(err).toBe(undefined);
+            expect(event).toBeTruthy();
+            done();
+        });
+    });
+    it('works for reporting sessions', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        var sessionClient = client.startSession();
+        sessionClient.notify(new Error('oh'));
+        client.pauseSession();
+        client.resumeSession();
+    });
+    it('works for leaving breadcrumbs', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.leaveBreadcrumb('testing 123');
+        expect(client._breadcrumbs.length).toBe(1);
+        expect(client._breadcrumbs[0].message).toBe('testing 123');
+    });
+    it('works adding and removing onError callbacks', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.addOnError(function () { });
+        client.removeOnError(function () { });
+    });
+    it('works adding and removing onSession callbacks', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.addOnSession(function () { });
+        client.removeOnSession(function () { });
+    });
+    it('works adding and removing onBreadcrumb callbacks', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.addOnBreadcrumb(function () { });
+        client.removeOnBreadcrumb(function () { });
+    });
+    it('works manipulating metadata on client', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.addMetadata('abc', { def: 'ghi' });
+        client.addMetadata('jkl', 'mno', 'pqr');
+        client.clearMetadata('jkl');
+        var val = client.getMetadata('jkl');
+        expect(val).toBe(undefined);
+        expect(client.getMetadata('abc', 'def')).toBe('ghi');
+    });
+    it('works setting context', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.setContext('foo');
+        expect(client.getContext()).toBe('foo');
+    });
+    it('works setting/clearing user', function () {
+        var client = createClient({ apiKey: 'API_KEY' });
+        client.setUser('123', 'ben.gourley@bugsnag.com', 'Ben');
+        expect(client.getUser()).toEqual({ id: '123', name: 'Ben', email: 'ben.gourley@bugsnag.com' });
+        client.setUser(undefined, undefined, undefined);
+        expect(client.getUser()).toEqual({ id: undefined, name: undefined, email: undefined });
+    });
+});

packages/plugin-browser-context/test/context.test.js

@@ -28,7 +28,7 @@ describe('plugin: context', () => {
     const payloads = []
     client.use(plugin, window)
 
-    client.context = 'something else'
+    client.setContext('something else')
 
     client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
     client.notify(new Error('noooo'))

packages/plugin-console-breadcrumbs/test/console-breadcrumbs.test.js

@@ -20,11 +20,11 @@ describe('plugin: console breadcrumbs', () => {
         rabbit: 'sniffer'
       }
     })
-    expect(c.breadcrumbs.length).toBe(3)
-    expect(c.breadcrumbs[0].metadata['[0]']).toBe('check 1, 2')
-    expect(c.breadcrumbs[1].metadata['[0]']).toBe('null')
-    expect(c.breadcrumbs[2].metadata['[0]']).toBe('{"foo":[1,2,3,"four"]}')
-    expect(c.breadcrumbs[2].metadata['[1]']).toBe('{"pets":{"cat":"scratcher","dog":"pupper","rabbit":"sniffer"}}')
+    expect(c._breadcrumbs.length).toBe(3)
+    expect(c._breadcrumbs[0].metadata['[0]']).toBe('check 1, 2')
+    expect(c._breadcrumbs[1].metadata['[0]']).toBe('null')
+    expect(c._breadcrumbs[2].metadata['[0]']).toBe('{"foo":[1,2,3,"four"]}')
+    expect(c._breadcrumbs[2].metadata['[1]']).toBe('{"pets":{"cat":"scratcher","dog":"pupper","rabbit":"sniffer"}}')
     // undo the global side effects of wrapping console.* for the rest of the tests
     plugin.destroy()
   })
@@ -33,33 +33,33 @@ describe('plugin: console breadcrumbs', () => {
     const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa' })
     c.use(plugin)
     expect(() => console.log(Object.create(null))).not.toThrow()
-    expect(c.breadcrumbs.length).toBe(1)
-    expect(c.breadcrumbs[0].message).toBe('Console output')
-    expect(c.breadcrumbs[0].metadata['[0]']).toBe('[Unknown value]')
+    expect(c._breadcrumbs.length).toBe(1)
+    expect(c._breadcrumbs[0].message).toBe('Console output')
+    expect(c._breadcrumbs[0].metadata['[0]']).toBe('[Unknown value]')
     plugin.destroy()
   })
 
   it('should not be enabled when enabledBreadcrumbTypes=[]', () => {
     const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', enabledBreadcrumbTypes: [] })
     c.use(plugin)
     console.log(123)
-    expect(c.breadcrumbs.length).toBe(0)
+    expect(c._breadcrumbs.length).toBe(0)
     plugin.destroy()
   })
 
   it('should be enabled when enabledBreadcrumbTypes=["log"]', () => {
     const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', enabledBreadcrumbTypes: ['log'] })
     c.use(plugin)
     console.log(123)
-    expect(c.breadcrumbs.length).toBe(1)
+    expect(c._breadcrumbs.length).toBe(1)
     plugin.destroy()
   })
 
   it('should be not enabled by default when releaseStage=development', () => {
     const c = new Client({ apiKey: 'aaaa-aaaa-aaaa-aaaa', releaseStage: 'development' })
     c.use(plugin)
     console.log(123)
-    expect(c.breadcrumbs.length).toBe(0)
+    expect(c._breadcrumbs.length).toBe(0)
     plugin.destroy()
   })
 })