From 0c927954a7449421aa77a62e87be59ec3170b99a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Wed, 1 Jul 2020 16:18:13 +0200 Subject: [PATCH 1/2] fix: Extract transaction from nested express paths correctly --- packages/node/src/handlers.ts | 18 +++++-- packages/node/test/handlers.test.ts | 73 +++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index f47703ccaca4..8c91f7449279 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -80,6 +80,8 @@ function extractTransaction(req: { [key: string]: any }, type: boolean | Transac try { // Express.js shape const request = req as { + url: string; + originalUrl: string; method: string; route: { path: string; @@ -91,9 +93,20 @@ function extractTransaction(req: { [key: string]: any }, type: boolean | Transac }; }; + let routePath; + try { + if (request.originalUrl) { + routePath = url.parse(request.originalUrl).pathname; + } else { + routePath = url.parse(request.url).pathname; + } + } catch (_oO) { + routePath = request.route.path; + } + switch (type) { case 'path': { - return request.route.path; + return routePath; } case 'handler': { return request.route.stack[0].name; @@ -101,8 +114,7 @@ function extractTransaction(req: { [key: string]: any }, type: boolean | Transac case 'methodPath': default: { const method = request.method.toUpperCase(); - const path = request.route.path; - return `${method}|${path}`; + return `${method}|${routePath}`; } } } catch (_oO) { diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 8a48e41c5bb3..93a37c4a763e 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -4,21 +4,34 @@ import { Event, Request, User } from '../src'; import { parseRequest } from '../src/handlers'; describe('parseRequest', () => { - const mockReq = { - body: 'foo', - cookies: { test: 'test' }, - headers: { - host: 'mattrobenolt.com', - }, - method: 'POST', - url: '/some/path?key=value', - user: { - custom_property: 'foo', - email: 'tobias@mail.com', - id: 123, - username: 'tobias', - }, - }; + let mockReq: { [key: string]: any }; + + beforeEach(() => { + mockReq = { + body: 'foo', + cookies: { test: 'test' }, + headers: { + host: 'mattrobenolt.com', + }, + method: 'POST', + url: '/some/url?key=value', + originalUrl: '/some/originalUrl?key=value', + user: { + custom_property: 'foo', + email: 'tobias@mail.com', + id: 123, + username: 'tobias', + }, + route: { + path: '/path', + stack: [ + { + name: 'routeHandler', + }, + ], + }, + }; + }); describe('parseRequest.contexts runtime', () => { test('runtime name must contain node', () => { @@ -121,4 +134,34 @@ describe('parseRequest', () => { expect(parseRequest({}, { ...mockReq, method: 'HEAD' }, {}).request).not.toHaveProperty('data'); }); }); + + describe('parseRequest.transaction property', () => { + test('extracts method and full route path by default from `originalUrl`', () => { + const parsedRequest: Event = parseRequest({}, mockReq); + expect(parsedRequest.transaction).toEqual('POST|/some/originalUrl'); + }); + + test('extracts method and full route path by default from `url` if `originalUrl` is not present', () => { + delete mockReq.originalUrl; + const parsedRequest: Event = parseRequest({}, mockReq); + expect(parsedRequest.transaction).toEqual('POST|/some/url'); + }); + + test('fallback to method and `route.path` if previous attempts failed', () => { + delete mockReq.originalUrl; + delete mockReq.url; + const parsedRequest: Event = parseRequest({}, mockReq); + expect(parsedRequest.transaction).toEqual('POST|/path'); + }); + + test('can extract path only instead if configured', () => { + const parsedRequest: Event = parseRequest({}, mockReq, { transaction: 'path' }); + expect(parsedRequest.transaction).toEqual('/some/originalUrl'); + }); + + test('can extract handler name instead if configured', () => { + const parsedRequest: Event = parseRequest({}, mockReq, { transaction: 'handler' }); + expect(parsedRequest.transaction).toEqual('routeHandler'); + }); + }); }); From 8df0554afac462316654222df53b557e89d3be3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20Og=C3=B3rek?= Date: Wed, 1 Jul 2020 16:34:53 +0200 Subject: [PATCH 2/2] linterz --- packages/node/src/handlers.ts | 6 +----- packages/node/test/handlers.test.ts | 14 +++++++------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index 8c91f7449279..586877b02df7 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -95,11 +95,7 @@ function extractTransaction(req: { [key: string]: any }, type: boolean | Transac let routePath; try { - if (request.originalUrl) { - routePath = url.parse(request.originalUrl).pathname; - } else { - routePath = url.parse(request.url).pathname; - } + routePath = url.parse(request.originalUrl || request.url).pathname; } catch (_oO) { routePath = request.route.path; } diff --git a/packages/node/test/handlers.test.ts b/packages/node/test/handlers.test.ts index 93a37c4a763e..84f3b5763582 100644 --- a/packages/node/test/handlers.test.ts +++ b/packages/node/test/handlers.test.ts @@ -14,14 +14,7 @@ describe('parseRequest', () => { host: 'mattrobenolt.com', }, method: 'POST', - url: '/some/url?key=value', originalUrl: '/some/originalUrl?key=value', - user: { - custom_property: 'foo', - email: 'tobias@mail.com', - id: 123, - username: 'tobias', - }, route: { path: '/path', stack: [ @@ -30,6 +23,13 @@ describe('parseRequest', () => { }, ], }, + url: '/some/url?key=value', + user: { + custom_property: 'foo', + email: 'tobias@mail.com', + id: 123, + username: 'tobias', + }, }; });