[PLAT-6174] feat: Ensure server plugins honour autoDetectErrors option

.buildkite/node-pipeline.yml

@@ -25,7 +25,7 @@ steps:
         run: node-maze-runner
         use-aliases: true
         verbose: true
-        command: ["-e", "koa.feature", "-e", "koa-1x.feature", "-e", "webpack.feature"]
+        command: ["-e", "koa.feature", "-e", "koa-1x.feature", "-e", "koa_disabled.feature", "-e", "webpack.feature"]
     env:
       NODE_VERSION: "4"
 
@@ -37,7 +37,7 @@ steps:
         run: node-maze-runner
         use-aliases: true
         verbose: true
-        command: ["-e", "koa.feature", "-e", "koa-1x.feature"]
+        command: ["-e", "koa.feature", "-e", "koa-1x.feature", "-e", "koa_disabled.feature"]
     env:
       NODE_VERSION: "6"
 

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## TBC
 
+### Added
+
+- Server framework plugins now honour the `autoDetectErrors` configuration option [#1464](https://github.com/bugsnag/bugsnag-js/pull/1464)
+
 ### Changed
 
 - (react-native): Update bugsnag-cocoa to v6.10.1

packages/plugin-express/src/express.js

@@ -31,6 +31,8 @@ module.exports = {
         requestClient.addMetadata('request', metadata)
       }, true)
 
+      if (!client._config.autoDetectErrors) return next()
+
       // unhandled errors caused by this request
       dom.on('error', (err) => {
         const event = client.Event.create(err, false, handledState, 'express middleware', 1)
@@ -48,6 +50,8 @@ module.exports = {
     }
 
     const errorHandler = (err, req, res, next) => {
+      if (!client._config.autoDetectErrors) return next(err)
+
       const event = client.Event.create(err, false, handledState, 'express middleware', 1)
 
       const { metadata, request } = getRequestAndMetadataFromReq(req)

packages/plugin-koa/src/koa.js

@@ -27,6 +27,8 @@ module.exports = {
         event.addMetadata('request', metadata)
       }, true)
 
+      if (!client._config.autoDetectErrors) return next()
+
       try {
         await next()
       } catch (err) {
@@ -59,6 +61,8 @@ module.exports = {
         event.request = { ...event.request, ...request }
       }, true)
 
+      if (!client._config.autoDetectErrors) return next()
+
       try {
         yield next
       } catch (err) {
@@ -71,6 +75,8 @@ module.exports = {
     }
 
     const errorHandler = (err, ctx) => {
+      if (!client._config.autoDetectErrors) return
+
       const event = client.Event.create(err, false, handledState, 'koa middleware', 1)
 
       const { metadata, request } = getRequestAndMetadataFromCtx(ctx)

packages/plugin-restify/src/restify.js

@@ -30,6 +30,8 @@ module.exports = {
         event.request = { ...event.request, ...request }
       }, true)
 
+      if (!client._config.autoDetectErrors) return next()
+
       // unhandled errors caused by this request
       dom.on('error', (err) => {
         const event = client.Event.create(err, false, handledState, 'restify middleware', 1)
@@ -51,6 +53,7 @@ module.exports = {
     }
 
     const errorHandler = (req, res, err, cb) => {
+      if (!client._config.autoDetectErrors) return cb()
       if (err.statusCode && err.statusCode < 500) return cb()
 
       const event = client.Event.create(err, false, handledState, 'restify middleware', 1)

test/node/features/express_disabled.feature

@@ -0,0 +1,54 @@
+Feature: @bugsnag/plugin-express autoDetectErrors=false
+
+Background:
+  Given I store the api key in the environment variable "BUGSNAG_API_KEY"
+  And I store the notify endpoint in the environment variable "BUGSNAG_NOTIFY_ENDPOINT"
+  And I store the sessions endpoint in the environment variable "BUGSNAG_SESSIONS_ENDPOINT"
+  And I start the service "express-disabled"
+  And I wait for the host "express-disabled" to open port "80"
+
+Scenario: a synchronous thrown error in a route
+  Then I open the URL "http://express-disabled/sync"
+  And I should receive no errors
+
+Scenario: an asynchronous thrown error in a route
+  Then I open the URL "http://express-disabled/async" tolerating any error
+  And I should receive no errors
+
+Scenario: an error passed to next(err)
+  Then I open the URL "http://express-disabled/next"
+  And I should receive no errors
+
+Scenario: a synchronous promise rejection in a route
+  Then I open the URL "http://express-disabled/rejection-sync"
+  And I should receive no errors
+
+Scenario: an asynchronous promise rejection in a route
+  Then I open the URL "http://express-disabled/rejection-async"
+  And I should receive no errors
+
+Scenario: a string passed to next(err)
+  Then I open the URL "http://express-disabled/string-as-error"
+  And I should receive no errors
+
+Scenario: throwing non-Error error
+  Then I open the URL "http://express-disabled/throw-non-error"
+  And I should receive no errors
+
+Scenario: a handled error passed to req.bugsnag.notify()
+  Then I open the URL "http://express-disabled/handled"
+  And I wait to receive an error
+  Then the error 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 exception "errorClass" equals "Error"
+  And the exception "message" equals "handled"
+  And the exception "type" equals "nodejs"
+  And the "file" of stack frame 0 equals "scenarios/app-disabled.js"
+  And the event "request.url" equals "http://express-disabled/handled"
+  And the event "request.httpMethod" equals "GET"
+  And the event "request.clientIp" is not null
+
+Scenario: adding body to request metadata
+  When I POST the data "data=in_request_body" to the URL "http://express-disabled/bodytest"
+  And I should receive no errors

test/node/features/fixtures/docker-compose.yml

@@ -91,6 +91,22 @@ services:
           - express
     restart: "no"
 
+  express-disabled:
+    build:
+      context: express
+      args:
+        - NODE_VERSION
+    environment:
+      - BUGSNAG_API_KEY
+      - BUGSNAG_NOTIFY_ENDPOINT
+      - BUGSNAG_SESSIONS_ENDPOINT
+    networks:
+      default:
+        aliases:
+          - express-disabled
+    restart: "no"
+    entrypoint: "node scenarios/app-disabled"
+
   restify:
     build:
       context: restify
@@ -106,6 +122,22 @@ services:
           - restify
     restart: "no"
 
+  restify-disabled:
+    build:
+      context: restify
+      args:
+        - NODE_VERSION
+    environment:
+      - BUGSNAG_API_KEY
+      - BUGSNAG_NOTIFY_ENDPOINT
+      - BUGSNAG_SESSIONS_ENDPOINT
+    networks:
+      default:
+        aliases:
+          - restify-disabled
+    restart: "no"
+    entrypoint: "node scenarios/app-disabled"
+
   koa:
     build:
       context: koa
@@ -121,6 +153,22 @@ services:
           - koa
     restart: "no"
 
+  koa-disabled:
+    build:
+      context: koa
+      args:
+        - NODE_VERSION
+    environment:
+      - BUGSNAG_API_KEY
+      - BUGSNAG_NOTIFY_ENDPOINT
+      - BUGSNAG_SESSIONS_ENDPOINT
+    networks:
+      default:
+        aliases:
+          - koa-disabled
+    restart: "no"
+    entrypoint: "node scenarios/app-disabled"
+
   koa-1x:
     build:
       context: koa-1x

test/node/features/fixtures/express/scenarios/app-disabled.js

@@ -0,0 +1,81 @@
+var Bugsnag = require('@bugsnag/node')
+var bugsnagExpress = require('@bugsnag/plugin-express')
+var express = require('express')
+var bodyParser = require('body-parser')
+
+Bugsnag.start({
+  apiKey: process.env.BUGSNAG_API_KEY,
+  endpoints: {
+    notify: process.env.BUGSNAG_NOTIFY_ENDPOINT,
+    sessions: process.env.BUGSNAG_SESSIONS_ENDPOINT
+  },
+  autoTrackSessions: false,
+  autoDetectErrors: false,
+  plugins: [bugsnagExpress]
+})
+
+
+var middleware = Bugsnag.getPlugin('express')
+
+var app = express()
+
+app.use(middleware.requestHandler)
+
+// If the server hasn't started sending something within 2 seconds
+// it probably won't. So end the request and hurry the failing test
+// along.
+app.use(function (req, res, next) {
+  setTimeout(function () {
+    if (!res.headersSent) return res.sendStatus(500)
+  }, 2000)
+  next()
+})
+
+app.get('/', function (req, res) {
+  res.end('ok')
+})
+
+app.get('/sync', function (req, res) {
+  throw new Error('sync')
+})
+
+app.get('/async', function (req, res) {
+  setTimeout(function () {
+    throw new Error('async')
+  }, 100)
+})
+
+app.get('/next', function (req, res, next) {
+  next(new Error('next'))
+})
+
+app.get('/rejection-sync', function (req, res, next) {
+  Promise.reject(new Error('reject sync')).catch(next)
+})
+
+app.get('/rejection-async', function (req, res, next) {
+  setTimeout(function () {
+    Promise.reject(new Error('reject async')).catch(next)
+  }, 100)
+})
+
+app.get('/string-as-error', function (req, res, next) {
+  next('errrr')
+})
+
+app.get('/throw-non-error', function (req, res, next) {
+  throw 1 // eslint-disable-line
+})
+
+app.get('/handled', function (req, res, next) {
+  req.bugsnag.notify(new Error('handled'))
+  res.end('OK')
+})
+
+app.post('/bodytest', bodyParser.urlencoded(), function (req, res, next) {
+  throw new Error('request body')
+})
+
+app.use(middleware.errorHandler)
+
+app.listen(80)

test/node/features/fixtures/koa/scenarios/app-disabled.js

@@ -0,0 +1,70 @@
+const Bugsnag = require('@bugsnag/node')
+const bugsnagKoa = require('@bugsnag/plugin-koa')
+const Koa = require('koa')
+const bodyParser = require('koa-bodyparser');
+
+Bugsnag.start({
+  apiKey: process.env.BUGSNAG_API_KEY,
+  endpoints: {
+    notify: process.env.BUGSNAG_NOTIFY_ENDPOINT,
+    sessions: process.env.BUGSNAG_SESSIONS_ENDPOINT
+  },
+  autoDetectErrors: false,
+  plugins: [bugsnagKoa]
+})
+
+
+const middleware = Bugsnag.getPlugin('koa')
+
+const app = new Koa()
+
+// If the server hasn't started sending something within 2 seconds
+// it probably won't. So end the request and hurry the failing test
+// along.
+app.use(async (ctx, next) => {
+  setTimeout(function () {
+    if (!ctx.response.headerSent) ctx.response.status = 500
+  }, 2000)
+  await next()
+})
+
+app.use(async (ctx, next) => {
+  if (ctx.path === '/error-before-handler') {
+    throw new Error('nope')
+  } else {
+    await next()
+  }
+})
+
+app.use(middleware.requestHandler)
+
+app.use(bodyParser());
+
+const erroneous = () => new Promise((resolve, reject) => reject(new Error('async noooop')))
+
+app.use(async (ctx, next) => {
+  if (ctx.path === '/') {
+    ctx.body = 'ok'
+  } else if (ctx.path === '/err') {
+    throw new Error('noooop')
+  } else if (ctx.path === '/async-err') {
+    await erroneous()
+  } else if (ctx.path === '/ctx-throw') {
+    ctx.throw(500, 'thrown')
+  } else if (ctx.path === '/ctx-throw-400') {
+    ctx.throw(400, 'thrown')
+  } else if (ctx.path === '/throw-non-error') {
+    throw 'error' // eslint-disable-line
+  } else if (ctx.path === '/handled') {
+    ctx.bugsnag.notify(new Error('handled'))
+    await next()
+  } else if (ctx.path === '/bodytest') {
+    throw new Error('request body')
+  } else {
+    await next()
+  }
+})
+
+app.on('error', middleware.errorHandler)
+
+app.listen(80)