Merge pull request #655 from bugsnag/v7-notify-signature

V7: Notify signature

CHANGELOG.md

@@ -9,6 +9,7 @@
 - Rename `notifyReleaseStages` -> `enabledReleaseStages` [#649](https://github.com/bugsnag/bugsnag-js/pull/649)
 - Remove individual breadcrumb flags in favour of `enabledBreadcrumbTypes`, rename `breadcrumb.{ name -> message, metaData -> metadata }`, and update `leaveBreadcrumb()` type signature to be more explicit [#650](https://github.com/bugsnag/bugsnag-js/pull/650)
 - Rename `beforeSend` -> `onError`, remove `event.ignore()` and refactor callback logic [#654](https://github.com/bugsnag/bugsnag-js/pull/654)
+- Update signature of `notify(err, opts?, cb?)` -> `notify(err, onError?, cb?)` for a canonical way to update events [#655](https://github.com/bugsnag/bugsnag-js/pull/655)
 
 ## 6.4.3 (2019-10-21)
 

examples/angular/src/app/app.component.ts

@@ -27,8 +27,8 @@ export class AppComponent {
       throw("Bad thing!");
     } catch (e) {
       // below modifies the handled error, and then sends it to your dashboard.
-      bugsnagClient.notify(e, {
-          context: 'Don\'t worry - I handled it!'
+      bugsnagClient.notify(e, function (event) {
+        event.context = 'Don\'t worry - I handled it!'
       });
     }
     // resets the button

examples/browser-cdn/app.js

@@ -77,15 +77,14 @@ function sendHandled () {
   } catch (e) {
     console.log('a handled error with metadata has been reported to your Bugsnag dashboard')
 
-    bugsnagClient.notify(e, {
-      context: 'a handled ReferenceError with metadata',
+    bugsnagClient.notify(e, event => {
+      event.context = 'a handled ReferenceError with metadata'
       // Note that metadata can be declared globally, in the notification (as below) or in an onError.
       // The below metadata will be supplemented (not replaced) by the metadata
       // in the onError method. See our docs if you prefer to overwrite/remove metadata.
-      metaData: {
-        details: {
-          info: 'Any important details specific to the context of this particular error/function.'}
-      }
+      event.addMetaData('details', {
+        info: 'Any important details specific to the context of this particular error/function.'
+      })
     })
   }
 }

examples/nuxtjs/modules/bugsnag/index.js

@@ -23,6 +23,6 @@ export default function (options) {
   this.nuxt.hook('render:setupMiddleware', app => app.use(bugsnagClient.getPlugin('express').requestHandler))
   this.nuxt.hook('render:errorMiddleware', app => app.use(bugsnagClient.getPlugin('express').errorHandler))
   this.nuxt.hook('generate:routeFailed', ({ route, errors }) => {
-    errors.forEach(({ error }) => bugsnagClient.notify(error, { metaData: { route } }))
+    errors.forEach(({ error }) => bugsnagClient.notify(error, event => { event.addMetaData({ route }) }))
   })
 }

examples/plain-node/app.js

@@ -73,10 +73,8 @@ function onError () {
   console.log('calling notify() with an onError callback…')
   // onError can be used to modify an event or prevent it from being sent at all
   // this example pseudo-randomly filters out approximately half of the events
-  bugsnagClient.notify(new Error('sometimes will send'), {
-    onError: (event) => {
-      const n = Math.random()
-      if (n <= 0.5) return false
-    }
+  bugsnagClient.notify(new Error('sometimes will send'), (event) => {
+    const n = Math.random()
+    if (n <= 0.5) return false
   })
 }

examples/react/src/components/BadButtons.js

@@ -15,8 +15,8 @@ class BadButtons extends React.Component {
       throw new Error('Bad Thing!')
     } catch (e) {
       console.log('a handled error was sent to our dashboard.')
-      bugsnagClient.notify(e, {
-        context: 'Don’t worry - I handled it.'
+      bugsnagClient.notify(e, event => {
+        event.context = 'Don’t worry - I handled it.'
       })
     }
   }

examples/typescript/src/app.ts

@@ -75,10 +75,8 @@ function onError () {
   console.log('calling notify() with an onError callback…')
   // onError can be used to modify an event or prevent it from being sent at all
   // this example pseudo-randomly filters out approximately half of the events
-  bugsnagClient.notify(new Error('sometimes will send'), {
-    onError: (event) => {
-      const n = Math.random()
-      if (n <= 0.5) return false
-    }
+  bugsnagClient.notify(new Error('sometimes will send'), (event) => {
+    const n = Math.random()
+    if (n <= 0.5) return false
   })
 }

packages/browser/types/test/fixtures/exposed-types.ts

@@ -1,8 +1,8 @@
 import { Bugsnag } from "../../..";
 let bugsnagInstance: Bugsnag.Client | undefined = undefined;
-export function notify(error: Bugsnag.NotifiableError, opts?: Bugsnag.NotifyOpts): void {
+export function notify(error: Bugsnag.NotifiableError, onError?: Bugsnag.OnError): void {
   if (bugsnagInstance === undefined) {
     return
   }
-  return bugsnagInstance.notify(error, opts)
+  return bugsnagInstance.notify(error, onError)
 }

packages/browser/types/test/fixtures/notify-callback.ts

@@ -1,7 +1,7 @@
 import bugsnag from "../../..";
 const bugsnagClient = bugsnag('api_key');
-bugsnagClient.notify(new Error('123'), {
-  onError: (event) => { return false }
+bugsnagClient.notify(new Error('123'), (event) => {
+  return false
 }, (err, event) => {
   console.log(event.originalError)
 })

packages/core/client.js

@@ -148,41 +148,31 @@ class BugsnagClient {
     return this
   }
 
-  notify (error, opts = {}, cb = () => {}) {
+  notify (error, onError, cb = () => {}) {
     if (!this._configured) throw new Error('client not configured')
 
     // releaseStage can be set via config.releaseStage or client.app.releaseStage
     const releaseStage = inferReleaseStage(this)
 
     // ensure we have an error (or a reasonable object representation of an error)
-    const { err, errorFramesToSkip, _opts } = normaliseError(error, opts, this._logger)
-    if (_opts) opts = _opts
-
-    // ensure opts is an object
-    if (typeof opts !== 'object' || opts === null) opts = {}
+    const { err, errorFramesToSkip } = normaliseError(error, this._logger)
 
     // create an event from the error, if it isn't one already
     const event = BugsnagEvent.ensureEvent(err, errorFramesToSkip, 2)
 
     event.app = { ...{ releaseStage }, ...event.app, ...this.app }
-    event.context = event.context || opts.context || this.context || undefined
-    event.device = { ...event.device, ...this.device, ...opts.device }
-    event.request = { ...event.request, ...this.request, ...opts.request }
-    event.user = { ...event.user, ...this.user, ...opts.user }
-    event.metaData = { ...event.metaData, ...this.metaData, ...opts.metaData }
+    event.context = event.context || this.context || undefined
+    event.device = { ...event.device, ...this.device }
+    event.request = { ...event.request, ...this.request }
+    event.user = { ...event.user, ...this.user }
+    event.metaData = { ...event.metaData, ...this.metaData }
     event.breadcrumbs = this.breadcrumbs.slice(0)
 
     if (this._session) {
       this._session.trackError(event)
       event.session = this._session
     }
 
-    // set severity if supplied
-    if (opts.severity !== undefined) {
-      event.severity = opts.severity
-      event._handledState.severityReason = { type: 'userSpecifiedSeverity' }
-    }
-
     // exit early if events should not be sent on the current releaseStage
     if (this.config.enabledReleaseStages.length > 0 && !includes(this.config.enabledReleaseStages, releaseStage)) {
       this._logger.warn('Event not sent due to releaseStage/enabledReleaseStages configuration')
@@ -191,14 +181,14 @@ class BugsnagClient {
 
     const originalSeverity = event.severity
 
-    const onError = [].concat(opts.onError).concat(this.config.onError)
     const onCallbackError = err => {
       // errors in callbacks are tolerated but we want to log them out
       this._logger.error('Error occurred in onError callback, continuing anyway…')
       this._logger.error(err)
     }
 
-    runCallbacks(onError, event, onCallbackError, (err, shouldSend) => {
+    const callbacks = [].concat(onError).concat(this.config.onError)
+    runCallbacks(callbacks, event, onCallbackError, (err, shouldSend) => {
       if (err) onCallbackError(err)
 
       if (!shouldSend) {
@@ -226,7 +216,7 @@ class BugsnagClient {
   }
 }
 
-const normaliseError = (error, opts, logger) => {
+const normaliseError = (error, logger) => {
   const synthesizedErrorFramesToSkip = 3
 
   const createAndLogUsageError = reason => {
@@ -237,18 +227,10 @@ const normaliseError = (error, opts, logger) => {
 
   let err
   let errorFramesToSkip = 0
-  let _opts
   switch (typeof error) {
     case 'string':
-      if (typeof opts === 'string') {
-        // ≤v3 used to have a notify('ErrorName', 'Error message') interface
-        // event usage/deprecation errors if this function is called like that
-        err = createAndLogUsageError('string/string')
-        _opts = { metaData: { notifier: { notifyArgs: [error, opts] } } }
-      } else {
-        err = new Error(String(error))
-        errorFramesToSkip = synthesizedErrorFramesToSkip
-      }
+      err = new Error(String(error))
+      errorFramesToSkip = synthesizedErrorFramesToSkip
       break
     case 'number':
     case 'boolean':
@@ -271,7 +253,7 @@ const normaliseError = (error, opts, logger) => {
     default:
       err = createAndLogUsageError('nothing')
   }
-  return { err, errorFramesToSkip, _opts }
+  return { err, errorFramesToSkip }
 }
 
 const hasNecessaryFields = error =>
@@ -282,7 +264,7 @@ const generateConfigErrorMessage = errors =>
   `Bugsnag configuration error\n${map(errors, (err) => `"${err.key}" ${err.message} \n    got ${stringify(err.value)}`).join('\n\n')}`
 
 const generateNotifyUsageMessage = actual =>
-  `notify() expected error/opts parameters, got ${actual}`
+  `notify(err) expected an error, got ${actual}`
 
 const stringify = val => typeof val === 'object' ? JSON.stringify(val) : String(val)
 

packages/core/test/client.test.js

@@ -112,23 +112,6 @@ describe('@bugsnag/core/client', () => {
       client.notify(new Error('oh em gee'))
     })
 
-    it('supports manually setting severity', done => {
-      const client = new Client(VALID_NOTIFIER)
-      client.delivery(client => ({
-        sendEvent: (payload) => {
-          expect(payload).toBeTruthy()
-          expect(Array.isArray(payload.events)).toBe(true)
-          const event = payload.events[0].toJSON()
-          expect(event.severity).toBe('error')
-          expect(event.severityReason).toEqual({ type: 'userSpecifiedSeverity' })
-          done()
-        }
-      }))
-      client.setOptions({ apiKey: 'API_KEY_YEAH' })
-      client.configure()
-      client.notify(new Error('oh em gee'), { severity: 'error' })
-    })
-
     it('supports setting severity via callback', done => {
       const client = new Client(VALID_NOTIFIER)
       client.delivery(client => ({
@@ -143,10 +126,8 @@ describe('@bugsnag/core/client', () => {
       }))
       client.setOptions({ apiKey: 'API_KEY_YEAH' })
       client.configure()
-      client.notify(new Error('oh em gee'), {
-        onError: event => {
-          event.severity = 'info'
-        }
+      client.notify(new Error('oh em gee'), event => {
+        event.severity = 'info'
       })
     })
 
@@ -160,7 +141,7 @@ describe('@bugsnag/core/client', () => {
       client.setOptions({ apiKey: 'API_KEY_YEAH' })
       client.configure()
 
-      client.notify(new Error('oh em gee'), { onError: event => false })
+      client.notify(new Error('oh em gee'), event => false)
 
       // give the event loop a tick to see if the event gets sent
       process.nextTick(() => done())
@@ -309,13 +290,13 @@ describe('@bugsnag/core/client', () => {
       client.notify('str1', 'str2')
       client.notify('str1', null)
 
-      expect(payloads[0].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify() expected error/opts parameters, got nothing')
-      expect(payloads[1].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify() expected error/opts parameters, got null')
-      expect(payloads[2].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify() expected error/opts parameters, got function')
-      expect(payloads[3].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify() expected error/opts parameters, got unsupported object')
+      expect(payloads[0].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify(err) expected an error, got nothing')
+      expect(payloads[1].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify(err) expected an error, got null')
+      expect(payloads[2].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify(err) expected an error, got function')
+      expect(payloads[3].events[0].toJSON().exceptions[0].message).toBe('Bugsnag usage error. notify(err) expected an error, got unsupported object')
       expect(payloads[4].events[0].toJSON().exceptions[0].message).toBe('1')
       expect(payloads[5].events[0].toJSON().exceptions[0].message).toBe('errrororor')
-      expect(payloads[6].events[0].toJSON().metaData).toEqual({ notifier: { notifyArgs: ['str1', 'str2'] } })
+      expect(payloads[6].events[0].toJSON().exceptions[0].message).toBe('str1')
       expect(payloads[7].events[0].toJSON().exceptions[0].message).toBe('str1')
       expect(payloads[7].events[0].toJSON().metaData).toEqual({})
     })
@@ -355,10 +336,8 @@ describe('@bugsnag/core/client', () => {
       client.setOptions({ apiKey: 'API_KEY_YEAH' })
       client.configure()
       client.metaData = { foo: [1, 2, 3] }
-      client.notify(new Error('changes afoot'), {
-        onError: (event) => {
-          event.updateMetaData('foo', '3', 1)
-        }
+      client.notify(new Error('changes afoot'), (event) => {
+        event.updateMetaData('foo', '3', 1)
       })
       expect(client.metaData.foo['3']).toBe(undefined)
     })

packages/core/types/client.d.ts

@@ -24,7 +24,7 @@ declare class Client {
   public sessionDelegate(sessionDelegate: common.SessionDelegate): Client;
   public notify(
     error: common.NotifiableError,
-    opts?: common.NotifyOpts,
+    onError?: common.OnError,
     cb?: (err: any, event: Event) => void,
   ): void;
   public leaveBreadcrumb(message: string, metadata?: { [key: string]: common.BreadcrumbMetadataValue }, type?: string): Client;

packages/expo/types/test/fixtures/exposed-types.ts

@@ -1,8 +1,8 @@
 import { Bugsnag } from "../../..";
 let bugsnagInstance: Bugsnag.Client | undefined = undefined;
-export function notify(error: Bugsnag.NotifiableError, opts?: Bugsnag.NotifyOpts): void {
+export function notify(error: Bugsnag.NotifiableError, onError?: Bugsnag.OnError): void {
   if (bugsnagInstance === undefined) {
     return
   }
-  return bugsnagInstance.notify(error, opts)
+  return bugsnagInstance.notify(error, onError)
 }

packages/plugin-client-ip/test/client-ip.test.js

@@ -14,9 +14,7 @@ describe('plugin: ip', () => {
     client.use(plugin)
 
     client.delivery(client => ({ sendEvent: (payload) => payloads.push(payload) }))
-    client.notify(new Error('noooo'), {
-      onError: event => { event.request = { some: 'detail' } }
-    })
+    client.notify(new Error('noooo'), event => { event.request = { some: 'detail' } })
 
     expect(payloads.length).toEqual(1)
     expect(payloads[0].events[0].request).toEqual({ some: 'detail' })

packages/plugin-contextualize/contextualize.js

@@ -6,7 +6,7 @@ const { getStack, maybeUseFallbackStack } = require('@bugsnag/core/lib/node-fall
 module.exports = {
   name: 'contextualize',
   init: client => {
-    const contextualize = (fn, opts) => {
+    const contextualize = (fn, onError) => {
       // capture a stacktrace in case a resulting error has nothing
       const fallbackStack = getStack()
 
@@ -19,7 +19,7 @@ module.exports = {
           unhandled: true,
           severityReason: { type: 'unhandledException' }
         })
-        client.notify(event, opts, (e, event) => {
+        client.notify(event, onError, (e, event) => {
           if (e) client._logger.error('Failed to send event to Bugsnag')
           client.config.onUncaughtException(err, event, client._logger)
         })

packages/plugin-contextualize/test/contextualize.test.js

@@ -56,13 +56,13 @@ describe('plugin: contextualize', () => {
       load(8, (err) => {
         if (err) throw err
       })
-    }, {
-      user: {
+    }, (event) => {
+      event.user = {
         id: '1a2c3cd4',
         name: 'Ben Gourley',
         email: 'ben.gourley@bugsnag.com'
-      },
-      severity: 'warning'
+      }
+      event.severity = 'warning'
     })
   })
 

packages/plugin-express/src/express.js

@@ -32,7 +32,7 @@ module.exports = {
 
       // unhandled errors caused by this request
       dom.on('error', (err) => {
-        req.bugsnag.notify(createEventFromErr(err, handledState), {}, (e, event) => {
+        req.bugsnag.notify(createEventFromErr(err, handledState), () => {}, (e, event) => {
           if (e) client._logger.error('Failed to send event to Bugsnag')
           req.bugsnag.config.onUncaughtException(err, event, client._logger)
         })
@@ -52,7 +52,11 @@ module.exports = {
         client._logger.warn(
           'req.bugsnag is not defined. Make sure the @bugsnag/plugin-express requestHandler middleware is added first.'
         )
-        client.notify(createEventFromErr(err, handledState, getRequestAndMetaDataFromReq(req)))
+        client.notify(createEventFromErr(err, handledState), (event) => {
+          const { metaData, request } = getRequestAndMetaDataFromReq(req)
+          event.request = { ...request }
+          event.metaData = { ...metaData }
+        })
       }
       next(err)
     }