Merge pull request #1879 from bugsnag/server-metadata-fix

Fix parts of request metadata being missing from some events

CHANGELOG.md

@@ -6,6 +6,10 @@
 
 - (react-native) Update bugsnag-android from v5.28.1 to [v5.28.3](https://github.com/bugsnag/bugsnag-android/blob/master/CHANGELOG.md#5283-2022-11-16)
 
+### Fixed
+
+- (plugin-express|plugin-koa|plugin-restify) Fix parts of request metadata being missing from some events [#1879](https://github.com/bugsnag/bugsnag-js/pull/1879)
+
 ## v7.18.2 (2022-11-01)
 
 ### Changed

packages/plugin-express/src/express.js

@@ -28,7 +28,7 @@ module.exports = {
       requestClient.addOnError((event) => {
         const { metadata, request } = getRequestAndMetadataFromReq(req)
         event.request = { ...event.request, ...request }
-        requestClient.addMetadata('request', metadata)
+        event.addMetadata('request', metadata)
       }, true)
 
       if (!client._config.autoDetectErrors) return next()

packages/plugin-koa/src/koa.js

@@ -41,7 +41,7 @@ module.exports = {
       requestClient.addOnError((event) => {
         const { request, metadata } = getRequestAndMetadataFromCtx(this)
         event.request = { ...event.request, ...request }
-        requestClient.addMetadata('request', metadata)
+        event.addMetadata('request', metadata)
       }, true)
 
       yield next

packages/plugin-restify/src/restify.js

@@ -27,7 +27,7 @@ module.exports = {
       requestClient.addOnError((event) => {
         const { request, metadata } = getRequestAndMetadataFromReq(req)
         event.request = { ...event.request, ...request }
-        requestClient.addMetadata('request', metadata)
+        event.addMetadata('request', metadata)
       }, true)
 
       if (!client._config.autoDetectErrors) return next()

test/node/features/express.feature

@@ -8,19 +8,23 @@ Background:
   And I wait for the host "express" to open port "80"
 
 Scenario: a synchronous thrown error in a route
-  Then I open the URL "http://express/sync"
+  Then I open the URL "http://express/sync/hello?a=1&b=2"
   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 true
   And the event "severity" equals "error"
   And the event "severityReason.type" equals "unhandledErrorMiddleware"
   And the exception "errorClass" equals "Error"
-  And the exception "message" equals "sync"
+  And the exception "message" equals "hello"
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/app.js"
-  And the event "request.url" equals "http://express/sync"
+  And the event "request.url" equals "http://express/sync/hello?a=1&b=2"
   And the event "request.httpMethod" equals "GET"
   And the event "request.clientIp" is not null
+  And the event "metaData.request.path" equals "/sync/hello"
+  And the event "metaData.request.query.a" equals "1"
+  And the event "metaData.request.query.b" equals "2"
+  And the event "metaData.request.connection" is not null
 
 Scenario: an asynchronous thrown error in a route
   Then I open the URL "http://express/async"
@@ -33,6 +37,7 @@ Scenario: an asynchronous thrown error in a route
   And the exception "message" equals "async"
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/app.js"
+  And the event "metaData.request.query" is null
 
 Scenario: an error passed to next(err)
   Then I open the URL "http://express/next"

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

@@ -38,8 +38,8 @@ app.get('/', function (req, res) {
   res.end('ok')
 })
 
-app.get('/sync', function (req, res) {
-  throw new Error('sync')
+app.get('/sync/:message', function (req, res) {
+  throw new Error(req.params.message)
 })
 
 app.get('/async', function (req, res) {

test/node/features/fixtures/restify/scenarios/app.js

@@ -18,7 +18,8 @@ var server = restify.createServer()
 
 server.pre(middleware.requestHandler)
 
-server.use(restify.plugins.bodyParser());
+server.use(restify.plugins.bodyParser())
+server.use(restify.plugins.queryParser())
 
 // If the server hasn't started sending something within 2 seconds
 // it probably won't. So end the request and hurry the failing test
@@ -34,8 +35,8 @@ server.get('/', function (req, res) {
   res.end('ok')
 })
 
-server.get('/sync', function (req, res) {
-  throw new Error('sync')
+server.get('/sync/:message', function (req, res) {
+  throw new Error(req.params.message)
 })
 
 server.get('/async', function (req, res) {

test/node/features/koa-1x.feature

@@ -9,7 +9,7 @@ Background:
   And I wait for the host "koa-1x" to open port "80"
 
 Scenario: a synchronous thrown error in a route
-  Then I open the URL "http://koa-1x/err"
+  Then I open the URL "http://koa-1x/err?a=1&b=2"
   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 true
@@ -19,8 +19,12 @@ Scenario: a synchronous thrown error in a route
   And the exception "message" equals "noooop"
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/app.js"
-  And the event "request.url" equals "http://koa-1x/err"
+  And the event "request.url" equals "http://koa-1x/err?a=1&b=2"
   And the event "request.httpMethod" equals "GET"
+  And the event "metaData.request.path" equals "/err?a=1&b=2"
+  And the event "metaData.request.query.a" equals "1"
+  And the event "metaData.request.query.b" equals "2"
+  And the event "metaData.request.connection" is not null
 
 Scenario: An error created with with ctx.throw()
   Then I open the URL "http://koa-1x/ctx-throw"

test/node/features/koa.feature

@@ -9,7 +9,7 @@ Background:
   And I wait for the host "koa" to open port "80"
 
 Scenario: a synchronous thrown error in a route
-  Then I open the URL "http://koa/err" and get a 500 response
+  Then I open the URL "http://koa/err?a=1&b=2" and get a 500 response
   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 true
@@ -19,11 +19,15 @@ Scenario: a synchronous thrown error in a route
   And the exception "message" equals "noooop"
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/app.js"
-  And the event "request.url" equals "http://koa/err"
+  And the event "request.url" equals "http://koa/err?a=1&b=2"
   And the event "request.httpMethod" equals "GET"
   And the event "request.clientIp" is not null
   And the event "metaData.error_handler.before" is true
   And the event "metaData.error_handler.after" is null
+  And the event "metaData.request.path" equals "/err?a=1&b=2"
+  And the event "metaData.request.query.a" equals "1"
+  And the event "metaData.request.query.b" equals "2"
+  And the event "metaData.request.connection" is not null
 
 Scenario: an asynchronous thrown error in a route
   Then I open the URL "http://koa/async-err" and get a 500 response

test/node/features/restify.feature

@@ -8,19 +8,25 @@ Background:
   And I wait for the host "restify" to open port "80"
 
 Scenario: a synchronous thrown error in a route
-  Then I open the URL "http://restify/sync"
+  Then I open the URL "http://restify/sync/hello?a=1&b=2&c=3"
   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 true
   And the event "severity" equals "error"
   And the event "severityReason.type" equals "unhandledErrorMiddleware"
   And the exception "errorClass" equals "Error"
-  And the exception "message" equals "sync"
+  And the exception "message" equals "hello"
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/app.js"
-  And the event "request.url" equals "http://restify/sync"
+  And the event "request.url" equals "http://restify/sync/hello"
   And the event "request.httpMethod" equals "GET"
   And the event "request.clientIp" is not null
+  And the event "metaData.request.path" equals "/sync/hello"
+  And the event "metaData.request.query.a" equals "1"
+  And the event "metaData.request.query.b" equals "2"
+  And the event "metaData.request.query.c" equals "3"
+  And the event "metaData.request.connection" is not null
+  And the event "metaData.request.params.message" equals "hello"
 
 Scenario: an asynchronous thrown error in a route
   Then I open the URL "http://restify/async"
@@ -33,6 +39,7 @@ Scenario: an asynchronous thrown error in a route
   And the exception "message" equals "async"
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/app.js"
+  And the event "metaData.request.query" is null
 
 Scenario: an error passed to next(err)
   Then I open the URL "http://restify/next"
@@ -101,4 +108,4 @@ Scenario: adding body to request metadata
   And the exception "type" equals "nodejs"
   And the "file" of stack frame 0 equals "scenarios/app.js"
   And the event "request.body.data" equals "in_request_body"
-  And the event "request.httpMethod" equals "POST"
+  And the event "request.httpMethod" equals "POST"