Merge pull request #659 from bugsnag/v7-remove-client-methods

V7: Remove non-public methods from client interface
bugsnag · Dec 5, 2019 · c063284 · c063284
2 parents 7e70758 + a7a96f6
commit c063284
Show file tree

Hide file tree

Showing 36 changed files with 115 additions and 145 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -13,6 +13,7 @@
 - Simplify client configuration, and store resulting config privately [#656](https://github.com/bugsnag/bugsnag-js/pull/656)
 - User is now stored privately on `client` and `event` and updated via get/set methods [#657](https://github.com/bugsnag/bugsnag-js/pull/657)
 - Rename `metaData` -> `metadata` and add consistent `add/get/clearMetadata()` methods to `Client`/`Event` for manipulating metadata explicitly, rather than mutating a property [#658](https://github.com/bugsnag/bugsnag-js/pull/658)
+- Remove non-public methods from `Client` interface: `logger()`, `delivery()` and `sessionDelegate()` [#659](https://github.com/bugsnag/bugsnag-js/pull/659)
 
 ## 6.4.3 (2019-10-21)
 

diff --git a/packages/browser/src/notifier.js b/packages/browser/src/notifier.js
@@ -37,7 +37,7 @@ module.exports = (opts) => {
   const bugsnag = new Client(opts, schema, { name, version, url })
 
   // set delivery based on browser capability (IE 8+9 have an XDomainRequest object)
-  bugsnag.delivery(window.XDomainRequest ? dXDomainRequest : dXMLHttpRequest)
+  bugsnag._setDelivery(window.XDomainRequest ? dXDomainRequest : dXMLHttpRequest)
 
   // add browser-specific plugins
   bugsnag.use(pluginDevice)

diff --git a/packages/core/client.js b/packages/core/client.js
@@ -80,7 +80,7 @@ class BugsnagClient {
     if (conf.appType) this.app.type = conf.appType
     if (conf.metadata) this._metadata = conf.metadata
     if (conf.user) this._user = conf.user
-    if (conf.logger) this.logger(conf.logger)
+    if (conf.logger) this._logger = conf.logger
 
     // merge with existing config
     this._config = { ...this._config, ...conf }
@@ -111,19 +111,8 @@ class BugsnagClient {
     return this._plugins[`~${name}~`]
   }
 
-  delivery (d) {
+  _setDelivery (d) {
     this._delivery = d(this)
-    return this
-  }
-
-  logger (l, sid) {
-    this._logger = l
-    return this
-  }
-
-  sessionDelegate (s) {
-    this._sessionDelegate = s
-    return this
   }
 
   startSession () {

diff --git a/packages/core/test/client.test.js b/packages/core/test/client.test.js
@@ -47,7 +47,7 @@ describe('@bugsnag/core/client', () => {
         expect(msg).toBeTruthy()
         done()
       }
-      client.logger({ debug: log, info: log, warn: log, error: log })
+      client._logger = { debug: log, info: log, warn: log, error: log }
       client._logger.debug('hey')
     })
     it('can supply a different logger via config', done => {
@@ -78,7 +78,7 @@ describe('@bugsnag/core/client', () => {
   describe('notify()', () => {
     it('delivers an error event', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: payload => {
           expect(payload).toBeTruthy()
           expect(Array.isArray(payload.events)).toBe(true)
@@ -93,7 +93,7 @@ describe('@bugsnag/core/client', () => {
 
     it('supports setting severity via callback', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           expect(payload).toBeTruthy()
           expect(Array.isArray(payload.events)).toBe(true)
@@ -110,7 +110,7 @@ describe('@bugsnag/core/client', () => {
 
     it('supports preventing send by returning false', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           fail('sendEvent() should not be called')
         }
@@ -133,7 +133,7 @@ describe('@bugsnag/core/client', () => {
           onErrorSpy
         ]
       })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           expect(payload.events[0].errorMessage).toBe('oh no!')
           expect(onErrorSpy).toHaveBeenCalledTimes(1)
@@ -146,7 +146,7 @@ describe('@bugsnag/core/client', () => {
 
     it('supports preventing send with enabledReleaseStages', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['qa'] })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           fail('sendEvent() should not be called')
         }
@@ -160,7 +160,7 @@ describe('@bugsnag/core/client', () => {
 
     it('supports setting releaseStage via config.releaseStage', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', releaseStage: 'staging', enabledReleaseStages: ['production'] })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           fail('sendEvent() should not be called')
         }
@@ -174,7 +174,7 @@ describe('@bugsnag/core/client', () => {
 
     it('supports setting releaseStage via client.app.releaseStage', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['production'] })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           fail('sendEvent() should not be called')
         }
@@ -189,7 +189,7 @@ describe('@bugsnag/core/client', () => {
 
     it('includes releaseStage in event.app', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['staging'] })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           expect(payload.events[0].app.releaseStage).toBe('staging')
           done()
@@ -201,7 +201,7 @@ describe('@bugsnag/core/client', () => {
 
     it('includes releaseStage in event.app when set via config', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['staging'], releaseStage: 'staging' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           expect(payload.events[0].app.releaseStage).toBe('staging')
           done()
@@ -212,7 +212,7 @@ describe('@bugsnag/core/client', () => {
 
     it('prefers client.app.releaseStage over config.releaseStage', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', enabledReleaseStages: ['testing'], releaseStage: 'staging' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           expect(payload.events[0].app.releaseStage).toBe('testing')
           done()
@@ -224,7 +224,7 @@ describe('@bugsnag/core/client', () => {
 
     it('populates client.app.version if config.appVersion is supplied', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH', appVersion: '1.2.3' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendEvent: (payload) => {
           expect(payload.events[0].app.version).toBe('1.2.3')
           done()
@@ -236,7 +236,7 @@ describe('@bugsnag/core/client', () => {
     it('can handle all kinds of bad input', () => {
       const payloads = []
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
-      client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+      client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
 
       client.notify(undefined)
       client.notify(null)
@@ -261,7 +261,7 @@ describe('@bugsnag/core/client', () => {
     it('supports { name, message } usage', () => {
       const payloads = []
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
-      client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+      client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
       client.notify({ name: 'UnknownThing', message: 'found a thing that couldn’t be dealt with' })
 
       expect(payloads.length).toBe(1)
@@ -274,7 +274,7 @@ describe('@bugsnag/core/client', () => {
     it('leaves a breadcrumb of the error', () => {
       const payloads = []
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
-      client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+      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')
@@ -295,7 +295,7 @@ describe('@bugsnag/core/client', () => {
 
     it('should call the callback (success)', done => {
       const client = new Client({ apiKey: 'API_KEY' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendSession: () => {},
         sendEvent: (payload, cb) => cb(null)
       }))
@@ -309,7 +309,7 @@ describe('@bugsnag/core/client', () => {
 
     it('should call the callback (err)', done => {
       const client = new Client({ apiKey: 'API_KEY' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendSession: () => {},
         sendEvent: (payload, cb) => cb(new Error('flerp'))
       }))
@@ -324,7 +324,7 @@ describe('@bugsnag/core/client', () => {
 
     it('should call the callback even if the event doesn’t send (enabledReleaseStages)', done => {
       const client = new Client({ apiKey: 'API_KEY', enabledReleaseStages: ['production'], releaseStage: 'development' })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendSession: () => {},
         sendEvent: (payload, cb) => cb(null)
       }))
@@ -338,7 +338,7 @@ describe('@bugsnag/core/client', () => {
 
     it('should call the callback even if the event doesn’t send (onError)', done => {
       const client = new Client({ apiKey: 'API_KEY', onError: () => false })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendSession: () => {},
         sendEvent: (payload, cb) => cb(null)
       }))
@@ -352,7 +352,7 @@ describe('@bugsnag/core/client', () => {
 
     it('should attach the original error to the event object', done => {
       const client = new Client({ apiKey: 'API_KEY', onError: () => false })
-      client.delivery(client => ({
+      client._setDelivery(client => ({
         sendSession: () => {},
         sendEvent: (payload, cb) => cb(null)
       }))
@@ -418,40 +418,40 @@ describe('@bugsnag/core/client', () => {
     it('calls the provided the session delegate and return delegate’s return value', () => {
       const client = new Client({ apiKey: 'API_KEY' })
       let ret
-      client.sessionDelegate({
+      client._sessionDelegate = {
         startSession: c => {
           expect(c).toBe(client)
           ret = {}
           return ret
         }
-      })
+      }
       expect(client.startSession()).toBe(ret)
     })
 
     it('calls warns if a session delegate is not provided', (done) => {
       const client = new Client({ apiKey: 'API_KEY' })
-      client.logger({
+      client._logger = {
         debug: () => {},
         info: () => {},
         warn: (...args) => {
           expect(args[0]).toMatch(/^No session/)
           done()
         },
         error: () => {}
-      })
+      }
       client.startSession()
     })
 
     it('tracks error counts using the session delegate and sends them in error payloads', (done) => {
       const client = new Client({ apiKey: 'API_KEY' })
       let i = 0
-      client.sessionDelegate({
+      client._sessionDelegate = {
         startSession: (client) => {
           client._session = new Session()
           return client
         }
-      })
-      client.delivery(client => ({
+      }
+      client._setDelivery(client => ({
         sendSession: () => {},
         sendEvent: (payload, cb) => {
           if (++i < 10) return

diff --git a/packages/core/types/client.d.ts b/packages/core/types/client.d.ts
@@ -24,9 +24,6 @@ declare class Client {
 
   public use(plugin: common.Plugin, ...args: any[]): Client;
   public getPlugin(name: string): any;
-  public delivery(delivery: common.Delivery): Client;
-  public logger(logger: common.Logger): Client;
-  public sessionDelegate(sessionDelegate: common.SessionDelegate): Client;
   public notify(
     error: common.NotifiableError,
     onError?: common.OnError,

diff --git a/packages/core/types/common.d.ts b/packages/core/types/common.d.ts
@@ -40,22 +40,6 @@ export interface ConfigSchema {
   [key: string]: ConfigSchemaEntry;
 }
 
-export interface Delivery {
-  name: string;
-  sendEvent: (
-    logger: Logger,
-    config: any,
-    event: EventPayload,
-    cb?: (e: Error | null, resText: string) => void,
-  ) => void;
-  sendSession: (
-    logger: Logger,
-    config: any,
-    event: SessionPayload,
-    cb?: (e: Error | null, resText: string) => void,
-  ) => void;
-}
-
 export interface Logger {
   debug: (...args: any[]) => void;
   info: (...args: any[]) => void;

diff --git a/packages/expo/src/notifier.js b/packages/expo/src/notifier.js
@@ -45,7 +45,7 @@ module.exports = (opts) => {
 
   const bugsnag = new Client(opts, schema, { name, version, url })
 
-  bugsnag.delivery(delivery)
+  bugsnag._setDelivery(delivery)
 
   plugins.forEach(pl => {
     switch (pl.name) {

diff --git a/packages/node/src/notifier.js b/packages/node/src/notifier.js
@@ -43,7 +43,7 @@ module.exports = (opts, userPlugins = []) => {
 
   const bugsnag = new Client(opts, schema, { name, version, url })
 
-  bugsnag.delivery(delivery)
+  bugsnag._setDelivery(delivery)
 
   plugins.forEach(pl => bugsnag.use(pl))
 

diff --git a/packages/plugin-browser-context/test/context.test.js b/packages/plugin-browser-context/test/context.test.js
@@ -16,7 +16,7 @@ describe('plugin: context', () => {
     const payloads = []
     client.use(plugin, window)
 
-    client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+    client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
     client.notify(new Error('noooo'))
 
     expect(payloads.length).toEqual(1)
@@ -30,7 +30,7 @@ describe('plugin: context', () => {
 
     client.context = 'something else'
 
-    client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+    client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
     client.notify(new Error('noooo'))
 
     expect(payloads.length).toEqual(1)

diff --git a/packages/plugin-browser-device/test/device.test.js b/packages/plugin-browser-device/test/device.test.js
@@ -14,7 +14,7 @@ describe('plugin: device', () => {
 
     expect(client._config.onError.length).toBe(1)
 
-    client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+    client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
     client.notify(new Error('noooo'))
 
     const ISO_8601 = /^\d{4}(-\d\d(-\d\d(T\d\d:\d\d(:\d\d)?(\.\d+)?(([+-]\d\d:\d\d)|Z)?)?)?)?$/i

diff --git a/packages/plugin-browser-request/test/request.test.js b/packages/plugin-browser-request/test/request.test.js
@@ -12,7 +12,7 @@ describe('plugin: request', () => {
     const payloads = []
     client.use(plugin, window)
 
-    client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+    client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
     client.notify(new Error('noooo'))
 
     expect(payloads.length).toEqual(1)
@@ -26,7 +26,7 @@ describe('plugin: request', () => {
 
     client.request = { url: 'foobar' }
 
-    client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
+    client._setDelivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
     client.notify(new Error('noooo'))
 
     expect(payloads.length).toEqual(1)

diff --git a/packages/plugin-browser-session/session.js b/packages/plugin-browser-session/session.js
@@ -2,7 +2,7 @@ const { includes } = require('@bugsnag/core/lib/es-utils')
 const inferReleaseStage = require('@bugsnag/core/lib/infer-release-stage')
 
 module.exports = {
-  init: client => client.sessionDelegate(sessionDelegate)
+  init: client => { client._sessionDelegate = sessionDelegate }
 }
 
 const sessionDelegate = {