Merge pull request #759 from bugsnag/v7-config-plugins

V7: Update config behaviour and plugin interface

CHANGELOG.md

@@ -34,6 +34,8 @@
 - Rename `filters` option to `redactedKeys` [#704](https://github.com/bugsnag/bugsnag-js/pull/704)
 - Rename `device.modelName` to `device.model` [#726](https://github.com/bugsnag/bugsnag-js/pull/726)
 - Rename `client.refresh()` to `client.resetEventCount()` [#727](https://github.com/bugsnag/bugsnag-js/pull/727)
+- `client.use(plugin)` has been removed and plugins must now be passed in to configuration [#759](https://github.com/bugsnag/bugsnag-js/pull/759)
+- Invalid configuration (except for `apiKey`) now falls back to default values rather than throwing an error [#759](https://github.com/bugsnag/bugsnag-js/pull/759)
 
 ### Removed
 - Remove non-public methods from `Client` interface: `logger()`, `delivery()` and `sessionDelegate()` [#659](https://github.com/bugsnag/bugsnag-js/pull/659)

UPGRADING.md

@@ -129,6 +129,9 @@ Many options have been renamed, reworked or replaced.
 +   onBreadcrumb: (breadcrumb) => {
 +     // a callback to run each time a breadcrumb is created
 +   }
+
+    // plugins must now be supplied in config rather than via client.use()
++   plugins: []
   }
 ```
 
@@ -364,6 +367,45 @@ Previously it was valid to supply a `notify` endpoint without supplying a `sessi
   }
 ```
 
+## Plugins
+
+Plugins must now be supplied in configuration, and the `client.use()` method has been removed. For users of the following plugins, some changes are required:
+
+### `@bugsnag/plugin-react`
+
+```diff
+- import bugsnagReact from '@bugsnag/plugin-react'
++ import BugsnagPluginReact from '@bugsnag/plugin-react'
+
+- const bugsnagClient = bugsnag('YOUR_API_KEY')
++ Bugsnag.start({ apiKey: 'YOUR_API_KEY', plugins: [new BugsnagPluginReact(React)] })
+
+- bugsnagClient.use(bugsnagReact, React)
+```
+
+### `@bugsnag/plugin-vue`
+
+```diff
+- import bugsnagVue from '@bugsnag/plugin-vue'
++ import BugsnagPluginVue from '@bugsnag/plugin-vue'
+
+- const bugsnagClient = bugsnag('YOUR_API_KEY')
++ Bugsnag.start({ apiKey: 'YOUR_API_KEY', plugins: [new BugsnagPluginVue(Vue)] })
+
+- bugsnagClient.use(bugsnagVue, Vue)
+```
+
+### `@bugsnag/plugin-{restify|koa|express}`
+
+Since these plugins don't need any input, their usage is simpler and follows this pattern
+
+```diff
+- bugsnagClient.use(plugin)
++ Bugsnag.start({
++   plugins: [plugin]
++ })
+```
+
 ---
 
 See the [full documentation](https://docs.bugsnag.com/platforms/javascript) for more information.

packages/browser/src/config.js

@@ -1,17 +1,14 @@
 const { schema } = require('@bugsnag/core/config')
 const map = require('@bugsnag/core/lib/es-utils/map')
 const assign = require('@bugsnag/core/lib/es-utils/assign')
-const stringWithLength = require('@bugsnag/core/lib/validators/string-with-length')
 
 module.exports = {
-  releaseStage: {
+  releaseStage: assign({}, schema.releaseStage, {
     defaultValue: () => {
       if (/^localhost(:\d+)?$/.test(window.location.host)) return 'development'
       return 'production'
-    },
-    message: 'should be set',
-    validate: stringWithLength
-  },
+    }
+  }),
   logger: assign({}, schema.logger, {
     defaultValue: () =>
       // set logger based on browser capability

packages/browser/src/notifier.js

@@ -40,30 +40,32 @@ const Bugsnag = {
     if (typeof opts === 'string') opts = { apiKey: opts }
     if (!opts) opts = {}
 
+    const internalPlugins = [
+      // add browser-specific plugins
+      pluginDevice(),
+      pluginContext(),
+      pluginRequest(),
+      pluginThrottle,
+      pluginSession,
+      pluginIp,
+      pluginStripQueryString,
+      pluginWindowOnerror(),
+      pluginUnhandledRejection(),
+      pluginNavigationBreadcrumbs(),
+      pluginInteractionBreadcrumbs(),
+      pluginNetworkBreadcrumbs(),
+      pluginConsoleBreadcrumbs,
+
+      // this one added last to avoid wrapping functionality before bugsnag uses it
+      pluginInlineScriptContent()
+    ]
+
     // configure a client with user supplied options
-    const bugsnag = new Client(opts, schema, { name, version, url })
+    const bugsnag = new Client(opts, schema, internalPlugins, { name, version, url })
 
     // set delivery based on browser capability (IE 8+9 have an XDomainRequest object)
     bugsnag._setDelivery(window.XDomainRequest ? dXDomainRequest : dXMLHttpRequest)
 
-    // add browser-specific plugins
-    bugsnag.use(pluginDevice)
-    bugsnag.use(pluginContext)
-    bugsnag.use(pluginRequest)
-    bugsnag.use(pluginThrottle)
-    bugsnag.use(pluginSession)
-    bugsnag.use(pluginIp)
-    bugsnag.use(pluginStripQueryString)
-    bugsnag.use(pluginWindowOnerror)
-    bugsnag.use(pluginUnhandledRejection)
-    bugsnag.use(pluginNavigationBreadcrumbs)
-    bugsnag.use(pluginInteractionBreadcrumbs)
-    bugsnag.use(pluginNetworkBreadcrumbs)
-    bugsnag.use(pluginConsoleBreadcrumbs)
-
-    // this one added last to avoid wrapping functionality before bugsnag uses it
-    bugsnag.use(pluginInlineScriptContent)
-
     bugsnag._logger.debug('Loaded!')
 
     return bugsnag._config.autoTrackSessions

packages/browser/types/test/fixtures/plugins.ts

@@ -1,7 +1,9 @@
 import Bugsnag from "../../..";
-Bugsnag.start('api_key');
-Bugsnag.use({
-  name: 'foobar',
-  init: client => 10
-})
+Bugsnag.start({
+  apiKey:'api_key',
+  plugins: [{
+    name: 'foobar',
+    load: client => 10
+  }]
+});
 console.log(Bugsnag.getPlugin('foo') === 10)

packages/core/client.d.ts

@@ -1,4 +1,4 @@
-import { Client, OnErrorCallback, Config, Breadcrumb, Session, OnSessionCallback, OnBreadcrumbCallback } from './types'
+import { Client, OnErrorCallback, Config, Breadcrumb, Session, OnSessionCallback, OnBreadcrumbCallback, Plugin } from './types'
 
 interface LoggerConfig {
   debug: (msg: any) => void
@@ -14,7 +14,7 @@ interface LoggerConfig {
  * module itself once it is converted to TypeScript.
  */
 export default class ClientWithInternals extends Client {
-  public constructor(opts: Config)
+  public constructor(opts: Config, schema?: {[key: string]: any}, internalPlugins?: Plugin[])
   _config: { [key: string]: {} }
   _logger: LoggerConfig
   _breadcrumbs: Breadcrumb[];

packages/core/client.js

@@ -5,6 +5,8 @@ const Session = require('./session')
 const map = require('./lib/es-utils/map')
 const includes = require('./lib/es-utils/includes')
 const filter = require('./lib/es-utils/filter')
+const reduce = require('./lib/es-utils/reduce')
+const keys = require('./lib/es-utils/keys')
 const assign = require('./lib/es-utils/assign')
 const runCallbacks = require('./lib/callback-runner')
 const metadataDelegate = require('./lib/metadata-delegate')
@@ -13,12 +15,11 @@ const runSyncCallbacks = require('./lib/sync-callback-runner')
 const noop = () => {}
 
 class Client {
-  constructor (configuration, schema = config.schema, notifier) {
+  constructor (configuration, schema = config.schema, internalPlugins = [], notifier) {
     // notifier id
     this._notifier = notifier
 
     // intialise opts and config
-    this._opts = configuration
     this._config = {}
     this._schema = schema
 
@@ -56,7 +57,10 @@ class Client {
     this.Breadcrumb = Breadcrumb
     this.Session = Session
 
-    this._extractConfiguration()
+    this._config = this._configure(configuration, internalPlugins)
+    map(internalPlugins.concat(this._config.plugins), pl => {
+      if (pl) this._loadPlugin(pl)
+    })
 
     // when notify() is called we need to know how many frames are from our own source
     // this inital value is 1 not 0 because we wrap notify() to ensure it is always
@@ -90,25 +94,53 @@ class Client {
     this._context = c
   }
 
-  _extractConfiguration (partialSchema = this._schema) {
-    const conf = config.mergeDefaults(this._opts, partialSchema)
-    const validity = config.validate(conf, partialSchema)
+  _configure (opts, internalPlugins) {
+    const schema = reduce(internalPlugins, (schema, plugin) => {
+      if (plugin && plugin.configSchema) return assign({}, schema, plugin.configSchema)
+      return schema
+    }, this._schema)
+
+    // accumulate configuration and error messages
+    const { errors, config } = reduce(keys(schema), (accum, key) => {
+      const defaultValue = schema[key].defaultValue(opts[key])
+
+      if (opts[key] !== undefined) {
+        const valid = schema[key].validate(opts[key])
+        if (!valid) {
+          accum.errors[key] = schema[key].message
+          accum.config[key] = defaultValue
+        } else {
+          accum.config[key] = opts[key]
+        }
+      } else {
+        accum.config[key] = defaultValue
+      }
 
-    if (!validity.valid === true) throw new Error(generateConfigErrorMessage(validity.errors))
+      return accum
+    }, { errors: {}, config: {} })
 
-    // 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
+    // missing api key is the only fatal error
+    if (schema.apiKey && !config.apiKey) {
+      throw new Error('No Bugsnag API Key set')
+    }
+
+    // update and elevate some options
+    this._metadata = config.metadata
+    this._user = config.user
+    this._context = config.context
+    if (config.logger) this._logger = config.logger
 
     // add callbacks
-    if (conf.onError && conf.onError.length) this._cbs.e = this._cbs.e.concat(conf.onError)
-    if (conf.onBreadcrumb && conf.onBreadcrumb.length) this._cbs.b = this._cbs.b.concat(conf.onBreadcrumb)
-    if (conf.onSession && conf.onSession.length) this._cbs.s = this._cbs.s.concat(conf.onSession)
+    if (config.onError && config.onError.length) this._cbs.e = this._cbs.e.concat(config.onError)
+    if (config.onBreadcrumb && config.onBreadcrumb.length) this._cbs.b = this._cbs.b.concat(config.onBreadcrumb)
+    if (config.onSession && config.onSession.length) this._cbs.s = this._cbs.s.concat(config.onSession)
 
-    // merge with existing config
-    this._config = assign({}, this._config, conf)
+    // finally warn about any invalid config where we fell back to the default
+    if (keys(errors).length) {
+      this._logger.warn(generateConfigErrorMessage(errors, opts))
+    }
+
+    return config
   }
 
   getUser () {
@@ -119,9 +151,8 @@ class Client {
     this._user = { id, email, name }
   }
 
-  use (plugin) {
-    if (plugin.configSchema) this._extractConfiguration(plugin.configSchema)
-    const result = plugin.init.apply(null, [this].concat([].slice.call(arguments, 1)))
+  _loadPlugin (plugin) {
+    const result = plugin.load(this)
     // JS objects are not the safest way to store arbitrarily keyed values,
     // so bookend the key with some characters that prevent tampering with
     // stuff like __proto__ etc. (only store the result if the plugin had a
@@ -286,9 +317,20 @@ class Client {
   }
 }
 
-const generateConfigErrorMessage = errors =>
-  `Bugsnag configuration error\n${map(errors, (err) => `"${err.key}" ${err.message} \n    got ${stringify(err.value)}`).join('\n\n')}`
+const generateConfigErrorMessage = (errors, rawInput) => {
+  const er = new Error(
+  `Invalid configuration\n${map(keys(errors), key => `  - ${key} ${errors[key]}, got ${stringify(rawInput[key])}`).join('\n\n')}`)
+  return er
+}
 
-const stringify = val => typeof val === 'object' ? JSON.stringify(val) : String(val)
+const stringify = val => {
+  switch (typeof val) {
+    case 'string':
+    case 'number':
+    case 'object':
+      return JSON.stringify(val)
+    default: return String(val)
+  }
+}
 
 module.exports = Client

packages/core/config.js

@@ -137,22 +137,13 @@ module.exports.schema = {
       isArray(value) && value.length === filter(value, s =>
         (typeof s === 'string' || (s && typeof s.test === 'function'))
       ).length
+  },
+  plugins: {
+    defaultValue: () => ([]),
+    message: 'should be an array of plugin objects',
+    validate: value =>
+      isArray(value) && value.length === filter(value, p =>
+        (p && typeof p === 'object' && typeof p.load === 'function')
+      ).length
   }
 }
-
-module.exports.mergeDefaults = (opts, schema) => {
-  if (!opts || !schema) throw new Error('opts and schema objects are required')
-  return reduce(keys(schema), (accum, key) => {
-    accum[key] = opts[key] !== undefined ? opts[key] : schema[key].defaultValue(opts[key], opts)
-    return accum
-  }, {})
-}
-
-module.exports.validate = (opts, schema) => {
-  if (!opts || !schema) throw new Error('opts and schema objects are required')
-  const errors = reduce(keys(schema), (accum, key) => {
-    if (schema[key].validate(opts[key], opts)) return accum
-    return accum.concat({ key, message: schema[key].message, value: opts[key] })
-  }, [])
-  return { valid: !errors.length, errors }
-}

packages/core/lib/clone-client.js

@@ -1,7 +1,7 @@
 const assign = require('./es-utils/assign')
 
 module.exports = (client) => {
-  const clone = new client.Client({}, {}, client._notifier)
+  const clone = new client.Client({}, {}, [], client._notifier)
 
   clone._config = client._config
 

packages/core/test/client.test.ts

@@ -29,16 +29,18 @@ describe('@bugsnag/core/client', () => {
 
   describe('use()', () => {
     it('supports plugins', done => {
-      const client = new Client({ apiKey: '123' })
-      client.use({
-        name: 'test plugin',
-        // @ts-ignore
-        description: 'nothing much to see here',
-        init: (c) => {
-          expect(c).toEqual(client)
-          done()
-        }
+      let pluginClient
+      const client = new Client({
+        apiKey: '123',
+        plugins: [{
+          name: 'test plugin',
+          load: (c) => {
+            pluginClient = c
+            done()
+          }
+        }]
       })
+      expect(pluginClient).toEqual(client)
     })
   })
 
@@ -442,7 +444,7 @@ describe('@bugsnag/core/client', () => {
   })
 
   describe('startSession()', () => {
-    it('calls the provided the session delegate and return delegate’s return value', () => {
+    it('calls the provided session delegate and return delegate’s return value', () => {
       const client = new Client({ apiKey: 'API_KEY' })
       let ret
       client._sessionDelegate = {