Allow onX callbacks with no arguments

Previously a callback like '() => false' would never be called due to the length check; it had to be written as '_ => false' instead
bugsnag · May 21, 2020 · fdec85f · fdec85f
1 parent 6a79c21
commit fdec85f
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 11 deletions.
diff --git a/packages/core/client.js b/packages/core/client.js
@@ -138,9 +138,9 @@ class Client {
     if (config.logger) this._logger = config.logger
 
     // add callbacks
-    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)
+    if (config.onError) this._cbs.e = this._cbs.e.concat(config.onError)
+    if (config.onBreadcrumb) this._cbs.b = this._cbs.b.concat(config.onBreadcrumb)
+    if (config.onSession) this._cbs.s = this._cbs.s.concat(config.onSession)
 
     // finally warn about any invalid config where we fell back to the default
     if (keys(errors).length) {

diff --git a/packages/core/test/client.test.ts b/packages/core/test/client.test.ts
@@ -159,16 +159,36 @@ describe('@bugsnag/core/client', () => {
       })
     })
     // eslint-disable-next-line jest/expect-expect
-    it('supports preventing send by returning false', done => {
+    it('supports preventing send by returning false in onError callback', done => {
+      const client = new Client({
+        apiKey: 'API_KEY_YEAH',
+        onError: () => false
+      })
+
+      client._setDelivery(client => ({
+        sendEvent: (payload) => {
+          done('sendEvent() should not be called')
+        },
+        sendSession: () => {}
+      }))
+
+      client.notify(new Error('oh em gee'))
+
+      // give the event loop a tick to see if the event gets sent
+      process.nextTick(() => done())
+    })
+    // eslint-disable-next-line jest/expect-expect
+    it('supports preventing send by returning false in notify callback', done => {
       const client = new Client({ apiKey: 'API_KEY_YEAH' })
+
       client._setDelivery(client => ({
         sendEvent: (payload) => {
           done('sendEvent() should not be called')
         },
         sendSession: () => {}
       }))
 
-      client.notify(new Error('oh em gee'), event => false)
+      client.notify(new Error('oh em gee'), () => false)
 
       // give the event loop a tick to see if the event gets sent
       process.nextTick(() => done())
@@ -362,10 +382,12 @@ 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._setDelivery(client => ({
         sendSession: () => {},
-        sendEvent: (payload, cb) => cb(null)
+        sendEvent: () => { done('sendEvent() should not be called') }
       }))
+
       // @ts-ignore
       client.notify(new Error('111'), {}, (err, event) => {
         expect(err).toBe(null)
@@ -377,10 +399,12 @@ 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._setDelivery(client => ({
         sendSession: () => {},
-        sendEvent: (payload, cb) => cb(null)
+        sendEvent: () => { done('sendEvent() should not be called') }
       }))
+
       // @ts-ignore
       client.notify(new Error('111'), {}, (err, event) => {
         expect(err).toBe(null)
@@ -462,7 +486,7 @@ describe('@bugsnag/core/client', () => {
       let calls = 0
       const client = new Client({
         apiKey: 'API_KEY_YEAH',
-        onBreadcrumb: b => {
+        onBreadcrumb: () => {
           calls++
           return false
         }
@@ -476,7 +500,7 @@ describe('@bugsnag/core/client', () => {
       let calls = 0
       const client = new Client({
         apiKey: 'API_KEY_YEAH',
-        onBreadcrumb: b => {
+        onBreadcrumb: () => {
           calls++
           throw new Error('uh oh')
         }
@@ -560,7 +584,7 @@ describe('@bugsnag/core/client', () => {
     })
 
     it('does not start the session if onSession returns false', () => {
-      const client = new Client({ apiKey: 'API_KEY', onSession: s => false })
+      const client = new Client({ apiKey: 'API_KEY', onSession: () => false })
       const sessionDelegate = {
         startSession: () => {},
         pauseSession: () => {},
@@ -577,7 +601,7 @@ describe('@bugsnag/core/client', () => {
     it('tolerates errors in onSession callbacks', () => {
       const client = new Client({
         apiKey: 'API_KEY',
-        onSession: s => {
+        onSession: () => {
           throw new Error('oh no')
         }
       })