Improve normaliseError() utility in Client class

packages/core/client.js

@@ -157,18 +157,11 @@ class BugsnagClient {
     let { err, errorFramesToSkip, _opts } = normaliseError(error, opts, this._logger)
     if (_opts) opts = _opts
 
-    // if we have something falsey at this point, report usage error
-    if (!err) {
-      const msg = generateNotifyUsageMessage('nothing')
-      this._logger.warn(`${LOG_USAGE_ERR_PREFIX} ${msg}`)
-      err = new Error(`${REPORT_USAGE_ERR_PREFIX} ${msg}`)
-    }
-
     // ensure opts is an object
     if (typeof opts !== 'object' || opts === null) opts = {}
 
     // create a report from the error, if it isn't one already
-    const report = BugsnagReport.ensureReport(err, errorFramesToSkip, 1)
+    const report = BugsnagReport.ensureReport(err, errorFramesToSkip, 2)
 
     report.app = { ...{ releaseStage }, ...report.app, ...this.app }
     report.context = report.context || opts.context || this.context || undefined
@@ -234,6 +227,14 @@ class BugsnagClient {
 }
 
 const normaliseError = (error, opts, logger) => {
+  const synthesizedErrorFramesToSkip = 3
+
+  const createAndLogUsageError = reason => {
+    const msg = generateNotifyUsageMessage(reason)
+    logger.warn(`${LOG_USAGE_ERR_PREFIX} ${msg}`)
+    return new Error(`${REPORT_USAGE_ERR_PREFIX} ${msg}`)
+  }
+
   let err
   let errorFramesToSkip = 0
   let _opts
@@ -242,37 +243,33 @@ const normaliseError = (error, opts, logger) => {
       if (typeof opts === 'string') {
         // ≤v3 used to have a notify('ErrorName', 'Error message') interface
         // report usage/deprecation errors if this function is called like that
-        const msg = generateNotifyUsageMessage('string/string')
-        logger.warn(`${LOG_USAGE_ERR_PREFIX} ${msg}`)
-        err = new Error(`${REPORT_USAGE_ERR_PREFIX} ${msg}`)
+        err = createAndLogUsageError('string/string')
         _opts = { metaData: { notifier: { notifyArgs: [ error, opts ] } } }
       } else {
         err = new Error(String(error))
-        errorFramesToSkip += 2
+        errorFramesToSkip = synthesizedErrorFramesToSkip
       }
       break
     case 'number':
     case 'boolean':
       err = new Error(String(error))
       break
     case 'function':
-      const msg = generateNotifyUsageMessage('function')
-      logger.warn(`${LOG_USAGE_ERR_PREFIX} ${msg}`)
-      err = new Error(`${REPORT_USAGE_ERR_PREFIX} ${msg}`)
+      err = createAndLogUsageError('function')
       break
     case 'object':
       if (error !== null && (isError(error) || error.__isBugsnagReport)) {
         err = error
       } else if (error !== null && hasNecessaryFields(error)) {
         err = new Error(error.message || error.errorMessage)
         err.name = error.name || error.errorClass
-        errorFramesToSkip += 2
+        errorFramesToSkip = synthesizedErrorFramesToSkip
       } else {
-        const msg = generateNotifyUsageMessage('unsupported object')
-        logger.warn(`${LOG_USAGE_ERR_PREFIX} ${msg}`)
-        err = new Error(`${REPORT_USAGE_ERR_PREFIX} ${msg}`)
+        err = createAndLogUsageError(error === null ? 'null' : 'unsupported object')
       }
       break
+    default:
+      err = createAndLogUsageError('nothing')
   }
   return { err, errorFramesToSkip, _opts }
 }

packages/core/test/client.test.js

@@ -288,11 +288,9 @@ describe('@bugsnag/core/client', () => {
       client.notify('str1', 'str2')
       client.notify('str1', null)
 
-      payloads
-        .filter((p, i) => i < 3)
-        .map(p => p.events[0].toJSON().exceptions[0].message)
-        .forEach(message => expect(message).toMatch(/^Bugsnag usage error/))
-
+      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('1')
       expect(payloads[4].events[0].toJSON().exceptions[0].message).toBe('errrororor')
       expect(payloads[5].events[0].toJSON().metaData).toEqual({ notifier: { notifyArgs: [ 'str1', 'str2' ] } })
@@ -309,6 +307,8 @@ describe('@bugsnag/core/client', () => {
       expect(payloads.length).toBe(1)
       expect(payloads[0].events[0].toJSON().exceptions[0].errorClass).toBe('UnknownThing')
       expect(payloads[0].events[0].toJSON().exceptions[0].message).toBe('found a thing that couldn’t be dealt with')
+      expect(payloads[0].events[0].toJSON().exceptions[0].stacktrace[0].method).not.toMatch(/BugsnagClient/)
+      expect(payloads[0].events[0].toJSON().exceptions[0].stacktrace[0].file).not.toMatch(/core\/client\.js/)
     })
 
     it('leaves a breadcrumb of the error', () => {

test/browser/features/fixtures/handled/script/d.html

@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+    <script src="/node_modules/@bugsnag/browser/dist/bugsnag.min.js"></script>
+    <script type="text/javascript">
+      var ENDPOINT = decodeURIComponent(window.location.search.match(/ENDPOINT=([^&]+)/)[1])
+      var API_KEY = decodeURIComponent(window.location.search.match(/API_KEY=([^&]+)/)[1])
+      var bugsnagClient = bugsnag({
+        apiKey: API_KEY,
+        endpoint: ENDPOINT
+      })
+    </script>
+  </head>
+  <body>
+    <script>
+      function a () {
+        bugsnagClient.notify({ name: 'Errr', message: 'make a stacktrace for me' })
+      }
+      function b () { a() }
+      function c () { b() }
+      c()
+    </script>
+  </body>
+</html>

test/browser/features/fixtures/handled/script/e.html

@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+  <head>
+    <meta charset="utf-8" />
+    <script src="/node_modules/@bugsnag/browser/dist/bugsnag.min.js"></script>
+    <script type="text/javascript">
+      var ENDPOINT = decodeURIComponent(window.location.search.match(/ENDPOINT=([^&]+)/)[1])
+      var API_KEY = decodeURIComponent(window.location.search.match(/API_KEY=([^&]+)/)[1])
+      var bugsnagClient = bugsnag({
+        apiKey: API_KEY,
+        endpoint: ENDPOINT
+      })
+    </script>
+  </head>
+  <body>
+    <script>
+      function a () {
+        bugsnagClient.notify('make a stacktrace for me')
+      }
+      function b () { a() }
+      function c () { b() }
+      c()
+    </script>
+  </body>
+</html>

test/browser/features/handled_errors.feature

@@ -51,3 +51,25 @@ Scenario Outline: calling notify() with Error within Promise catch
     | browserify |
     | rollup     |
     | typescript |
+
+Scenario: calling notify() with an object, getting a generated a stacktrace
+  When I navigate to the URL "/handled/script/d.html"
+  Then I wait to receive a request
+  And the request is a valid browser payload for the error reporting API
+  And the exception "errorClass" equals "Errr"
+  And the exception "message" equals "make a stacktrace for me"
+  And the exception "type" equals "browserjs"
+
+  # this ensures the first generated stackframe doesn't come from bugsnag's source
+  And the payload field "events.0.exceptions.0.stacktrace.0.method" equals "a"
+
+Scenario: calling notify() with a string, getting a generated stacktrace
+  When I navigate to the URL "/handled/script/e.html"
+  Then I wait to receive a request
+  And the request is a valid browser payload for the error reporting API
+  And the exception "errorClass" equals "Error"
+  And the exception "message" equals "make a stacktrace for me"
+  And the exception "type" equals "browserjs"
+
+  # this ensures the first generated stackframe doesn't come from bugsnag's source
+  And the payload field "events.0.exceptions.0.stacktrace.0.method" equals "a"

test/node/features/fixtures/handled/scenarios/notify-string.js

@@ -0,0 +1,10 @@
+var bugsnag = require('@bugsnag/node')
+var bugsnagClient = bugsnag({
+  apiKey: process.env.BUGSNAG_API_KEY,
+  endpoints: {
+    notify: process.env.BUGSNAG_NOTIFY_ENDPOINT,
+    sessions: process.env.BUGSNAG_SESSIONS_ENDPOINT
+  }
+})
+
+bugsnagClient.notify('create an error for me')

test/node/features/handled_errors.feature

@@ -17,7 +17,7 @@ Scenario: calling notify() with an error
   And the "file" of stack frame 0 equals "scenarios/notify.js"
   And the "lineNumber" of stack frame 0 equals 10
 
-Scenario: calling notify() with am error from try/catch
+Scenario: calling notify() with an error from try/catch
   And I run the service "handled" with the command "node scenarios/notify-try-catch"
   And I wait to receive a request
   Then the request is valid for the error reporting API version "4" for the "Bugsnag Node" notifier
@@ -67,3 +67,16 @@ Scenario: using intercept to notify a promise rejection
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/intercept-rejection.js"
   And the "lineNumber" of stack frame 0 equals 21
+
+Scenario: calling notify with a string
+  And I run the service "handled" with the command "node scenarios/notify-string"
+  And I wait to receive a request
+  Then the request is valid for the error reporting API version "4" for the "Bugsnag Node" notifier
+  And the event "unhandled" is false
+  And the event "severity" equals "warning"
+  And the event "severityReason.type" equals "handledException"
+  And the exception "errorClass" equals "Error"
+  And the exception "message" equals "create an error for me"
+  And the exception "type" equals "nodejs"
+  And the "file" of stack frame 0 equals "scenarios/notify-string.js"
+  And the "lineNumber" of stack frame 0 equals 10