Improve logging and available information when error reports are not sent

packages/browser/types/test/types.test.js

@@ -51,9 +51,9 @@ bugsnag({
     const program = `
 import { Bugsnag } from "../../..";
 let bugsnagInstance: Bugsnag.Client | undefined = undefined;
-export function notify(error: Bugsnag.NotifiableError, opts?: Bugsnag.INotifyOpts): boolean {
+export function notify(error: Bugsnag.NotifiableError, opts?: Bugsnag.INotifyOpts): void {
   if (bugsnagInstance === undefined) {
-    return false
+    return
   }
   return bugsnagInstance.notify(error, opts)
 }
@@ -98,6 +98,24 @@ bugsnagClient.use({
   init: client => 10
 })
 console.log(bugsnagClient.getPlugin('foo') === 10)
+`.trim()
+    writeFileSync(`${__dirname}/fixtures/app.ts`, program)
+    const { stdout } = spawnSync('./node_modules/.bin/tsc', [
+      '--strict',
+      `${__dirname}/fixtures/app.ts`
+    ])
+    expect(stdout.toString()).toBe('')
+  })
+
+  it('should work with the notify() callback', () => {
+    const program = `
+import bugsnag from "../../..";
+const bugsnagClient = bugsnag('api_key');
+bugsnagClient.notify(new Error('123'), {
+  beforeSend: (report) => { return false }
+}, (err, report) => {
+  console.log(report.originalError)
+})
 `.trim()
     writeFileSync(`${__dirname}/fixtures/app.ts`, program)
     const { stdout } = spawnSync('./node_modules/.bin/tsc', [

packages/core/client.js

@@ -185,7 +185,7 @@ class BugsnagClient {
     // exit early if the reports should not be sent on the current releaseStage
     if (isArray(this.config.notifyReleaseStages) && !includes(this.config.notifyReleaseStages, releaseStage)) {
       this._logger.warn(`Report not sent due to releaseStage/notifyReleaseStages configuration`)
-      return false
+      return cb(null, report)
     }
 
     const originalSeverity = report.severity
@@ -201,7 +201,7 @@ class BugsnagClient {
 
       if (preventSend) {
         this._logger.debug(`Report not sent due to beforeSend callback`)
-        return false
+        return cb(null, report)
       }
 
       // only leave a crumb for the error if actually got sent

packages/core/lib/report-from-error.js

@@ -9,7 +9,8 @@ module.exports = (maybeError, handledState) => {
     actualError.name,
     actualError.message,
     Report.getStacktrace(actualError),
-    handledState
+    handledState,
+    maybeError
   )
   if (maybeError !== actualError) report.updateMetaData('error', 'non-error value', String(maybeError))
   return report

packages/core/report.js

@@ -4,7 +4,7 @@ const hasStack = require('./lib/has-stack')
 const { reduce, filter } = require('./lib/es-utils')
 
 class BugsnagReport {
-  constructor (errorClass, errorMessage, stacktrace = [], handledState = defaultHandledState()) {
+  constructor (errorClass, errorMessage, stacktrace = [], handledState = defaultHandledState(), originalError) {
     // duck-typing ftw >_<
     this.__isBugsnagReport = true
 
@@ -37,6 +37,7 @@ class BugsnagReport {
     }, [])
     this.user = undefined
     this.session = undefined
+    this.originalError = originalError
   }
 
   ignore () {
@@ -164,9 +165,9 @@ BugsnagReport.ensureReport = function (reportOrError, errorFramesToSkip = 0, gen
   if (reportOrError.__isBugsnagReport) return reportOrError
   try {
     const stacktrace = BugsnagReport.getStacktrace(reportOrError, errorFramesToSkip, 1 + generatedFramesToSkip)
-    return new BugsnagReport(reportOrError.name, reportOrError.message, stacktrace)
+    return new BugsnagReport(reportOrError.name, reportOrError.message, stacktrace, undefined, reportOrError)
   } catch (e) {
-    return new BugsnagReport(reportOrError.name, reportOrError.message, [])
+    return new BugsnagReport(reportOrError.name, reportOrError.message, [], undefined, reportOrError)
   }
 }
 

packages/core/test/client.test.js

@@ -177,8 +177,7 @@ describe('@bugsnag/core/client', () => {
       client.setOptions({ apiKey: 'API_KEY_YEAH', notifyReleaseStages: [] })
       client.configure()
 
-      const sent = client.notify(new Error('oh em eff gee'))
-      expect(sent).toBe(false)
+      client.notify(new Error('oh em eff gee'))
 
       // give the event loop a tick to see if the reports get send
       process.nextTick(() => done())
@@ -194,8 +193,7 @@ describe('@bugsnag/core/client', () => {
       client.setOptions({ apiKey: 'API_KEY_YEAH', releaseStage: 'staging', notifyReleaseStages: [ 'production' ] })
       client.configure()
 
-      const sent = client.notify(new Error('oh em eff gee'))
-      expect(sent).toBe(false)
+      client.notify(new Error('oh em eff gee'))
 
       // give the event loop a tick to see if the reports get send
       process.nextTick(() => done())
@@ -212,8 +210,7 @@ describe('@bugsnag/core/client', () => {
       client.configure()
       client.app.releaseStage = 'staging'
 
-      const sent = client.notify(new Error('oh em eff gee'))
-      expect(sent).toBe(false)
+      client.notify(new Error('oh em eff gee'))
 
       // give the event loop a tick to see if the reports get send
       process.nextTick(() => done())
@@ -338,6 +335,88 @@ describe('@bugsnag/core/client', () => {
       })
       expect(client.metaData.foo['3']).toBe(undefined)
     })
+
+    it('should call the callback (success)', done => {
+      const client = new Client(VALID_NOTIFIER)
+      client.setOptions({ apiKey: 'API_KEY' })
+      client.configure()
+      client.delivery({
+        sendSession: () => {},
+        sendReport: (logger, config, report, cb) => cb(null)
+      })
+      client.notify(new Error('111'), {}, (err, report) => {
+        expect(err).toBe(null)
+        expect(report).toBeTruthy()
+        expect(report.errorMessage).toBe('111')
+        done()
+      })
+    })
+
+    it('should call the callback (err)', done => {
+      const client = new Client(VALID_NOTIFIER)
+      client.setOptions({ apiKey: 'API_KEY' })
+      client.configure()
+      client.delivery({
+        sendSession: () => {},
+        sendReport: (logger, config, report, cb) => cb(new Error('flerp'))
+      })
+      client.notify(new Error('111'), {}, (err, report) => {
+        expect(err).toBeTruthy()
+        expect(err.message).toBe('flerp')
+        expect(report).toBeTruthy()
+        expect(report.errorMessage).toBe('111')
+        done()
+      })
+    })
+
+    it('should call the callback even if the report doesn’t send (notifyReleaseStages)', done => {
+      const client = new Client(VALID_NOTIFIER)
+      client.setOptions({ apiKey: 'API_KEY', notifyReleaseStages: [ 'production' ], releaseStage: 'development' })
+      client.configure()
+      client.delivery({
+        sendSession: () => {},
+        sendReport: (logger, config, report, cb) => cb(null)
+      })
+      client.notify(new Error('111'), {}, (err, report) => {
+        expect(err).toBe(null)
+        expect(report).toBeTruthy()
+        expect(report.errorMessage).toBe('111')
+        done()
+      })
+    })
+
+    it('should call the callback even if the report doesn’t send (beforeSend)', done => {
+      const client = new Client(VALID_NOTIFIER)
+      client.setOptions({ apiKey: 'API_KEY', beforeSend: () => false })
+      client.configure()
+      client.delivery({
+        sendSession: () => {},
+        sendReport: (logger, config, report, cb) => cb(null)
+      })
+      client.notify(new Error('111'), {}, (err, report) => {
+        expect(err).toBe(null)
+        expect(report).toBeTruthy()
+        expect(report.errorMessage).toBe('111')
+        done()
+      })
+    })
+
+    it('should attach the original error to the report object', done => {
+      const client = new Client(VALID_NOTIFIER)
+      client.setOptions({ apiKey: 'API_KEY', beforeSend: () => false })
+      client.configure()
+      client.delivery({
+        sendSession: () => {},
+        sendReport: (logger, config, report, cb) => cb(null)
+      })
+      const orig = new Error('111')
+      client.notify(orig, {}, (err, report) => {
+        expect(err).toBe(null)
+        expect(report).toBeTruthy()
+        expect(report.originalError).toBe(orig)
+        done()
+      })
+    })
   })
 
   describe('leaveBreadcrumb()', () => {

packages/core/types/client.d.ts

@@ -22,7 +22,11 @@ declare class Client {
   public delivery(delivery: common.IDelivery): Client;
   public logger(logger: common.ILogger): Client;
   public sessionDelegate(sessionDelegate: common.ISessionDelegate): Client;
-  public notify(error: common.NotifiableError, opts?: common.INotifyOpts): boolean;
+  public notify(
+    error: common.NotifiableError,
+    opts?: common.INotifyOpts,
+    cb?: (err: any, report: Report) => void,
+  ): void;
   public leaveBreadcrumb(name: string, metaData?: any, type?: string, timestamp?: string): Client;
   public startSession(): Client;
 }

packages/core/types/common.d.ts

@@ -20,7 +20,7 @@ export interface IConfig {
   [key: string]: any;
 }
 
-export type BeforeSend = (report: Report, cb?: (err: null | Error) => void) => void | Promise<void>;
+export type BeforeSend = (report: Report, cb?: (err: null | Error) => void) => void | Promise<void> | boolean;
 
 export interface IPlugin {
   name?: string;

packages/core/types/report.d.ts

@@ -32,8 +32,16 @@ declare class Report {
   public request: {
     url: string;
   };
+  public originalError: any;
+
+  constructor(
+    errorClass: string,
+    errorMessage: string,
+    stacktrace?: any[],
+    handledState?: IHandledState,
+    originalError?: any,
+  );
 
-  constructor(errorClass: string, errorMessage: string, stacktrace?: any[], handledState?: IHandledState);
   public isIgnored(): boolean;
   public ignore(): void;
   public updateMetaData(section: string, value: object): Report;

packages/node/src/config.js

@@ -30,15 +30,15 @@ module.exports = {
   },
   onUncaughtException: {
     defaultValue: () => (err, report, logger) => {
-      logger.error(`Reported an uncaught exception${getContext(report)}, the process will now terminate…\n${(err && err.stack) ? err.stack : err}`)
+      logger.error(`Uncaught exception${getContext(report)}, the process will now terminate…\n${(err && err.stack) ? err.stack : err}`)
       process.exit(1)
     },
     message: 'should be a function',
     validate: value => typeof value === 'function'
   },
   onUnhandledRejection: {
     defaultValue: () => (err, report, logger) => {
-      logger.error(`Reported an unhandled rejection${getContext(report)}…\n${(err && err.stack) ? err.stack : err}`)
+      logger.error(`Unhandled rejection${getContext(report)}…\n${(err && err.stack) ? err.stack : err}`)
     },
     message: 'should be a function',
     validate: value => typeof value === 'function'

packages/plugin-angular/src/index.ts

@@ -21,6 +21,7 @@ export class BugsnagErrorHandler extends ErrorHandler {
       error.message,
       this.bugsnagClient.BugsnagReport.getStacktrace(error),
       handledState,
+      error,
     );
 
     if (error.ngDebugContext) {

packages/plugin-react/src/index.js

@@ -15,7 +15,7 @@ module.exports = {
         const { beforeSend } = this.props
         const BugsnagReport = client.BugsnagReport
         const handledState = { severity: 'error', unhandled: true, severityReason: { type: 'unhandledException' } }
-        const report = new BugsnagReport(error.name, error.message, BugsnagReport.getStacktrace(error), handledState)
+        const report = new BugsnagReport(error.name, error.message, BugsnagReport.getStacktrace(error), handledState, error)
         if (info && info.componentStack) info.componentStack = formatComponentStack(info.componentStack)
         report.updateMetaData('react', info)
         client.notify(report, { beforeSend })

packages/plugin-vue/src/index.js

@@ -6,7 +6,7 @@ module.exports = {
 
     const handler = (err, vm, info) => {
       const handledState = { severity: 'error', unhandled: true, severityReason: { type: 'unhandledException' } }
-      const report = new client.BugsnagReport(err.name, err.message, client.BugsnagReport.getStacktrace(err), handledState)
+      const report = new client.BugsnagReport(err.name, err.message, client.BugsnagReport.getStacktrace(err), handledState, err)
 
       report.updateMetaData('vue', {
         errorInfo: info,

packages/plugin-window-onerror/onerror.js

@@ -27,7 +27,8 @@ module.exports = {
               error.name,
               error.message,
               decorateStack(client.BugsnagReport.getStacktrace(error), url, lineNo, charNo),
-              handledState
+              handledState,
+              error
             )
           } else {
             // otherwise, for non error values that were thrown, stringify it for
@@ -36,7 +37,8 @@ module.exports = {
               'window.onerror',
               String(error),
               decorateStack(client.BugsnagReport.getStacktrace(error, 1), url, lineNo, charNo),
-              handledState
+              handledState,
+              error
             )
             // include the raw input as metadata
             report.updateMetaData('window onerror', { error })
@@ -63,7 +65,8 @@ module.exports = {
             name,
             message,
             client.BugsnagReport.getStacktrace(new Error(), 1).slice(1),
-            handledState
+            handledState,
+            messageOrEvent
           )
           // include the raw input as metadata – it might contain more info than we extracted
           report.updateMetaData('window onerror', { event: messageOrEvent, extraParameters: url })
@@ -74,7 +77,8 @@ module.exports = {
             'window.onerror',
             String(messageOrEvent),
             decorateStack(client.BugsnagReport.getStacktrace(error, 1), url, lineNo, charNo),
-            handledState
+            handledState,
+            messageOrEvent
           )
           // include the raw input as metadata – it might contain more info than we extracted
           report.updateMetaData('window onerror', { event: messageOrEvent })

packages/plugin-window-unhandled-rejection/unhandled-rejection.js

@@ -29,7 +29,7 @@ exports.init = (client, win = window) => {
     let report
     if (error && hasStack(error)) {
       // if it quacks like an Error…
-      report = new client.BugsnagReport(error.name, error.message, ErrorStackParser.parse(error), handledState)
+      report = new client.BugsnagReport(error.name, error.message, ErrorStackParser.parse(error), handledState, error)
       if (isBluebird) {
         report.stacktrace = reduce(report.stacktrace, fixBluebirdStacktrace(error), [])
       }
@@ -40,7 +40,8 @@ exports.init = (client, win = window) => {
         error && error.name ? error.name : 'UnhandledRejection',
         error && error.message ? error.message : msg,
         [],
-        handledState
+        handledState,
+        error
       )
       // stuff the rejection reason into metaData, it could be useful
       report.updateMetaData('promise', 'rejection reason', serializableReason(error))