Skip to content

Commit

Permalink
Merge branch 'develop' into sig/nuxt-plugin-template
Browse files Browse the repository at this point in the history
  • Loading branch information
s1gr1d authored Jul 4, 2024
2 parents 69dc0df + 4018f80 commit d802cb0
Show file tree
Hide file tree
Showing 7 changed files with 371 additions and 219 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@
"test:assert": "pnpm test"
},
"dependencies": {
"@hapi/hapi": "21.3.2",
"@hapi/boom": "10.0.1",
"@hapi/hapi": "21.3.10",
"@sentry/node": "latest || *",
"@sentry/types": "latest || *"
},
Expand Down
50 changes: 50 additions & 0 deletions dev-packages/e2e-tests/test-applications/node-hapi/src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ Sentry.init({
});

const Hapi = require('@hapi/hapi');
const Boom = require('@hapi/boom');

const server = Hapi.server({
port: 3030,
Expand Down Expand Up @@ -63,6 +64,55 @@ const init = async () => {
throw new Error('This is an error');
},
});

server.route({
method: 'GET',
path: '/test-failure-boom-4xx',
handler: async function (request, h) {
throw new Error('This is a JS error (boom in onPreResponse)');
},
});

server.route({
method: 'GET',
path: '/test-failure-boom-5xx',
handler: async function (request, h) {
throw new Error('This is an error (boom in onPreResponse)');
},
});

server.route({
method: 'GET',
path: '/test-failure-JS-error-onPreResponse',
handler: async function (request, h) {
throw new Error('This is an error (another JS error in onPreResponse)');
},
});

server.route({
method: 'GET',
path: '/test-failure-2xx-override-onPreResponse',
handler: async function (request, h) {
throw new Error('This is a JS error (2xx override in onPreResponse)');
},
});

// This runs after the route handler
server.ext('onPreResponse', (request, h) => {
const path = request.route.path;

if (path.includes('boom-4xx')) {
throw Boom.notFound('4xx not found (onPreResponse)');
} else if (path.includes('boom-5xx')) {
throw Boom.gatewayTimeout('5xx not implemented (onPreResponse)');
} else if (path.includes('JS-error-onPreResponse')) {
throw new Error('JS error (onPreResponse)');
} else if (path.includes('2xx-override-onPreResponse')) {
return h.response('2xx override').code(200);
} else {
return h.continue;
}
});
};

(async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,97 @@ test('sends error with parameterized transaction name', async ({ baseURL }) => {

expect(errorEvent?.transaction).toBe('GET /test-error/{id}');
});

test('Does not send errors to Sentry if boom throws in "onPreResponse" after JS error in route handler', async ({
baseURL,
}) => {
let errorEventOccurred = false;

waitForError('node-hapi', event => {
if (event.exception?.values?.[0]?.value?.includes('This is a JS error (boom in onPreResponse)')) {
errorEventOccurred = true;
}
return false; // expects to return a boolean (but not relevant here)
});

const transactionEventPromise4xx = waitForTransaction('node-hapi', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-failure-boom-4xx';
});

const transactionEventPromise5xx = waitForTransaction('node-hapi', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-failure-boom-5xx';
});

const response4xx = await fetch(`${baseURL}/test-failure-boom-4xx`);
const response5xx = await fetch(`${baseURL}/test-failure-boom-5xx`);

expect(response4xx.status).toBe(404);
expect(response5xx.status).toBe(504);

const transactionEvent4xx = await transactionEventPromise4xx;
const transactionEvent5xx = await transactionEventPromise5xx;

expect(errorEventOccurred).toBe(false);
expect(transactionEvent4xx.transaction).toBe('GET /test-failure-boom-4xx');
expect(transactionEvent5xx.transaction).toBe('GET /test-failure-boom-5xx');
});

test('Does not send error to Sentry if error response is overwritten with 2xx in "onPreResponse"', async ({
baseURL,
}) => {
let errorEventOccurred = false;

waitForError('node-hapi', event => {
if (event.exception?.values?.[0]?.value?.includes('This is a JS error (2xx override in onPreResponse)')) {
errorEventOccurred = true;
}
return false; // expects to return a boolean (but not relevant here)
});

const transactionEventPromise = waitForTransaction('node-hapi', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-failure-2xx-override-onPreResponse';
});

const response = await fetch(`${baseURL}/test-failure-2xx-override-onPreResponse`);

const transactionEvent = await transactionEventPromise;

expect(response.status).toBe(200);
expect(errorEventOccurred).toBe(false);
expect(transactionEvent.transaction).toBe('GET /test-failure-2xx-override-onPreResponse');
});

test('Only sends onPreResponse error to Sentry if JS error is thrown in route handler AND onPreResponse', async ({
baseURL,
}) => {
const errorEventPromise = waitForError('node-hapi', errorEvent => {
return errorEvent?.exception?.values?.[0]?.value?.includes('JS error (onPreResponse)') || false;
});

let routeHandlerErrorOccurred = false;

waitForError('node-hapi', event => {
if (
!event.type &&
event.exception?.values?.[0]?.value?.includes('This is an error (another JS error in onPreResponse)')
) {
routeHandlerErrorOccurred = true;
}
return false; // expects to return a boolean (but not relevant here)
});

const transactionEventPromise = waitForTransaction('node-hapi', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-failure-JS-error-onPreResponse';
});

const response = await fetch(`${baseURL}/test-failure-JS-error-onPreResponse`);

expect(response.status).toBe(500);

const errorEvent = await errorEventPromise;
const transactionEvent = await transactionEventPromise;

expect(routeHandlerErrorOccurred).toBe(false);
expect(transactionEvent.transaction).toBe('GET /test-failure-JS-error-onPreResponse');
expect(errorEvent.transaction).toEqual('GET /test-failure-JS-error-onPreResponse');
});
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,25 @@ test('Sends successful transaction', async ({ baseURL }) => {
timestamp: expect.any(Number),
trace_id: expect.any(String),
},
{
// this comes from "onPreResponse"
data: {
'hapi.type': 'server.ext',
'otel.kind': 'INTERNAL',
'sentry.op': 'server.ext.hapi',
'sentry.origin': 'auto.http.otel.hapi',
'server.ext.type': 'onPreResponse',
},
description: 'ext - onPreResponse',
op: 'server.ext.hapi',
origin: 'auto.http.otel.hapi',
parent_span_id: expect.any(String),
span_id: expect.any(String),
start_timestamp: expect.any(Number),
status: 'ok',
timestamp: expect.any(Number),
trace_id: expect.any(String),
},
]);
});

Expand Down
2 changes: 1 addition & 1 deletion dev-packages/node-integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
},
"dependencies": {
"@aws-sdk/client-s3": "^3.552.0",
"@hapi/hapi": "^20.3.0",
"@hapi/hapi": "^21.3.10",
"@nestjs/common": "^10.3.7",
"@nestjs/core": "^10.3.3",
"@nestjs/platform-express": "^10.3.3",
Expand Down
2 changes: 1 addition & 1 deletion packages/node/src/integrations/tracing/hapi/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export const hapiErrorPlugin = {
register: async function (serverArg: Record<any, any>) {
const server = serverArg as unknown as Server;

server.events.on('request', (request: Request, event: RequestEvent) => {
server.events.on({ name: 'request', channels: ['error'] }, (request: Request, event: RequestEvent) => {
if (getIsolationScope() !== getDefaultIsolationScope()) {
const route = request.route;
if (route && route.path) {
Expand Down
Loading

0 comments on commit d802cb0

Please sign in to comment.