From 54d6241bf1510e2797db0b5b1029fde4fddd97bd Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Sun, 17 Mar 2024 16:48:26 +0000 Subject: [PATCH 1/3] ref(node): Skip capturing Hapi Boom error responses. --- .../tracing-experimental/hapi/scenario.js | 8 ++++++++ .../suites/tracing-experimental/hapi/test.ts | 19 +++++++++++++++++++ .../node-integration-tests/utils/runner.ts | 18 +++++++++++++++++- packages/node/src/integrations/hapi/index.ts | 8 +------- 4 files changed, 45 insertions(+), 8 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js index 9a00fa36957d..3948217eb751 100644 --- a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js @@ -26,6 +26,14 @@ const init = async () => { }, }); + server.route({ + method: 'GET', + path: '/error', + handler: (_request, _h) => { + throw new Error('Sentry Test Error'); + }, + }); + await Sentry.setupHapiErrorHandler(server); await server.start(); diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts index 93e3203f6470..957aeedae7d9 100644 --- a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts @@ -25,10 +25,29 @@ describe('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 errors in routes.', done => { + createRunner(__dirname, 'scenario.js') + .expect({ event: EXPECTED_ERROR_EVENT }) + .expectError() + .start(done) + .makeRequest('get', '/error'); + }); }); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 515a2627acf8..5da33508bda0 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/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; diff --git a/packages/node/src/integrations/hapi/index.ts b/packages/node/src/integrations/hapi/index.ts index 0c500fbd1ae1..58e329479f6a 100644 --- a/packages/node/src/integrations/hapi/index.ts +++ b/packages/node/src/integrations/hapi/index.ts @@ -23,10 +23,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; } @@ -54,9 +50,7 @@ export const hapiErrorPlugin = { const activeSpan = getActiveSpan(); const rootSpan = activeSpan && getRootSpan(activeSpan); - if (request.response && isBoomObject(request.response)) { - sendErrorToSentry(request.response); - } else if (isErrorEvent(event)) { + if (isErrorEvent(event)) { sendErrorToSentry(event.error); } From d9146cb6f19486f3dc7451245e57a6e73cf80ce8 Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Wed, 20 Mar 2024 15:03:57 +0000 Subject: [PATCH 2/3] Update tests. --- .../suites/tracing-experimental/hapi/scenario.js | 11 ++++++++++- .../suites/tracing-experimental/hapi/test.ts | 10 +++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js index 3948217eb751..ce5aff58286b 100644 --- a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js @@ -9,6 +9,7 @@ Sentry.init({ }); const Hapi = require('@hapi/hapi'); +const Boom = require('@hapi/boom'); const port = 5999; @@ -30,7 +31,15 @@ const init = async () => { method: 'GET', path: '/error', handler: (_request, _h) => { - throw new Error('Sentry Test Error'); + return new Error('Sentry Test Error'); + }, + }); + + server.route({ + method: 'GET', + path: '/boom-error', + handler: (_request, _h) => { + return new Boom.Boom('Sentry Test Error'); }, }); diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts index 957aeedae7d9..a4a7701871ad 100644 --- a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts @@ -43,11 +43,19 @@ describe('hapi auto-instrumentation', () => { .makeRequest('get', '/'); }); - test('CJS - should handle errors in routes.', done => { + 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'); + }); }); From c671e1f4e2d18aa27cf56bdd1b0762be350d00ec Mon Sep 17 00:00:00 2001 From: Onur Temizkan Date: Sun, 24 Mar 2024 22:14:11 +0000 Subject: [PATCH 3/3] Add more tests. --- .../suites/tracing-experimental/hapi/scenario.js | 8 ++++++++ .../suites/tracing-experimental/hapi/test.ts | 8 ++++++++ 2 files changed, 16 insertions(+) diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js index ce5aff58286b..69443559e9a8 100644 --- a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/scenario.js @@ -43,6 +43,14 @@ const init = async () => { }, }); + 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(); diff --git a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts index a4a7701871ad..da8a07c20cc1 100644 --- a/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts +++ b/dev-packages/node-integration-tests/suites/tracing-experimental/hapi/test.ts @@ -58,4 +58,12 @@ describe('hapi auto-instrumentation', () => { .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'); + }); });