fix(node): Skip capturing Hapi Boom error responses. (#11151)

Resolves: #11069

After checking the behaviour, it seems to me that we don't need to
capture any Boom responses, as the errors that may cause a `5xx`
response are already captured by the core logic. To add an option to
control this behaviour, we need to update the usage of
`hapiErrorPlugin`, converting it to a function signature, which IMO may
not worth doing, as I'm not sure if users in general would need to use
it.

This also adds `expectError()` to the `node-integration-tests` runner to
allow `5xx` responses to be tested.

Author: onurtemizkan

dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js

@@ -10,6 +10,7 @@ Sentry.init({
 });
 
 const Hapi = require('@hapi/hapi');
+const Boom = require('@hapi/boom');
 
 const port = 5999;
 
@@ -27,6 +28,31 @@ const init = async () => {
     },
   });
 
+  server.route({
+    method: 'GET',
+    path: '/error',
+    handler: (_request, _h) => {
+      return new Error('Sentry Test Error');
+    },
+  });
+
+  server.route({
+    method: 'GET',
+    path: '/boom-error',
+    handler: (_request, _h) => {
+      return new Boom.Boom('Sentry Test Error');
+    },
+  });
+
+  server.route({
+    method: 'GET',
+    path: '/promise-error',
+    handler: async (_request, _h) => {
+      return Promise.reject(new Error('Sentry Test Error'));
+    },
+  });
+
+  await Sentry.setupHapiErrorHandler(server);
   await server.start();
 
   sendPortToRunner(port);

dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts

@@ -26,10 +26,45 @@ conditionalTest({ min: 14 })('hapi auto-instrumentation', () => {
     ]),
   };
 
+  const EXPECTED_ERROR_EVENT = {
+    exception: {
+      values: [
+        {
+          type: 'Error',
+          value: 'Sentry Test Error',
+        },
+      ],
+    },
+  };
+
   test('CJS - should auto-instrument `@hapi/hapi` package.', done => {
     createRunner(__dirname, 'scenario.js')
       .expect({ transaction: EXPECTED_TRANSACTION })
       .start(done)
       .makeRequest('get', '/');
   });
+
+  test('CJS - should handle returned plain errors in routes.', done => {
+    createRunner(__dirname, 'scenario.js')
+      .expect({ event: EXPECTED_ERROR_EVENT })
+      .expectError()
+      .start(done)
+      .makeRequest('get', '/error');
+  });
+
+  test('CJS - should handle returned Boom errors in routes.', done => {
+    createRunner(__dirname, 'scenario.js')
+      .expect({ event: EXPECTED_ERROR_EVENT })
+      .expectError()
+      .start(done)
+      .makeRequest('get', '/boom-error');
+  });
+
+  test('CJS - should handle promise rejections in routes.', done => {
+    createRunner(__dirname, 'scenario.js')
+      .expect({ event: EXPECTED_ERROR_EVENT })
+      .expectError()
+      .start(done)
+      .makeRequest('get', '/promise-error');
+  });
 });

dev-packages/node-integration-tests/utils/runner.ts

@@ -126,6 +126,7 @@ export function createRunner(...paths: string[]) {
   let withSentryServer = false;
   let dockerOptions: DockerOptions | undefined;
   let ensureNoErrorOutput = false;
+  let expectError = false;
 
   if (testPath.endsWith('.ts')) {
     flags.push('-r', 'ts-node/register');
@@ -136,6 +137,10 @@ export function createRunner(...paths: string[]) {
       expectedEnvelopes.push(expected);
       return this;
     },
+    expectError: function () {
+      expectError = true;
+      return this;
+    },
     withFlags: function (...args: string[]) {
       flags.push(...args);
       return this;
@@ -347,7 +352,18 @@ export function createRunner(...paths: string[]) {
           }
 
           const url = `http://localhost:${scenarioServerPort}${path}`;
-          if (method === 'get') {
+          if (expectError) {
+            try {
+              if (method === 'get') {
+                await axios.get(url, { headers });
+              } else {
+                await axios.post(url, { headers });
+              }
+            } catch (e) {
+              return;
+            }
+            return;
+          } else if (method === 'get') {
             return (await axios.get(url, { headers })).data;
           } else {
             return (await axios.post(url, { headers })).data;

packages/node/src/integrations/hapi/index.ts

@@ -20,10 +20,6 @@ function isResponseObject(response: ResponseObject | Boom): response is Response
   return response && (response as ResponseObject).statusCode !== undefined;
 }
 
-function isBoomObject(response: ResponseObject | Boom): response is Boom {
-  return response && (response as Boom).isBoom !== undefined;
-}
-
 function isErrorEvent(event: RequestEvent): event is RequestEvent {
   return event && (event as RequestEvent).error !== undefined;
 }
@@ -51,9 +47,7 @@ export const hapiErrorPlugin = {
       // eslint-disable-next-line deprecation/deprecation
       const transaction = getActiveTransaction();
 
-      if (request.response && isBoomObject(request.response)) {
-        sendErrorToSentry(request.response);
-      } else if (isErrorEvent(event)) {
+      if (isErrorEvent(event)) {
         sendErrorToSentry(event.error);
       }