From 4a4b4a3a2288ca1c87c25166131df61886640530 Mon Sep 17 00:00:00 2001 From: isaacs Date: Fri, 20 Mar 2026 13:40:30 -0700 Subject: [PATCH 1/2] core: add wrapMethod/unwrapMethod util --- packages/core/src/utils/object.ts | 31 +++++++++++++++++ packages/core/test/lib/utils/object.test.ts | 37 +++++++++++++++++++++ 2 files changed, 68 insertions(+) diff --git a/packages/core/src/utils/object.ts b/packages/core/src/utils/object.ts index 1ffabca10bb2..f66de0bd8f5f 100644 --- a/packages/core/src/utils/object.ts +++ b/packages/core/src/utils/object.ts @@ -18,6 +18,7 @@ import { isElement, isError, isEvent, isInstanceOf, isPrimitive } from './is'; * args>)` or `origMethod.apply(this, [])` (rather than being called directly), again to preserve `this`. * @returns void */ + export function fill(source: { [key: string]: any }, name: string, replacementFactory: (...args: any[]) => any): void { if (!(name in source)) { return; @@ -80,6 +81,36 @@ export function markFunctionWrapped(wrapped: WrappedFunction, original: WrappedF } catch {} // eslint-disable-line no-empty } +/** + * Wrap a method on an object by name, only if it is not already wrapped. + */ +export function wrapMethod(obj: O, field: T, wrapped: WrappedFunction): void { + const original = obj[field]; + if (typeof original !== 'function') { + throw new Error(`Cannot wrap method: ${field} is not a function`); + } + if (getOriginalFunction(original)) { + throw new Error(`Attempting to wrap method ${field} multiple times`); + } + markFunctionWrapped(wrapped, original); + addNonEnumerableProperty(obj, field, wrapped); +} + +/** + * Unwrap a previously wrapped method on an object by name + */ +export function unwrapMethod(obj: O, field: T): void { + const wrapped = obj[field]; + if (typeof wrapped !== 'function') { + throw new Error(`Cannot unwrap method: ${field} is not a function`); + } + const original = getOriginalFunction(wrapped); + if (!original) { + throw new Error(`Method ${field} is not wrapped, and cannot be unwrapped`); + } + addNonEnumerableProperty(obj, field, original); +} + /** * This extracts the original function if available. See * `markFunctionWrapped` for more information. diff --git a/packages/core/test/lib/utils/object.test.ts b/packages/core/test/lib/utils/object.test.ts index e34260edef50..e6033216ef8e 100644 --- a/packages/core/test/lib/utils/object.test.ts +++ b/packages/core/test/lib/utils/object.test.ts @@ -11,6 +11,8 @@ import { fill, markFunctionWrapped, objectify, + unwrapMethod, + wrapMethod, } from '../../../src/utils/object'; import { testOnlyIfNodeVersionAtLeast } from '../../testutils'; @@ -455,3 +457,38 @@ describe('markFunctionWrapped', () => { expect(originalFunc).not.toHaveBeenCalled(); }); }); + +describe('unwrapMethod, wrapMethod', () => { + it('can wrap a method on an object and unwrap it later', () => { + const wrapped = () => {}; + const original = () => {}; + const obj = { m: original }; + wrapMethod(obj, 'm', wrapped); + expect(obj.m).toBe(wrapped); + expect((obj.m as WrappedFunction).__sentry_original__).toBe(original); + unwrapMethod(obj, 'm'); + expect(obj.m).toBe(original); + }); + + it('throws if misused', () => { + const wrapped = () => {}; + const original = () => {}; + const obj = { m: original }; + wrapMethod(obj, 'm', wrapped); + expect(() => { + //@ts-expect-error verify type checking prevents this mistake + wrapMethod(obj, 'foo', wrapped); + }).toThrowError('Cannot wrap method: foo is not a function'); + expect(() => { + //@ts-expect-error verify type checking prevents this mistake + unwrapMethod(obj, 'foo'); + }).toThrowError('Cannot unwrap method: foo is not a function'); + expect(() => { + wrapMethod(obj, 'm', wrapped); + }).toThrowError('Attempting to wrap method m multiple times'); + unwrapMethod(obj, 'm'); + expect(() => { + unwrapMethod(obj, 'm'); + }).toThrowError('Method m is not wrapped, and cannot be unwrapped'); + }); +}); From db667fc53f51b5013f2a75a9bccfaafd35fb1e90 Mon Sep 17 00:00:00 2001 From: isaacs Date: Sat, 21 Mar 2026 15:13:25 -0700 Subject: [PATCH 2/2] feat(core, node): portable Express integration This extracts the functionality from the OTel Express intstrumentation, replacing it with a portable standalone integration in `@sentry/core`, which can be extended and applied to patch any Express module import in whatever way makes sense for the platform in question. Currently in node, that is still an OpenTelemetry intstrumentation, but just handling the automatic module load instrumentation, not the entire tracing integration. This is somewhat a proof of concept, to see what it takes to port a fairly invovled OTel integration into a state where it can support all of the platforms that we care about, but it does impose a bit less of a translation layer between OTel and Sentry semantics (for example, no need to use the no-op `span.recordException()`). User-visible changes (beyond the added export in `@sentry/core`): - Express router spans have an origin of `auto.http.express` rather than `auto.http.otel.express`, since it's no longer technically an otel integration. - The empty `measurements: {}` object is no longer attached to span data, as that was an artifact of otel's `span.recordError`, which is a no-op anyway, and no longer executed. Obviously this is not a full clean-room reimplementation, and relies on the fact that the opentelemetry-js-contrib project is Apache 2.0 licensed. I included the link to the upstream license in the index file for the Express integration, but there may be a more appropriate way to ensure that the license is respected properly. It was arguably a derivative work already, but simple redistribution is somewhat different than re-implementation with subtly different context. This reduces the node overhead and makes the Express instrumentation portable to other SDKs, but it of course *increases* the bundle size of `@sentry/core` considerably. It would be a good idea to explore splitting out integrations from core, so that they're bundled and analyzed separately, rather than shipping to all SDKs that extend core. --- .../nestjs-11/tests/transactions.test.ts | 4 +- .../nestjs-8/tests/transactions.test.ts | 4 +- .../nestjs-basic/tests/transactions.test.ts | 4 +- .../tests/transactions.test.ts | 4 +- .../tests/transactions.test.ts | 4 +- .../tests/server.test.ts | 12 +- .../tests/server.test.ts | 12 +- .../tests/server.test.ts | 12 +- .../tests/transactions.test.ts | 4 +- .../node-express/tests/transactions.test.ts | 25 +- .../tests/sampling.test.ts | 12 +- .../tests/transactions.test.ts | 25 +- .../node-otel/tests/transactions.test.ts | 25 +- .../tsx-express/tests/transactions.test.ts | 25 +- .../suites/express/tracing/test.ts | 4 +- packages/core/src/index.ts | 12 + .../core/src/integrations/express/index.ts | 264 +++++++++ .../src/integrations/express/patch-layer.ts | 202 +++++++ .../express/request-layer-store.ts | 11 + .../core/src/integrations/express/types.ts | 157 +++++ .../core/src/integrations/express/utils.ts | 283 +++++++++ .../lib/integrations/express/index.test.ts | 359 ++++++++++++ .../integrations/express/patch-layer.test.ts | 423 +++++++++++++ .../express/request-layer-store.test.ts | 18 + .../lib/integrations/express/types.test.ts | 18 + .../lib/integrations/express/utils.test.ts | 554 ++++++++++++++++++ .../node-core/src/utils/ensureIsWrapped.ts | 15 +- .../test/utils/ensureIsWrapped.test.ts | 21 + packages/node/package.json | 1 - .../node/src/integrations/tracing/express.ts | 242 +++----- yarn.lock | 12 +- 31 files changed, 2491 insertions(+), 277 deletions(-) create mode 100644 packages/core/src/integrations/express/index.ts create mode 100644 packages/core/src/integrations/express/patch-layer.ts create mode 100644 packages/core/src/integrations/express/request-layer-store.ts create mode 100644 packages/core/src/integrations/express/types.ts create mode 100644 packages/core/src/integrations/express/utils.ts create mode 100644 packages/core/test/lib/integrations/express/index.test.ts create mode 100644 packages/core/test/lib/integrations/express/patch-layer.test.ts create mode 100644 packages/core/test/lib/integrations/express/request-layer-store.test.ts create mode 100644 packages/core/test/lib/integrations/express/types.test.ts create mode 100644 packages/core/test/lib/integrations/express/utils.test.ts diff --git a/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts index a8b8f25e46c1..375c56a845d6 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-11/tests/transactions.test.ts @@ -61,7 +61,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'express.name': '/test-transaction', 'express.type': 'request_handler', 'http.route': '/test-transaction', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', @@ -72,7 +72,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { status: 'ok', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }, { data: { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts index 862f730636c0..7270ad211909 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-8/tests/transactions.test.ts @@ -65,7 +65,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'express.name': '/test-transaction', 'express.type': 'request_handler', 'http.route': '/test-transaction', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', @@ -76,7 +76,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { status: 'ok', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }, { data: { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts index 440a1391556a..a1c1e80762c0 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-basic/tests/transactions.test.ts @@ -61,7 +61,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { 'express.name': '/test-transaction', 'express.type': 'request_handler', 'http.route': '/test-transaction', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', @@ -72,7 +72,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { status: 'ok', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }, { data: { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts index 380e5fdc018e..9ca18ec0888f 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules-decorator/tests/transactions.test.ts @@ -61,7 +61,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { 'express.name': '/example-module/transaction', 'express.type': 'request_handler', 'http.route': '/example-module/transaction', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', @@ -72,7 +72,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { status: 'ok', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }, { data: { diff --git a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts index 416ff72e946b..ddfcb1192edf 100644 --- a/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/nestjs-with-submodules/tests/transactions.test.ts @@ -61,7 +61,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { 'express.name': '/example-module/transaction', 'express.type': 'request_handler', 'http.route': '/example-module/transaction', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', @@ -72,7 +72,7 @@ test('Sends an API route transaction from module', async ({ baseURL }) => { status: 'ok', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }, { data: { diff --git a/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/tests/server.test.ts b/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/tests/server.test.ts index a403a23bebda..df5ba8e47352 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/tests/server.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-cjs-preload/tests/server.test.ts @@ -65,12 +65,12 @@ test('Should record a transaction for route with parameters', async ({ request } data: { 'express.name': 'query', 'express.type': 'middleware', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', }, op: 'middleware.express', description: 'query', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -83,12 +83,12 @@ test('Should record a transaction for route with parameters', async ({ request } data: { 'express.name': 'expressInit', 'express.type': 'middleware', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', }, op: 'middleware.express', description: 'expressInit', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -102,12 +102,12 @@ test('Should record a transaction for route with parameters', async ({ request } 'express.name': '/test-transaction/:param', 'express.type': 'request_handler', 'http.route': '/test-transaction/:param', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', description: '/test-transaction/:param', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), diff --git a/dev-packages/e2e-tests/test-applications/node-express-esm-loader/tests/server.test.ts b/dev-packages/e2e-tests/test-applications/node-express-esm-loader/tests/server.test.ts index d919c75ea61b..e6337bf7ba83 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-esm-loader/tests/server.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-esm-loader/tests/server.test.ts @@ -65,12 +65,12 @@ test('Should record a transaction for route with parameters', async ({ request } data: { 'express.name': 'query', 'express.type': 'middleware', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', }, op: 'middleware.express', description: 'query', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -83,12 +83,12 @@ test('Should record a transaction for route with parameters', async ({ request } data: { 'express.name': 'expressInit', 'express.type': 'middleware', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', }, op: 'middleware.express', description: 'expressInit', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -102,12 +102,12 @@ test('Should record a transaction for route with parameters', async ({ request } 'express.name': '/test-transaction/:param', 'express.type': 'request_handler', 'http.route': '/test-transaction/:param', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', description: '/test-transaction/:param', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), diff --git a/dev-packages/e2e-tests/test-applications/node-express-esm-preload/tests/server.test.ts b/dev-packages/e2e-tests/test-applications/node-express-esm-preload/tests/server.test.ts index 706fe1b4460f..937f2b7acc27 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-esm-preload/tests/server.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-esm-preload/tests/server.test.ts @@ -65,12 +65,12 @@ test('Should record a transaction for route with parameters', async ({ request } data: { 'express.name': 'query', 'express.type': 'middleware', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', }, op: 'middleware.express', description: 'query', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -83,12 +83,12 @@ test('Should record a transaction for route with parameters', async ({ request } data: { 'express.name': 'expressInit', 'express.type': 'middleware', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', }, op: 'middleware.express', description: 'expressInit', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -102,12 +102,12 @@ test('Should record a transaction for route with parameters', async ({ request } 'express.name': '/test-transaction/:param', 'express.type': 'request_handler', 'http.route': '/test-transaction/:param', - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', }, op: 'request_handler.express', description: '/test-transaction/:param', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), diff --git a/dev-packages/e2e-tests/test-applications/node-express-v5/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-express-v5/tests/transactions.test.ts index 048f70a1aba8..ba9632aaf952 100644 --- a/dev-packages/e2e-tests/test-applications/node-express-v5/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express-v5/tests/transactions.test.ts @@ -85,7 +85,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { // auto instrumented span expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-transaction', 'express.name': '/test-transaction', @@ -93,7 +93,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { }, description: '/test-transaction', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), diff --git a/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts index 7784d7fbe3fe..c0c286da3345 100644 --- a/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-express/tests/transactions.test.ts @@ -85,14 +85,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { // auto instrumented spans expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -103,14 +103,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -121,7 +121,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-transaction', 'express.name': '/test-transaction', @@ -129,7 +129,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { }, description: '/test-transaction', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -161,14 +161,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -179,14 +179,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -197,7 +197,7 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-exception/:id', 'express.name': '/test-exception/:id', @@ -205,14 +205,13 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) }, description: '/test-exception/:id', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), status: 'internal_error', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - measurements: {}, }); }); diff --git a/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/tests/sampling.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/tests/sampling.test.ts index 84d783b1d567..49d35cb9e85f 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/tests/sampling.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-custom-sampler/tests/sampling.test.ts @@ -55,7 +55,7 @@ test('Sends a sampled API route transaction', async ({ baseURL }) => { span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', @@ -66,14 +66,14 @@ test('Sends a sampled API route transaction', async ({ baseURL }) => { timestamp: expect.any(Number), status: 'ok', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }); expect(transactionEvent.spans).toContainEqual({ span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', @@ -84,14 +84,14 @@ test('Sends a sampled API route transaction', async ({ baseURL }) => { timestamp: expect.any(Number), status: 'ok', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }); expect(transactionEvent.spans).toContainEqual({ span_id: expect.stringMatching(/[a-f0-9]{16}/), trace_id: expect.stringMatching(/[a-f0-9]{32}/), data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/task', 'express.name': '/task', @@ -103,7 +103,7 @@ test('Sends a sampled API route transaction', async ({ baseURL }) => { timestamp: expect.any(Number), status: 'ok', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }); expect(transactionEvent.spans).toContainEqual({ diff --git a/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/tests/transactions.test.ts index f724eee3fc64..299d3c2b80ec 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel-sdk-node/tests/transactions.test.ts @@ -79,14 +79,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -97,14 +97,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -115,7 +115,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-transaction', 'express.name': '/test-transaction', @@ -123,7 +123,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { }, description: '/test-transaction', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -155,14 +155,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -173,14 +173,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -191,7 +191,7 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-exception/:id', 'express.name': '/test-exception/:id', @@ -199,13 +199,12 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) }, description: '/test-exception/:id', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), status: 'internal_error', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - measurements: {}, }); }); diff --git a/dev-packages/e2e-tests/test-applications/node-otel/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/node-otel/tests/transactions.test.ts index 8a561e91a76a..ba77e6a3b294 100644 --- a/dev-packages/e2e-tests/test-applications/node-otel/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/node-otel/tests/transactions.test.ts @@ -79,14 +79,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -97,14 +97,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -115,7 +115,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-transaction', 'express.name': '/test-transaction', @@ -123,7 +123,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { }, description: '/test-transaction', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -155,14 +155,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -173,14 +173,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -191,7 +191,7 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-exception/:id', 'express.name': '/test-exception/:id', @@ -199,13 +199,12 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) }, description: '/test-exception/:id', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), status: 'internal_error', timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), - measurements: {}, }); }); diff --git a/dev-packages/e2e-tests/test-applications/tsx-express/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/tsx-express/tests/transactions.test.ts index 5957dbfd9738..35fe8f17bd94 100644 --- a/dev-packages/e2e-tests/test-applications/tsx-express/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/tsx-express/tests/transactions.test.ts @@ -71,14 +71,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -89,14 +89,14 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -107,7 +107,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-transaction', 'express.name': '/test-transaction', @@ -115,7 +115,7 @@ test('Sends an API route transaction', async ({ baseURL }) => { }, description: '/test-transaction', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -147,14 +147,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'query', 'express.type': 'middleware', }, description: 'query', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -165,14 +165,14 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'middleware.express', 'express.name': 'expressInit', 'express.type': 'middleware', }, description: 'expressInit', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), @@ -183,7 +183,7 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) expect(spans).toContainEqual({ data: { - 'sentry.origin': 'auto.http.otel.express', + 'sentry.origin': 'auto.http.express', 'sentry.op': 'request_handler.express', 'http.route': '/test-exception/:id', 'express.name': '/test-exception/:id', @@ -191,12 +191,11 @@ test('Sends an API route transaction for an errored route', async ({ baseURL }) }, description: '/test-exception/:id', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', parent_span_id: expect.stringMatching(/[a-f0-9]{16}/), span_id: expect.stringMatching(/[a-f0-9]{16}/), start_timestamp: expect.any(Number), status: 'internal_error', - measurements: {}, timestamp: expect.any(Number), trace_id: expect.stringMatching(/[a-f0-9]{32}/), }); diff --git a/dev-packages/node-integration-tests/suites/express/tracing/test.ts b/dev-packages/node-integration-tests/suites/express/tracing/test.ts index 759df300181a..62f827440522 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing/test.ts +++ b/dev-packages/node-integration-tests/suites/express/tracing/test.ts @@ -32,7 +32,7 @@ describe('express tracing', () => { }), description: 'corsMiddleware', op: 'middleware.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }), expect.objectContaining({ data: expect.objectContaining({ @@ -41,7 +41,7 @@ describe('express tracing', () => { }), description: '/test/express', op: 'request_handler.express', - origin: 'auto.http.otel.express', + origin: 'auto.http.express', }), ]), }, diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 61865ea7ba3c..b44a8489a0ed 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -116,6 +116,18 @@ export { linkedErrorsIntegration } from './integrations/linkederrors'; export { moduleMetadataIntegration } from './integrations/moduleMetadata'; export { requestDataIntegration } from './integrations/requestdata'; export { captureConsoleIntegration } from './integrations/captureconsole'; +export { + unpatchExpressModule, + patchExpressModule, + setupExpressErrorHandler, + expressErrorHandler, +} from './integrations/express/index'; +export type { + ExpressIntegrationOptions, + ExpressHandlerOptions, + ExpressMiddleware, + ExpressErrorMiddleware, +} from './integrations/express/types'; export { dedupeIntegration } from './integrations/dedupe'; export { extraErrorDataIntegration } from './integrations/extraerrordata'; export { rewriteFramesIntegration } from './integrations/rewriteframes'; diff --git a/packages/core/src/integrations/express/index.ts b/packages/core/src/integrations/express/index.ts new file mode 100644 index 000000000000..084de47571f6 --- /dev/null +++ b/packages/core/src/integrations/express/index.ts @@ -0,0 +1,264 @@ +/** + * Platform-portable Express tracing integration, OTel-free, for use + * on Cloudflare, Deno, Bun, etc. + * + * @module + * + * This Sentry integration is a derivative work based on the OpenTelemetry + * Express instrumentation. + * + * + * + * Extended under the terms of the Apache 2.0 license linked below: + * + * ---- + * + * Copyright The OpenTelemetry Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { debug } from '../../utils/debug-logger'; +import { captureException } from '../../exports'; +import { getIsolationScope } from '../../currentScopes'; +import { httpRequestToRequestData } from '../../utils/request'; +import { DEBUG_BUILD } from '../../debug-build'; +import type { + ExpressApplication, + ExpressErrorMiddleware, + ExpressExport, + ExpressHandlerOptions, + ExpressIntegrationOptions, + ExpressLayer, + ExpressMiddleware, + ExpressModuleExport, + ExpressRequest, + ExpressResponse, + ExpressRouter, + ExpressRouterv4, + ExpressRouterv5, + MiddlewareError, +} from './types'; +import { + defaultShouldHandleError, + getLayerPath, + hasDefaultProp, + isExpressWithoutRouterPrototype, + isExpressWithRouterPrototype, +} from './utils'; +import { unwrapMethod, wrapMethod } from '../../utils/object'; +import { patchLayer } from './patch-layer'; + +const getExpressExport = (express: ExpressModuleExport): ExpressExport => + hasDefaultProp(express) ? express.default : (express as ExpressExport); + +/** + * This is a portable instrumentatiton function that works in any environment + * where Express can be loaded, without depending on OpenTelemetry. + * + * @example + * ```javascript + * import express from 'express'; + * import * as Sentry from '@sentry/deno'; // or any SDK that extends core + * + * Sentry.patchExpressModule({ express }) + */ +export const patchExpressModule = (options: ExpressIntegrationOptions) => { + // pass in the require() or import() result of express + const express = getExpressExport(options.express); + const routerProto: ExpressRouterv4 | ExpressRouterv5 | undefined = isExpressWithRouterPrototype(express) + ? express.Router.prototype // Express v5 + : isExpressWithoutRouterPrototype(express) + ? express.Router // Express v4 + : undefined; + + if (!routerProto) { + throw new TypeError('no valid Express route function to instrument'); + } + + // oxlint-disable-next-line @typescript-eslint/unbound-method + const originalRouteMethod = routerProto.route; + try { + wrapMethod( + routerProto, + 'route', + function routeTrace(this: ExpressRouter, ...args: Parameters[]) { + const route = originalRouteMethod.apply(this, args); + const layer = this.stack[this.stack.length - 1] as ExpressLayer; + patchLayer(options, layer, getLayerPath(args)); + return route; + }, + ); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to patch express route method:', e); + } + + // oxlint-disable-next-line @typescript-eslint/unbound-method + const originalRouterUse = routerProto.use; + try { + wrapMethod( + routerProto, + 'use', + function useTrace(this: ExpressApplication, ...args: Parameters) { + const route = originalRouterUse.apply(this, args); + const layer = this.stack[this.stack.length - 1]; + if (!layer) return route; + patchLayer(options, layer, getLayerPath(args)); + return route; + }, + ); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to patch express use method:', e); + } + + const { application } = express; + const originalApplicationUse = application.use; + try { + wrapMethod( + application, + 'use', + function appUseTrace( + this: ExpressApplication & { + _router?: ExpressRouter; + router?: ExpressRouter; + }, + ...args: Parameters + ) { + // If we access app.router in express 4.x we trigger an assertion error. + // This property existed in v3, was removed in v4 and then re-added in v5. + const router = isExpressWithRouterPrototype(express) ? this.router : this._router; + const route = originalApplicationUse.apply(this, args); + if (router) { + const layer = router.stack[router.stack.length - 1]; + if (layer) { + patchLayer(options, layer, getLayerPath(args)); + } + } + return route; + }, + ); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to patch express application.use method:', e); + } + + return express; +}; + +export const unpatchExpressModule = (options: ExpressIntegrationOptions) => { + const express = getExpressExport(options.express); + const routerProto: ExpressRouterv5 | ExpressRouterv4 | undefined = isExpressWithRouterPrototype(express) + ? express.Router.prototype // Express v5 + : isExpressWithoutRouterPrototype(express) + ? express.Router // Express v4 + : undefined; + + if (!routerProto) { + throw new TypeError('no valid Express route function to deinstrument'); + } + + try { + unwrapMethod(routerProto, 'route'); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to unpatch express route method:', e); + } + + try { + unwrapMethod(routerProto, 'use'); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to unpatch express use method:', e); + } + + const { application } = express; + try { + unwrapMethod(application, 'use'); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to unpatch express application.use method:', e); + } + + return express; +}; + +/** + * An Express-compatible error handler, used by setupExpressErrorHandler + */ +export function expressErrorHandler(options?: ExpressHandlerOptions): ExpressErrorMiddleware { + return function sentryErrorMiddleware( + error: MiddlewareError, + request: ExpressRequest, + res: ExpressResponse, + next: (error: MiddlewareError) => void, + ): void { + const normalizedRequest = httpRequestToRequestData(request); + // Ensure we use the express-enhanced request here, instead of the plain HTTP one + // When an error happens, the `expressRequestHandler` middleware does not run, so we set it here too + getIsolationScope().setSDKProcessingMetadata({ normalizedRequest }); + + const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError; + + if (shouldHandleError(error)) { + const eventId = captureException(error, { + mechanism: { type: 'auto.middleware.express', handled: false }, + }); + (res as { sentry?: string }).sentry = eventId; + } + + next(error); + }; +} + +/** + * Add an Express error handler to capture errors to Sentry. + * + * The error handler must be before any other middleware and after all controllers. + * + * @param app The Express instances + * @param options {ExpressHandlerOptions} Configuration options for the handler + * + * @example + * ```javascript + * import * as Sentry from 'sentry/deno'; // or any other @sentry/ + * import * as express from 'express'; + * + * Sentry.instrumentExpress(express); + * + * const app = express(); + * + * // Add your routes, etc. + * + * // Add this after all routes, + * // but before any and other error-handling middlewares are defined + * Sentry.setupExpressErrorHandler(app); + * + * app.listen(3000); + * ``` + */ +export function setupExpressErrorHandler( + app: { + //oxlint-disable-next-line no-explicit-any + use: (middleware: any) => unknown; + }, + options?: ExpressHandlerOptions, +): void { + app.use(expressRequestHandler()); + app.use(expressErrorHandler(options)); +} + +function expressRequestHandler(): ExpressMiddleware { + return function sentryRequestMiddleware(request: ExpressRequest, _res: ExpressResponse, next: () => void): void { + const normalizedRequest = httpRequestToRequestData(request); + // Ensure we use the express-enhanced request here, instead of the plain HTTP one + getIsolationScope().setSDKProcessingMetadata({ normalizedRequest }); + + next(); + }; +} diff --git a/packages/core/src/integrations/express/patch-layer.ts b/packages/core/src/integrations/express/patch-layer.ts new file mode 100644 index 000000000000..da70d73a8f40 --- /dev/null +++ b/packages/core/src/integrations/express/patch-layer.ts @@ -0,0 +1,202 @@ +import { SEMANTIC_ATTRIBUTE_SENTRY_OP, SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN } from '../../semanticAttributes'; +import { SPAN_STATUS_ERROR, startSpanManual, withActiveSpan } from '../../tracing'; +import type { SpanAttributes } from '../../types-hoist/span'; +import { getActiveSpan, spanToJSON } from '../../utils/spanUtils'; +import { getStoredLayers, storeLayer } from './request-layer-store'; +import { + type ExpressRequest, + type ExpressResponse, + kLayerPatched, + type ExpressIntegrationOptions, + type ExpressLayer, + ATTR_HTTP_ROUTE, + ATTR_EXPRESS_TYPE, + ATTR_EXPRESS_NAME, + type ExpressLayerType, + ExpressLayerType_MIDDLEWARE, + ExpressLayerType_ROUTER, +} from './types'; +import { + asErrorAndMessage, + getActualMatchedRoute, + getConstructedRoute, + getLayerMetadata, + getSpanName, + isLayerIgnored, +} from './utils'; + +export type ExpressPatchLayerOptions = Pick< + ExpressIntegrationOptions, + 'onRouteResolved' | 'ignoreLayers' | 'ignoreLayersType' +>; + +export function patchLayer(options: ExpressPatchLayerOptions, layer?: ExpressLayer, layerPath?: string): void { + if (!layer) return; + + // avoid patching multiple times the same layer + if (layer[kLayerPatched] === true) return; + layer[kLayerPatched] = true; + + const originalHandle = layer.handle; + if (originalHandle.length === 4) { + // todo: instrument error handlers + return; + } + + Object.defineProperty(layer, 'handle', { + enumerable: true, + configurable: true, + value: function layerHandlePatched( + this: ExpressLayer, + req: ExpressRequest, + res: ExpressResponse, + //oxlint-disable-next-line no-explicit-any + ...otherArgs: any[] + ) { + // Only create spans when there's an active parent span + // Without a parent span, this request is being ignored, so skip it + const parentSpan = getActiveSpan(); + if (!parentSpan) { + return originalHandle.apply(this, [req, res, ...otherArgs]); + } + + if (layerPath) storeLayer(req, layerPath); + const storedLayers = getStoredLayers(req); + const isLayerPathStored = !!layerPath; + + const constructedRoute = getConstructedRoute(req); + const actualMatchedRoute = getActualMatchedRoute(req, constructedRoute); + + options.onRouteResolved?.(actualMatchedRoute); + + const attributes: SpanAttributes = actualMatchedRoute + ? { + [ATTR_HTTP_ROUTE]: actualMatchedRoute, + } + : {}; + const metadata = getLayerMetadata(constructedRoute, layer, layerPath); + const type = metadata.attributes[ATTR_EXPRESS_TYPE] as ExpressLayerType; + + // verify against the config if the layer should be ignored + if (isLayerIgnored(metadata.name, type, options)) { + // XXX: the isLayerPathStored guard here is *not* present in the + // original @opentelemetry/instrumentation-express impl, but was + // suggested by the Sentry code review bot. It appears to correctly + // prevent improper layer calculation in the case where there's a + // middleware without a layerPath argument. It's unclear whether + // that's possible, or if any existing code depends on that "bug". + if (isLayerPathStored && type === ExpressLayerType_MIDDLEWARE) { + storedLayers.pop(); + } + return originalHandle.apply(this, [req, res, ...otherArgs]); + } + + const spanName = getSpanName( + { + request: req, + layerType: type, + route: constructedRoute, + }, + metadata.name, + ); + return startSpanManual( + { + name: spanName, + attributes: Object.assign(attributes, metadata.attributes), + }, + span => { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, 'auto.http.express'); + + const attributes = spanToJSON(span).data; + // this is one of: middleware, request_handler, router + const type = attributes[ATTR_EXPRESS_TYPE]; + + if (type) { + span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.express`); + } + + // Also update the name, we don't need to "middleware - " prefix + const name = attributes[ATTR_EXPRESS_NAME]; + if (typeof name === 'string') { + // should this be updateSpanName? + span.updateName(name); + } + + let spanHasEnded = false; + // TODO: Fix router spans (getRouterPath does not work properly) to + // have useful names before removing this branch + if (metadata.attributes[ATTR_EXPRESS_TYPE] === ExpressLayerType_ROUTER) { + span.end(); + spanHasEnded = true; + } + // listener for response.on('finish') + const onResponseFinish = () => { + if (spanHasEnded === false) { + spanHasEnded = true; + span.end(); + } + }; + + // verify we have a callback + for (let i = 0; i < otherArgs.length; i++) { + const callback = otherArgs[i] as Function; + if (typeof callback !== 'function') continue; + + //oxlint-disable-next-line no-explicit-any + otherArgs[i] = function (...args: any[]) { + // express considers anything but an empty value, "route" or "router" + // passed to its callback to be an error + const maybeError = args[0]; + const isError = ![undefined, null, 'route', 'router'].includes(maybeError); + if (!spanHasEnded && isError) { + const [_, message] = asErrorAndMessage(maybeError); + // intentionally do not record the exception here, because + // the error handler we assign does that + span.setStatus({ + code: SPAN_STATUS_ERROR, + message, + }); + } + + if (spanHasEnded === false) { + spanHasEnded = true; + req.res?.removeListener('finish', onResponseFinish); + span.end(); + } + if (!(req.route && isError) && isLayerPathStored) { + storedLayers.pop(); + } + // execute the callback back in the parent's scope, so that + // we bubble up each level as next() is called. + return withActiveSpan(parentSpan, () => callback.apply(this, args)); + }; + break; + } + + try { + return originalHandle.apply(this, [req, res, ...otherArgs]); + } catch (anyError) { + const [_, message] = asErrorAndMessage(anyError); + // intentionally do not record the exception here, because + // the error handler we assign does that + span.setStatus({ + code: SPAN_STATUS_ERROR, + message, + }); + throw anyError; + /* v8 ignore next - it sees the block end at the throw */ + } finally { + // At this point if the callback wasn't called, that means + // either the layer is asynchronous (so it will call the + // callback later on) or that the layer directly ends the + // http response, so we'll hook into the "finish" event to + // handle the later case. + if (!spanHasEnded) { + res.once('finish', onResponseFinish); + } + } + }, + ); + }, + }); +} diff --git a/packages/core/src/integrations/express/request-layer-store.ts b/packages/core/src/integrations/express/request-layer-store.ts new file mode 100644 index 000000000000..fc3d4869147b --- /dev/null +++ b/packages/core/src/integrations/express/request-layer-store.ts @@ -0,0 +1,11 @@ +import type { ExpressRequest } from './types'; + +// map of patched request objects to stored layers +const requestLayerStore = new WeakMap(); +export const storeLayer = (req: ExpressRequest, layer: string) => { + const store = requestLayerStore.get(req); + if (!store) requestLayerStore.set(req, [layer]); + else store.push(layer); +}; + +export const getStoredLayers = (req: ExpressRequest) => requestLayerStore.get(req) ?? []; diff --git a/packages/core/src/integrations/express/types.ts b/packages/core/src/integrations/express/types.ts new file mode 100644 index 000000000000..c88f490816eb --- /dev/null +++ b/packages/core/src/integrations/express/types.ts @@ -0,0 +1,157 @@ +import type { RequestEventData } from '../../types-hoist/request'; +import type { SpanAttributes } from '../../types-hoist/span'; + +export const kLayerPatched: unique symbol = Symbol('express-layer-patched'); +export const ATTR_EXPRESS_NAME = 'express.name'; +export const ATTR_HTTP_ROUTE = 'http.route'; +export const ATTR_EXPRESS_TYPE = 'express.type'; + +export type ExpressExport = { + Router: ExpressRouterv5 | ExpressRouterv4; + application: ExpressApplication; +}; + +export type ExpressExportv5 = ExpressExport & { + Router: ExpressRouterv5; +}; + +export type ExpressExportv4 = ExpressExport & { + Router: ExpressRouterv4; +}; + +export type ExpressModuleExport = ExpressExport | { default: ExpressExport }; + +export interface ExpressRequest extends RequestEventData { + originalUrl: string; + route: unknown; + // Note: req.res is typed as optional (only present after middleware init). + // mark optional to preserve compat with express v4 types. + res?: ExpressResponse; +} + +// just a minimum type def for what we need, since this also needs to +// work in environments lacking node:http +export interface ExpressResponse { + once(ev: string, listener: Function): this; + removeListener(ev: string, listener?: Function): this; + emit(ev: string, ...data: unknown[]): this; +} + +export interface NextFunction { + (err?: unknown): void; + /** + * "Break-out" of a router by calling {next('router')}; + * @see {https://expressjs.com/en/guide/using-middleware.html#middleware.router} + */ + (deferToNext: 'router'): void; + /** + * "Break-out" of a route by calling {next('route')}; + * @see {https://expressjs.com/en/guide/using-middleware.html#middleware.application} + */ + (deferToNext: 'route'): void; +} + +export type ExpressApplicationRequestHandler = (...handlers: unknown[]) => unknown; + +export type ExpressRequestInfo = { + /** An express request object */ + request: T; + route: string; + layerType: ExpressLayerType; +}; + +export type ExpressLayerType = 'router' | 'middleware' | 'request_handler'; +export const ExpressLayerType_ROUTER = 'router'; +export const ExpressLayerType_MIDDLEWARE = 'middleware'; +export const ExpressLayerType_REQUEST_HANDLER = 'request_handler'; + +export type PathParams = string | RegExp | Array; +export type LayerPathSegment = string | RegExp | number; + +export interface ExpressRoute { + path: string; + stack: ExpressLayer[]; +} + +export type ExpressRouterv4 = ExpressRouter; + +export interface ExpressRouterv5 { + prototype: ExpressRouter; +} + +// https://github.com/expressjs/express/blob/main/lib/router/layer.js#L33 +export type ExpressLayer = { + handle: Function & + Record & { + stack?: ExpressLayer[]; + }; + [kLayerPatched]?: boolean; + name: string; + params: { [key: string]: string }; + path?: string; + regexp: RegExp; + route?: ExpressLayer; +}; + +export type ExpressRouter = { + params: { [key: string]: string }; + _params: string[]; + caseSensitive: boolean; + mergeParams: boolean; + strict: boolean; + stack: ExpressLayer[]; + route(prefix: PathParams): ExpressRoute; + use(...handlers: unknown[]): unknown; +}; + +export type IgnoreMatcher = string | RegExp | ((name: string) => boolean); + +export type ExpressIntegrationOptions = { + express: ExpressExport; //Express + /** Ignore specific based on their name */ + ignoreLayers?: IgnoreMatcher[]; + /** Ignore specific layers based on their type */ + ignoreLayersType?: ExpressLayerType[]; + /** + * Optional callback invoked each time a layer resolves the matched HTTP route. + * Platform-specific integrations (e.g. Node.js) use this to propagate the + * resolved route to the underlying transport layer (e.g. OTel RPCMetadata). + */ + onRouteResolved?: (route: string | undefined) => void; +}; + +export type LayerMetadata = { + attributes: SpanAttributes; + name: string; +}; + +export interface ExpressApplication { + stack: ExpressLayer[]; + use: ExpressApplicationRequestHandler; +} + +export interface MiddlewareError extends Error { + status?: number | string; + statusCode?: number | string; + status_code?: number | string; + output?: { + statusCode?: number | string; + }; +} + +export type ExpressMiddleware = (req: ExpressRequest, res: ExpressResponse, next: () => void) => void; + +export type ExpressErrorMiddleware = ( + error: MiddlewareError, + req: ExpressRequest, + res: ExpressResponse, + next: (error: MiddlewareError) => void, +) => void; + +export interface ExpressHandlerOptions { + /** + * Callback method deciding whether error should be captured and sent to Sentry + * @param error Captured middleware error + */ + shouldHandleError?(this: void, error: MiddlewareError): boolean; +} diff --git a/packages/core/src/integrations/express/utils.ts b/packages/core/src/integrations/express/utils.ts new file mode 100644 index 000000000000..92559323baf5 --- /dev/null +++ b/packages/core/src/integrations/express/utils.ts @@ -0,0 +1,283 @@ +import { getIsolationScope } from '../../currentScopes'; +import { DEBUG_BUILD } from '../../debug-build'; +import { getDefaultIsolationScope } from '../../defaultScopes'; +import { debug } from '../../utils/debug-logger'; +import type { SpanAttributes } from '../../types-hoist/span'; +import { getStoredLayers } from './request-layer-store'; +import type { + ExpressExport, + ExpressIntegrationOptions, + ExpressLayer, + ExpressLayerType, + ExpressRequest, + ExpressRequestInfo, + IgnoreMatcher, + LayerPathSegment, + MiddlewareError, + ExpressRouterv4, + ExpressExportv5, + ExpressExportv4, +} from './types'; +import { + ATTR_EXPRESS_NAME, + ATTR_EXPRESS_TYPE, + ExpressLayerType_MIDDLEWARE, + ExpressLayerType_REQUEST_HANDLER, + ExpressLayerType_ROUTER, +} from './types'; + +/** + * Check whether the given obj match pattern + * @param constant e.g URL of request + * @param obj obj to inspect + * @param pattern Match pattern + */ +export const satisfiesPattern = (constant: string, pattern: IgnoreMatcher): boolean => { + if (typeof pattern === 'string') { + return pattern === constant; + } else if (pattern instanceof RegExp) { + return pattern.test(constant); + } else if (typeof pattern === 'function') { + return pattern(constant); + } else { + throw new TypeError('Pattern is unsupported datatype'); + } +}; + +/** + * Converts a user-provided error value into an error and error message pair + * + * @param error - User-provided error value + * @returns Both an Error or string representation of the value and an error message + */ +export const asErrorAndMessage = (error: unknown): [string | Error, string] => + error instanceof Error ? [error, error.message] : [String(error), String(error)]; + +/** + * Checks if a route contains parameter patterns (e.g., :id, :userId) + * which are valid even if they don't exactly match the original URL + */ +export function isRoutePattern(route: string): boolean { + return route.includes(':') || route.includes('*'); +} + +/** + * Parse express layer context to retrieve a name and attributes. + * @param route The route of the layer + * @param layer Express layer + * @param [layerPath] if present, the path on which the layer has been mounted + */ +export const getLayerMetadata = ( + route: string, + layer: ExpressLayer, + layerPath?: string, +): { + attributes: SpanAttributes; + name: string; +} => { + if (layer.name === 'router') { + const maybeRouterPath = getRouterPath('', layer); + const extractedRouterPath = maybeRouterPath ? maybeRouterPath : layerPath || route || '/'; + + return { + attributes: { + [ATTR_EXPRESS_NAME]: extractedRouterPath, + [ATTR_EXPRESS_TYPE]: ExpressLayerType_ROUTER, + }, + name: `router - ${extractedRouterPath}`, + }; + } else if (layer.name === 'bound dispatch' || layer.name === 'handle') { + return { + attributes: { + [ATTR_EXPRESS_NAME]: (route || layerPath) ?? 'request handler', + [ATTR_EXPRESS_TYPE]: ExpressLayerType_REQUEST_HANDLER, + }, + name: `request handler${layer.path ? ` - ${route || layerPath}` : ''}`, + }; + } else { + return { + attributes: { + [ATTR_EXPRESS_NAME]: layer.name, + [ATTR_EXPRESS_TYPE]: ExpressLayerType_MIDDLEWARE, + }, + name: `middleware - ${layer.name}`, + }; + } +}; + +/** + * Recursively search the router path from layer stack + * @param path The path to reconstruct + * @param layer The layer to reconstruct from + * @returns The reconstructed path + */ +export const getRouterPath = (path: string, layer: ExpressLayer): string => { + const stackLayer = Array.isArray(layer.handle?.stack) ? layer.handle?.stack?.[0] : undefined; + + if (stackLayer?.route?.path) { + return `${path}${stackLayer.route.path}`; + } + + if (stackLayer && Array.isArray(stackLayer?.handle?.stack)) { + return getRouterPath(path, stackLayer); + } + + return path; +}; + +/** + * Check whether the given request is ignored by configuration + * It will not re-throw exceptions from `list` provided by the client + * @param constant e.g URL of request + * @param [list] List of ignore patterns + * @param [onException] callback for doing something when an exception has + * occurred + */ +export type ExpressIsLayerIgnoredOptions = Pick; +export const isLayerIgnored = ( + name: string, + type: ExpressLayerType, + config?: ExpressIsLayerIgnoredOptions, +): boolean => { + if (Array.isArray(config?.ignoreLayersType) && config?.ignoreLayersType?.includes(type)) { + return true; + } + if (!Array.isArray(config?.ignoreLayers)) return false; + try { + for (const pattern of config.ignoreLayers) { + if (satisfiesPattern(name, pattern)) { + return true; + } + } + } catch {} + + return false; +}; + +/** + * Extracts the actual matched route from Express request for OpenTelemetry instrumentation. + * Returns the route that should be used as the http.route attribute. + * + * @param req - The Express request object with layers store + * @param constructedRoute - The constructed route from `getConstructedRoute` + * @returns The matched route string or undefined if no valid route is found + */ +export function getActualMatchedRoute(req: ExpressRequest, constructedRoute: string): string | undefined { + const layersStore = getStoredLayers(req); + + // If no layers are stored, no route can be determined + if (layersStore.length === 0) { + return undefined; + } + + // Handle root path case - if all paths are root, only return root if originalUrl is also root + // The layer store also includes root paths in case a non-existing url was requested + if (layersStore.every(path => path === '/')) { + return req.originalUrl === '/' ? '/' : undefined; + } + + if (constructedRoute === '*') { + return constructedRoute; + } + + // For RegExp routes or route arrays, return the constructed route + // This handles the case where the route is defined using RegExp or an array + if ( + constructedRoute.includes('/') && + (constructedRoute.includes(',') || + constructedRoute.includes('\\') || + constructedRoute.includes('*') || + constructedRoute.includes('[')) + ) { + return constructedRoute; + } + + // Ensure route starts with '/' if it doesn't already + const normalizedRoute = constructedRoute.startsWith('/') ? constructedRoute : `/${constructedRoute}`; + + // Validate that this appears to be a matched route + // A route is considered matched if: + // 1. We have a constructed route + // 2. The original URL matches or starts with our route pattern + const isValidRoute = + normalizedRoute.length > 0 && + (req.originalUrl === normalizedRoute || + req.originalUrl.startsWith(normalizedRoute) || + isRoutePattern(normalizedRoute)); + + return isValidRoute ? normalizedRoute : undefined; +} + +export function getConstructedRoute(req: ExpressRequest) { + const layersStore: string[] = getStoredLayers(req); + + const meaningfulPaths = layersStore.filter(path => path !== '/' && path !== '/*'); + + if (meaningfulPaths.length === 1 && meaningfulPaths[0] === '*') { + return '*'; + } + + // Join parts and remove duplicate slashes + return meaningfulPaths.join('').replace(/\/{2,}/g, '/'); +} + +export const getLayerPath = (args: unknown[]): string | undefined => { + const firstArg = args[0]; + + if (Array.isArray(firstArg)) { + return firstArg.map(arg => extractLayerPathSegment(arg) || '').join(','); + } + + return extractLayerPathSegment(firstArg as LayerPathSegment); +}; + +const extractLayerPathSegment = (arg: LayerPathSegment): string | undefined => + typeof arg === 'string' ? arg : arg instanceof RegExp || typeof arg === 'number' ? String(arg) : undefined; + +// v5 we instrument Router.prototype +// v4 we instrument Router itself +export const isExpressWithRouterPrototype = (express: unknown): express is ExpressExportv5 => + isExpressRouterPrototype((express as ExpressExportv5)?.Router?.prototype); + +// In Express v4, Router is a function (not a plain object), so we need to accept both +const isExpressRouterPrototype = (routerProto?: unknown): routerProto is ExpressRouterv4 => + (typeof routerProto === 'object' || typeof routerProto === 'function') && + !!routerProto && + 'route' in routerProto && + typeof (routerProto as ExpressRouterv4).route === 'function'; + +export const isExpressWithoutRouterPrototype = (express: unknown): express is ExpressExportv4 => + isExpressRouterPrototype((express as ExpressExportv4).Router) && !isExpressWithRouterPrototype(express); + +// dynamic puts the default on .default, require or normal import are fine +export const hasDefaultProp = ( + express: unknown, +): express is { + [k: string]: unknown; + default: ExpressExport; +} => !!express && typeof express === 'object' && 'default' in express && typeof express.default === 'function'; + +export function getSpanName(info: ExpressRequestInfo, defaultName: string): string { + if (getIsolationScope() === getDefaultIsolationScope()) { + DEBUG_BUILD && debug.warn('Isolation scope is still default isolation scope - skipping setting transactionName'); + return defaultName; + } + if (info.layerType === 'request_handler') { + // type cast b/c Otel unfortunately types info.request as any :( + const req = info.request as { method?: string }; + const method = req.method ? req.method.toUpperCase() : 'GET'; + getIsolationScope().setTransactionName(`${method} ${info.route}`); + } + return defaultName; +} + +function getStatusCodeFromResponse(error: MiddlewareError): number { + const statusCode = error.status || error.statusCode || error.status_code || error.output?.statusCode; + return statusCode ? parseInt(statusCode as string, 10) : 500; +} + +/** Returns true if response code is internal server error */ +export function defaultShouldHandleError(error: MiddlewareError): boolean { + const status = getStatusCodeFromResponse(error); + return status >= 500; +} diff --git a/packages/core/test/lib/integrations/express/index.test.ts b/packages/core/test/lib/integrations/express/index.test.ts new file mode 100644 index 000000000000..8fe42dd19700 --- /dev/null +++ b/packages/core/test/lib/integrations/express/index.test.ts @@ -0,0 +1,359 @@ +import { + patchExpressModule, + unpatchExpressModule, + expressErrorHandler, + setupExpressErrorHandler, +} from '../../../../src/integrations/express/index'; + +import { describe, it, expect, vi } from 'vitest'; +import type { + ExpressIntegrationOptions, + ExpressExportv5, + ExpressExportv4, + ExpressLayer, + ExpressModuleExport, + ExpressRoute, + ExpressRouterv4, + ExpressRouterv5, + ExpressResponse, + ExpressRequest, + ExpressMiddleware, + ExpressErrorMiddleware, + ExpressHandlerOptions, +} from '../../../../src/integrations/express/types'; +import type { WrappedFunction } from '../../../../src/types-hoist/wrappedfunction'; +import type { IncomingMessage, ServerResponse } from 'http'; + +const sdkProcessingMetadata: unknown[] = []; +const isolationScope = { + setSDKProcessingMetadata({ normalizedRequest }: { normalizedRequest: unknown }) { + sdkProcessingMetadata.push(normalizedRequest); + }, +}; + +vi.mock('../../../../src/currentScopes', () => ({ + getIsolationScope() { + return isolationScope; + }, +})); + +const capturedExceptions: [unknown, unknown][] = []; +vi.mock('../../../../src/exports', () => ({ + captureException(error: unknown, hint: unknown) { + capturedExceptions.push([error, hint]); + return 'eventId'; + }, +})); + +vi.mock('../../../../src/debug-build', () => ({ + DEBUG_BUILD: true, +})); +const debugErrors: [string, Error][] = []; +vi.mock('../../../../src/utils/debug-logger', () => ({ + debug: { + error: (msg: string, er: Error) => { + debugErrors.push([msg, er]); + }, + }, +})); + +const patchLayerCalls: [options: ExpressIntegrationOptions, layer: ExpressLayer, layerPath?: string][] = []; + +vi.mock('../../../../src/integrations/express/patch-layer', () => ({ + patchLayer: (options: ExpressIntegrationOptions, layer?: ExpressLayer, layerPath?: string) => { + if (layer) { + patchLayerCalls.push([options, layer, layerPath]); + } + }, +})); + +describe('(un)patchExpressModule', () => { + it('throws trying to patch/unpatch the wrong thing', () => { + expect(() => { + patchExpressModule({ + express: {} as unknown as ExpressModuleExport, + } as unknown as ExpressIntegrationOptions); + }).toThrowError('no valid Express route function to instrument'); + expect(() => { + unpatchExpressModule({ + express: {}, + } as unknown as ExpressIntegrationOptions); + }).toThrowError('no valid Express route function to deinstrument'); + }); + + // these are called in the unit tests below, so set spies + const routerv4route = vi.fn(); + const routerv4use = vi.fn(); + const appv4use = vi.fn(); + const expressv4 = Object.assign(function express() {}, { + application: { use: appv4use }, + Router: Object.assign(function Router() {}, { + route: routerv4route, + use: routerv4use, + stack: [{ name: 'layer0' }, { name: 'layer1' }, { name: 'layerFinal' }], + }), + }) as unknown as ExpressExportv4; + Object.assign(expressv4.application, { _router: expressv4.Router }); + + const appv5use = vi.fn(); + const expressv5 = Object.assign(function express() {}, { + application: { use: appv5use }, + Router: class Router { + stack: ExpressLayer[] = []; + route() {} + use() {} + }, + }) as unknown as ExpressExportv5; + Object.assign(expressv5.application, { + router: { + stack: [{ name: 'layer0' }, { name: 'layer1' }, { name: 'layerFinal' }], + }, + }); + + it('can patch and restore expressv4 style module', () => { + const r = expressv4.Router as ExpressRouterv4; + const a = expressv4.application; + for (const module of [expressv4, { default: expressv4 }]) { + const options = { express: module } as unknown as ExpressIntegrationOptions; + expect((r.use as WrappedFunction).__sentry_original__).toBe(undefined); + expect((r.route as WrappedFunction).__sentry_original__).toBe(undefined); + expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined); + + patchExpressModule(options); + + expect(typeof (r.use as WrappedFunction).__sentry_original__).toBe('function'); + expect(typeof (r.route as WrappedFunction).__sentry_original__).toBe('function'); + expect(typeof (a.use as WrappedFunction).__sentry_original__).toBe('function'); + + unpatchExpressModule(options); + + expect((r.use as WrappedFunction).__sentry_original__).toBe(undefined); + expect((r.route as WrappedFunction).__sentry_original__).toBe(undefined); + expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined); + } + }); + + it('can patch and restore expressv5 style module', () => { + const r = expressv5.Router as ExpressRouterv5; + const a = expressv5.application; + for (const module of [expressv5, { default: expressv5 }]) { + const options = { express: module } as unknown as ExpressIntegrationOptions; + expect((r.prototype.use as WrappedFunction).__sentry_original__).toBe(undefined); + expect((r.prototype.route as WrappedFunction).__sentry_original__).toBe(undefined); + expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined); + + patchExpressModule(options); + + expect(typeof (r.prototype.use as WrappedFunction).__sentry_original__).toBe('function'); + expect(typeof (r.prototype.route as WrappedFunction).__sentry_original__).toBe('function'); + expect(typeof (a.use as WrappedFunction).__sentry_original__).toBe('function'); + + unpatchExpressModule(options); + + expect((r.prototype.use as WrappedFunction).__sentry_original__).toBe(undefined); + expect((r.prototype.route as WrappedFunction).__sentry_original__).toBe(undefined); + expect((a.use as WrappedFunction).__sentry_original__).toBe(undefined); + } + }); + + it('calls patched and original Router.route', () => { + const options = { express: expressv4 }; + patchExpressModule(options); + expressv4.Router.route('a'); + unpatchExpressModule(options); + expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]); + patchLayerCalls.length = 0; + expect(routerv4route).toHaveBeenCalledExactlyOnceWith('a'); + routerv4route.mockReset(); + }); + + it('calls patched and original Router.use', () => { + const options = { express: expressv4 }; + patchExpressModule(options); + expressv4.Router.use('a'); + unpatchExpressModule(options); + expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]); + patchLayerCalls.length = 0; + expect(routerv4use).toHaveBeenCalledExactlyOnceWith('a'); + routerv4use.mockReset(); + }); + + it('skips patchLayer call in Router.use if no layer in the stack', () => { + const options = { express: expressv4 }; + patchExpressModule(options); + const { stack } = expressv4.Router; + expressv4.Router.stack = []; + expressv4.Router.use('a'); + expressv4.Router.stack = stack; + unpatchExpressModule(options); + expect(patchLayerCalls).toStrictEqual([]); + patchLayerCalls.length = 0; + expect(routerv4use).toHaveBeenCalledExactlyOnceWith('a'); + routerv4use.mockReset(); + }); + + it('calls patched and original application.use', () => { + const options = { express: expressv4 }; + patchExpressModule(options); + expressv4.application.use('a'); + unpatchExpressModule(options); + expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]); + patchLayerCalls.length = 0; + expect(appv4use).toHaveBeenCalledExactlyOnceWith('a'); + appv4use.mockReset(); + }); + + it('calls patched and original application.use on express v5', () => { + const options = { express: expressv5 }; + patchExpressModule(options); + expressv5.application.use('a'); + unpatchExpressModule(options); + expect(patchLayerCalls).toStrictEqual([[options, { name: 'layerFinal' }, 'a']]); + patchLayerCalls.length = 0; + expect(appv5use).toHaveBeenCalledExactlyOnceWith('a'); + appv5use.mockReset(); + }); + + it('skips patchLayer on application.use if no router found', () => { + const options = { express: expressv4 }; + patchExpressModule(options); + const app = expressv4.application as { + _router?: ExpressRoute; + }; + const { _router } = app; + delete app._router; + expressv4.application.use('a'); + unpatchExpressModule(options); + app._router = _router; + // no router, so no layers to patch! + expect(patchLayerCalls).toStrictEqual([]); + patchLayerCalls.length = 0; + expect(appv4use).toHaveBeenCalledExactlyOnceWith('a'); + appv4use.mockReset(); + }); + + it('debug error when unpatching fails', () => { + unpatchExpressModule({ express: expressv5 }); + expect(debugErrors).toStrictEqual([ + ['Failed to unpatch express route method:', new Error('Method route is not wrapped, and cannot be unwrapped')], + ['Failed to unpatch express use method:', new Error('Method use is not wrapped, and cannot be unwrapped')], + [ + 'Failed to unpatch express application.use method:', + new Error('Method use is not wrapped, and cannot be unwrapped'), + ], + ]); + debugErrors.length = 0; + }); + + it('debug error when patching fails', () => { + patchExpressModule({ express: expressv5 }); + patchExpressModule({ express: expressv5 }); + expect(debugErrors).toStrictEqual([ + ['Failed to patch express route method:', new Error('Attempting to wrap method route multiple times')], + ['Failed to patch express use method:', new Error('Attempting to wrap method use multiple times')], + ['Failed to patch express application.use method:', new Error('Attempting to wrap method use multiple times')], + ]); + }); +}); + +describe('expressErrorHandler', () => { + it('handles the error if it should', () => { + const errorMiddleware = expressErrorHandler(); + const res = { status: 500 } as unknown as ExpressResponse; + const next = vi.fn(); + const err = new Error('err'); + const req = { headers: { request: 'headers' } } as unknown as ExpressRequest; + errorMiddleware(err, req, res, next); + expect((res as unknown as { sentry: string }).sentry).toBe('eventId'); + expect(capturedExceptions).toStrictEqual([ + [ + new Error('err'), + { + mechanism: { + handled: false, + type: 'auto.middleware.express', + }, + }, + ], + ]); + capturedExceptions.length = 0; + expect(sdkProcessingMetadata).toStrictEqual([ + { + url: undefined, + method: undefined, + query_string: undefined, + headers: Object.assign(Object.create(null), { request: 'headers' }), + cookies: undefined, + data: undefined, + }, + ]); + sdkProcessingMetadata.length = 0; + expect(next).toHaveBeenCalledExactlyOnceWith(err); + next.mockReset(); + }); + + it('does not the error if it should not', () => { + const errorMiddleware = expressErrorHandler({ + shouldHandleError: () => false, + }); + const res = { status: 500 } as unknown as ExpressResponse; + const req = { headers: { request: 'headers' } } as unknown as ExpressRequest; + const next = vi.fn(); + const err = new Error('err'); + errorMiddleware(err, req, res, next); + expect((res as unknown as { sentry?: string }).sentry).toBe(undefined); + expect(capturedExceptions).toStrictEqual([]); + expect(sdkProcessingMetadata).toStrictEqual([ + { + url: undefined, + method: undefined, + query_string: undefined, + headers: Object.assign(Object.create(null), { request: 'headers' }), + cookies: undefined, + data: undefined, + }, + ]); + sdkProcessingMetadata.length = 0; + expect(next).toHaveBeenCalledExactlyOnceWith(err); + next.mockReset(); + }); +}); + +describe('setupExpressErrorHandler', () => { + const appUseCalls: unknown[] = []; + const app = { + use: vi.fn((fn: unknown) => appUseCalls.push(fn)) as ( + middleware: ExpressMiddleware | ExpressErrorMiddleware, + ) => unknown, + }; + const options = {} as ExpressHandlerOptions; + it('should have a test here lolz', () => { + setupExpressErrorHandler(app, options); + expect(app.use).toHaveBeenCalledTimes(2); + const reqHandler = appUseCalls[0]; + expect(typeof reqHandler).toBe('function'); + const next = vi.fn(); + (reqHandler as (request: IncomingMessage, _res: ServerResponse, next: () => void) => void)( + { + method: 'GET', + headers: { request: 'headers' }, + } as unknown as ExpressRequest, + {} as unknown as ExpressResponse, + next, + ); + expect(next).toHaveBeenCalledOnce(); + expect(sdkProcessingMetadata).toStrictEqual([ + { + cookies: undefined, + data: undefined, + headers: Object.assign(Object.create(null), { + request: 'headers', + }), + method: 'GET', + query_string: undefined, + url: undefined, + }, + ]); + sdkProcessingMetadata.length = 0; + }); +}); diff --git a/packages/core/test/lib/integrations/express/patch-layer.test.ts b/packages/core/test/lib/integrations/express/patch-layer.test.ts new file mode 100644 index 000000000000..e24d3586507b --- /dev/null +++ b/packages/core/test/lib/integrations/express/patch-layer.test.ts @@ -0,0 +1,423 @@ +import { describe, it, expect, vi } from 'vitest'; +import { type ExpressPatchLayerOptions, patchLayer } from '../../../../src/integrations/express/patch-layer'; +import { + type ExpressRequest, + type ExpressLayer, + kLayerPatched, + type ExpressResponse, +} from '../../../../src/integrations/express/types'; +import { getStoredLayers, storeLayer } from '../../../../src/integrations/express/request-layer-store'; +import { type StartSpanOptions } from '../../../../src/types-hoist/startSpanOptions'; +import { type Span } from '../../../../src/types-hoist/span'; +import { EventEmitter } from 'node:events'; + +const mockSpans: MockSpan[] = []; +class MockSpan { + ended = false; + status: { code: number; message: string } = { code: 0, message: 'OK' }; + attributes: Record; + name: string; + + constructor(options: StartSpanOptions) { + this.name = options.name; + this.attributes = options.attributes ?? {}; + } + + updateName(name: string) { + this.name = name; + return this; + } + + setStatus(status: { code: number; message: string }) { + this.status = status; + } + + setAttributes(o: Record) { + for (const [k, v] of Object.entries(o)) { + this.setAttribute(k, v); + } + } + + setAttribute(key: string, value: unknown) { + this.attributes[key] = value; + } + + end() { + if (this.ended) throw new Error('ended span multiple times!'); + this.ended = true; + } + getSpanJSON(): MockSpanJSON { + // not the whole thing obviously, just enough to know we called it + return { + status: this.status, + data: this.attributes, + description: this.name, + }; + } +} +type MockSpanJSON = { + status?: { code: number; message: string }; + description: string; + data: Record; +}; + +/** verify we get all the expected spans and no more */ +const checkSpans = (expectations: Partial[]) => { + for (const exp of expectations) { + const span = mockSpans.pop()?.getSpanJSON(); + expect(span).toMatchObject(exp); + } + expect(mockSpans.map(m => m.getSpanJSON())).toStrictEqual([]); +}; + +let hasActiveSpan = true; +const parentSpan = {}; +vi.mock('../../../../src/utils/spanUtils', async () => ({ + ...(await import('../../../../src/utils/spanUtils')), + getActiveSpan() { + return hasActiveSpan ? parentSpan : undefined; + }, +})); + +vi.mock('../../../../src/tracing', () => ({ + SPAN_STATUS_ERROR: 2, + withActiveSpan(span: unknown, cb: Function) { + expect(span).toBe(parentSpan); + return cb(); + }, + startSpanManual(options: StartSpanOptions, callback: (span: Span) => T): T { + const span = new MockSpan(options); + mockSpans.push(span); + return callback(span as unknown as Span); + }, +})); + +describe('patchLayer', () => { + describe('no-ops', () => { + it('if layer is missing', () => { + // mostly for coverage, verifying it doesn't throw or anything + patchLayer({}); + }); + + it('if layer already patched', () => { + // mostly for coverage, verifying it doesn't throw or anything + const layer = { + [kLayerPatched]: true, + } as unknown as ExpressLayer; + patchLayer({}, layer); + }); + + it('if layer handler of length 4', () => { + // TODO: this should be expanded when we instrument error handlers + const layer = { + handle(_1: unknown, _2: unknown, _3: unknown, _4: unknown) {}, + } as unknown as ExpressLayer; + patchLayer({}, layer); + expect(layer[kLayerPatched]).toBe(true); + }); + }); + + it('ignores when no parent span has been started', () => { + hasActiveSpan = false; + const options: ExpressPatchLayerOptions = {}; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c', + }) as unknown as ExpressRequest; + + const layerHandleOriginal = vi.fn(); + const layer = { + name: 'mw', + handle: layerHandleOriginal, + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, 'a'); + storeLayer(req, '/:boo'); + storeLayer(req, '/:car'); + + patchLayer(options, layer); + layer.handle(req, res); + expect(layerHandleOriginal).toHaveBeenCalledOnce(); + + // should not have emitted any spans, it was ignored. + checkSpans([]); + + hasActiveSpan = true; + }); + + it('ignores layers that should be ignored, runs otherwise', () => { + const onRouteResolved = vi.fn(); + const options: ExpressPatchLayerOptions = { + onRouteResolved, + ignoreLayersType: ['middleware'], + }; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c/layerPath', + }) as unknown as ExpressRequest; + + const layerHandleOriginal = vi.fn(); + const layer = { + name: 'mw', + handle: layerHandleOriginal, + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, 'a'); + storeLayer(req, '/:boo'); + storeLayer(req, '/:car'); + + patchLayer(options, layer, '/layerPath'); + layer.handle(req, res); + expect(onRouteResolved).toHaveBeenCalledExactlyOnceWith('/a/:boo/:car/layerPath'); + expect(layerHandleOriginal).toHaveBeenCalledOnce(); + + // should not have emitted any spans, it was ignored. + checkSpans([]); + options.ignoreLayersType = []; + layer.handle(req, res); + const span = mockSpans[0]; + expect(span?.ended).toBe(false); + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': 'mw', + 'express.type': 'middleware', + 'http.route': '/a/:boo/:car/layerPath', + 'sentry.op': 'middleware.express', + 'sentry.origin': 'auto.http.express', + }, + description: 'mw', + }, + ]); + res.emit('finish'); + expect(span?.ended).toBe(true); + checkSpans([]); + }); + + it('works with layerPath field', () => { + const onRouteResolved = vi.fn(); + const options: ExpressPatchLayerOptions = { onRouteResolved }; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c/d', + }) as unknown as ExpressRequest; + + const layerHandleOriginal = vi.fn(); + const layer = { + name: 'mw', + handle: layerHandleOriginal, + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + + patchLayer(options, layer, '/c'); + layer.handle(req, res); + expect(onRouteResolved).toHaveBeenCalledExactlyOnceWith('/a/b/c'); + const span = mockSpans[0]; + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': 'mw', + 'express.type': 'middleware', + 'http.route': '/a/b/c', + 'sentry.op': 'middleware.express', + 'sentry.origin': 'auto.http.express', + }, + description: 'mw', + }, + ]); + expect(span?.ended).toBe(false); + res.emit('finish'); + expect(span?.ended).toBe(true); + checkSpans([]); + }); + + it('handles case when route does not match url', () => { + const onRouteResolved = vi.fn(); + const options: ExpressPatchLayerOptions = { onRouteResolved }; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/abcdef', + }) as unknown as ExpressRequest; + + const layerHandleOriginal = vi.fn(); + const layer = { + name: 'router', + handle: layerHandleOriginal, + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + + patchLayer(options, layer, '/c'); + layer.handle(req, res); + expect(onRouteResolved).toHaveBeenCalledExactlyOnceWith(undefined); + const span = mockSpans[0]; + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': '/c', + 'express.type': 'router', + 'sentry.op': 'router.express', + 'sentry.origin': 'auto.http.express', + }, + description: '/c', + }, + ]); + expect(span?.ended).toBe(true); + checkSpans([]); + }); + + it('wraps the callback', () => { + const options: ExpressPatchLayerOptions = {}; + + const layerHandleOriginal = vi.fn((...args) => { + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b', '/c']); + (args[3] as Function)(); + // removes the added layer when the cb indicates it's done + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']); + }); + + const layer = { + name: 'mw', + handle: layerHandleOriginal, + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c', + res, + route: {}, + }) as unknown as ExpressRequest; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + patchLayer(options, layer, '/c'); + + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']); + const callback = vi.fn(() => { + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']); + }); + layer.handle(req, res, 'random', callback, 'whatever'); + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']); + + const span = mockSpans[0]; + checkSpans([ + { + status: { code: 0, message: 'OK' }, + data: { + 'express.name': 'mw', + 'express.type': 'middleware', + 'sentry.op': 'middleware.express', + 'sentry.origin': 'auto.http.express', + }, + description: 'mw', + }, + ]); + expect(span?.ended).toBe(true); + checkSpans([]); + }); + + it('handles callback being called with an error', () => { + const options: ExpressPatchLayerOptions = {}; + + const layerHandleOriginal = vi.fn((...args) => { + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b', '/c']); + (args[3] as Function)(new Error('oopsie')); + // do not remove extra layer if this is where it failed though! + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b', '/c']); + }); + + const layer = { + name: 'mw', + handle: layerHandleOriginal, + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c', + res, + route: {}, + }) as unknown as ExpressRequest; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + patchLayer(options, layer, '/c'); + + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b']); + const callback = vi.fn(() => { + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b', '/c']); + }); + layer.handle(req, res, 'random', callback, 'whatever'); + expect(getStoredLayers(req)).toStrictEqual(['/a', '/b', '/c']); + + const span = mockSpans[0]; + checkSpans([ + { + status: { code: 2, message: 'oopsie' }, + data: { + 'express.name': 'mw', + 'express.type': 'middleware', + 'sentry.op': 'middleware.express', + 'sentry.origin': 'auto.http.express', + }, + description: 'mw', + }, + ]); + expect(span?.ended).toBe(true); + checkSpans([]); + }); + + it('handles throws in layer.handle', () => { + const onRouteResolved = vi.fn(); + const options: ExpressPatchLayerOptions = { onRouteResolved }; + const req = Object.assign(new EventEmitter(), { + originalUrl: '/a/b/c/d', + }) as unknown as ExpressRequest; + + const layerHandleOriginal = vi.fn(() => { + throw new Error('yur head asplode'); + }); + const layer = { + name: 'mw', + handle: layerHandleOriginal, + } as unknown as ExpressLayer; + + const res = Object.assign(new EventEmitter(), {}) as unknown as ExpressResponse; + + storeLayer(req, '/a'); + storeLayer(req, '/b'); + + patchLayer(options, layer, '/c'); + expect(() => { + layer.handle(req, res); + }).toThrowError('yur head asplode'); + expect(onRouteResolved).toHaveBeenCalledExactlyOnceWith('/a/b/c'); + const span = mockSpans[0]; + checkSpans([ + { + status: { code: 2, message: 'yur head asplode' }, + data: { + 'express.name': 'mw', + 'express.type': 'middleware', + 'http.route': '/a/b/c', + 'sentry.op': 'middleware.express', + 'sentry.origin': 'auto.http.express', + }, + description: 'mw', + }, + ]); + expect(span?.ended).toBe(false); + res.emit('finish'); + expect(span?.ended).toBe(true); + checkSpans([]); + }); +}); diff --git a/packages/core/test/lib/integrations/express/request-layer-store.test.ts b/packages/core/test/lib/integrations/express/request-layer-store.test.ts new file mode 100644 index 000000000000..c2f58ac0e6f4 --- /dev/null +++ b/packages/core/test/lib/integrations/express/request-layer-store.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from 'vitest'; +import type { ExpressRequest } from '../../../../src/integrations/express/types'; +import { getStoredLayers, storeLayer } from '../../../../src/integrations/express/request-layer-store'; + +describe('storeLayer', () => { + it('handles case when nothing stored yet', () => { + const req = {} as unknown as ExpressRequest; + const empty = getStoredLayers(req); + expect(empty).toStrictEqual([]); + expect(getStoredLayers(req)).toStrictEqual([]); + }); + it('stores layer for a request', () => { + const req = {} as unknown as ExpressRequest; + storeLayer(req, 'a'); + storeLayer(req, 'b'); + expect(getStoredLayers(req)).toStrictEqual(['a', 'b']); + }); +}); diff --git a/packages/core/test/lib/integrations/express/types.test.ts b/packages/core/test/lib/integrations/express/types.test.ts new file mode 100644 index 000000000000..fa5bd378ded0 --- /dev/null +++ b/packages/core/test/lib/integrations/express/types.test.ts @@ -0,0 +1,18 @@ +import * as types from '../../../../src/integrations/express/types'; +import { describe, it, expect } from 'vitest'; + +// this is mostly just a types-bag, but it does have some keys and such. +describe('types', () => { + it('exports some stuff', () => { + const { kLayerPatched, ...vals } = types; + expect(String(kLayerPatched)).toBe('Symbol(express-layer-patched)'); + expect(vals).toStrictEqual({ + ATTR_EXPRESS_NAME: 'express.name', + ATTR_HTTP_ROUTE: 'http.route', + ATTR_EXPRESS_TYPE: 'express.type', + ExpressLayerType_ROUTER: 'router', + ExpressLayerType_MIDDLEWARE: 'middleware', + ExpressLayerType_REQUEST_HANDLER: 'request_handler', + }); + }); +}); diff --git a/packages/core/test/lib/integrations/express/utils.test.ts b/packages/core/test/lib/integrations/express/utils.test.ts new file mode 100644 index 000000000000..9b866aadccaa --- /dev/null +++ b/packages/core/test/lib/integrations/express/utils.test.ts @@ -0,0 +1,554 @@ +import { storeLayer } from '../../../../src/integrations/express/request-layer-store'; +import { + ATTR_EXPRESS_NAME, + ATTR_EXPRESS_TYPE, + type MiddlewareError, + type ExpressIntegrationOptions, + type ExpressLayer, + type ExpressRequest, + type ExpressRequestInfo, +} from '../../../../src/integrations/express/types'; +import { + asErrorAndMessage, + defaultShouldHandleError, + getActualMatchedRoute, + getConstructedRoute, + getLayerMetadata, + getLayerPath, + getRouterPath, + getSpanName, + hasDefaultProp, + isExpressWithoutRouterPrototype, + isExpressWithRouterPrototype, + isLayerIgnored, + isRoutePattern, + satisfiesPattern, +} from '../../../../src/integrations/express/utils'; + +import { describe, it, expect, vi } from 'vitest'; + +describe('asErrorAndMessage', () => { + it('returns an Error with its message', () => { + const er = new Error('message'); + expect(asErrorAndMessage(er)).toStrictEqual([er, 'message']); + }); + it('returns an non-Error cast to string', () => { + const er = { + toString() { + return 'message'; + }, + }; + expect(asErrorAndMessage(er)).toStrictEqual(['message', 'message']); + }); +}); + +describe('isRoutePattern', () => { + it('searches for : and *', () => { + expect(isRoutePattern('a:b')).toBe(true); + expect(isRoutePattern('abc*')).toBe(true); + expect(isRoutePattern('abc')).toBe(false); + }); +}); + +describe('satisfiesPattern', () => { + it('matches a string', () => { + expect(satisfiesPattern('a', 'a')).toBe(true); + expect(satisfiesPattern('a', 'b')).toBe(false); + }); + it('matches a regexp', () => { + expect(satisfiesPattern('a', /a/)).toBe(true); + expect(satisfiesPattern('a', /b/)).toBe(false); + }); + it('calls a function', () => { + const m = (s: string) => s === 'a'; + expect(satisfiesPattern('a', m)).toBe(true); + expect(satisfiesPattern('b', m)).toBe(false); + }); + it('throws otherwise', () => { + expect(() => { + //@ts-expect-error test verifies type prevents this mistake + satisfiesPattern('a', {}); + }).toThrowError('Pattern is unsupported datatype'); + }); +}); + +describe('getRouterPath', () => { + it('reconstructs returns path if layer is empty', () => { + expect(getRouterPath('/a', {} as unknown as ExpressLayer)).toBe('/a'); + expect( + getRouterPath('/a', { + handle: {}, + } as unknown as ExpressLayer), + ).toBe('/a'); + expect( + getRouterPath('/a', { + handle: { stack: [] }, + } as unknown as ExpressLayer), + ).toBe('/a'); + expect( + getRouterPath('/a', { + handle: { + stack: [ + { + handle: { + stack: [ + { + handle: { + stack: [], + }, + }, + ], + }, + }, + ], + }, + } as unknown as ExpressLayer), + ).toBe('/a'); + }); + + it('uses the stackLayer route path if present', () => { + expect( + getRouterPath('/a', { + handle: { + stack: [{ route: { path: '/b' } }], + }, + } as unknown as ExpressLayer), + ).toBe('/a/b'); + }); + + it('recurses to search layer stack', () => { + expect( + getRouterPath('/a', { + handle: { + stack: [ + { + handle: { + stack: [ + { + handle: { + stack: [{ route: { path: '/b' } }], + }, + }, + ], + }, + }, + ], + }, + } as unknown as ExpressLayer), + ).toBe('/a/b'); + }); +}); + +describe('getLayerMetadata', () => { + it('returns the metadata from router layer', () => { + expect( + getLayerMetadata('/a', { + name: 'router', + route: { path: '/b' }, + } as unknown as ExpressLayer), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: '/a', + [ATTR_EXPRESS_TYPE]: 'router', + }, + name: 'router - /a', + }); + expect( + getLayerMetadata( + '/a', + { + name: 'router', + route: {}, + } as unknown as ExpressLayer, + '/c', + ), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: '/c', + [ATTR_EXPRESS_TYPE]: 'router', + }, + name: 'router - /c', + }); + expect( + getLayerMetadata('/a', { + name: 'router', + route: {}, + } as unknown as ExpressLayer), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: '/a', + [ATTR_EXPRESS_TYPE]: 'router', + }, + name: 'router - /a', + }); + expect( + getLayerMetadata('', { + name: 'router', + route: {}, + } as unknown as ExpressLayer), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: '/', + [ATTR_EXPRESS_TYPE]: 'router', + }, + name: 'router - /', + }); + expect( + getLayerMetadata('', { + name: 'router', + handle: { + stack: [{ route: { path: '/b' } }], + }, + } as unknown as ExpressLayer), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: '/b', + [ATTR_EXPRESS_TYPE]: 'router', + }, + name: 'router - /b', + }); + expect( + getLayerMetadata('', { + name: 'bound dispatch', + handle: { + stack: [{ route: { path: '/b' } }], + }, + } as unknown as ExpressLayer), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: 'request handler', + [ATTR_EXPRESS_TYPE]: 'request_handler', + }, + name: 'request handler', + }); + expect( + getLayerMetadata('/r', { + name: 'handle', + path: '/l', + handle: { + stack: [{ route: { path: '/b' } }], + }, + } as unknown as ExpressLayer), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: '/r', + [ATTR_EXPRESS_TYPE]: 'request_handler', + }, + name: 'request handler - /r', + }); + expect( + getLayerMetadata( + '', + { + name: 'handle', + path: '/l', + handle: { + stack: [{ route: { path: '/b' } }], + }, + } as unknown as ExpressLayer, + '/x', + ), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: '/x', + [ATTR_EXPRESS_TYPE]: 'request_handler', + }, + name: 'request handler - /x', + }); + expect( + getLayerMetadata( + '', + { + name: 'some_other_thing', + path: '/l', + handle: { + stack: [{ route: { path: '/b' } }], + }, + } as unknown as ExpressLayer, + '/x', + ), + ).toStrictEqual({ + attributes: { + [ATTR_EXPRESS_NAME]: 'some_other_thing', + [ATTR_EXPRESS_TYPE]: 'middleware', + }, + name: 'middleware - some_other_thing', + }); + }); +}); + +describe('isLayerIgnored', () => { + it('ignores layers that include the ignored type', () => { + expect( + isLayerIgnored('x', 'router', { + ignoreLayersType: ['router'], + } as unknown as ExpressIntegrationOptions), + ).toBe(true); + + expect( + isLayerIgnored('x', 'router', { + ignoreLayers: [/^x$/], + } as unknown as ExpressIntegrationOptions), + ).toBe(true); + + expect( + isLayerIgnored('x', 'router', { + ignoreLayers: ['x'], + } as unknown as ExpressIntegrationOptions), + ).toBe(true); + expect( + isLayerIgnored('x', 'router', { + ignoreLayers: [() => true], + } as unknown as ExpressIntegrationOptions), + ).toBe(true); + + expect(isLayerIgnored('x', 'router', {} as unknown as ExpressIntegrationOptions)).toBe(false); + expect( + isLayerIgnored('x', 'router', { + ignoreLayersType: ['middleware'], + } as unknown as ExpressIntegrationOptions), + ).toBe(false); + expect( + isLayerIgnored('x', 'router', { + ignoreLayers: [() => false], + } as unknown as ExpressIntegrationOptions), + ).toBe(false); + expect( + isLayerIgnored('x', 'router', { + ignoreLayers: [ + () => { + throw new Error('x'); + }, + ], + } as unknown as ExpressIntegrationOptions), + ).toBe(false); + }); +}); + +describe('getActualMatchedRoute', () => { + it('handles empty layersStore', () => { + const req = {} as unknown as ExpressRequest; + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe(undefined); + }); + + it('handles case when all stored layers are /', () => { + const req = { originalUrl: '/' } as unknown as ExpressRequest; + storeLayer(req, '/'); + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe('/'); + req.originalUrl = '/other-thing'; + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe(undefined); + }); + + it('returns constructed route if *', () => { + const req = { originalUrl: '/xyz' } as unknown as ExpressRequest; + storeLayer(req, '*'); + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe('*'); + }); + + it('returns constructed route when it looks regexp-ish', () => { + const req = { originalUrl: '/xyz' } as unknown as ExpressRequest; + storeLayer(req, '/\\,[*]/'); + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe('/\\,[*]/'); + }); + + it('ensures constructed route starts with /', () => { + const req = { originalUrl: '/a/b' } as unknown as ExpressRequest; + storeLayer(req, 'a'); + storeLayer(req, '/b'); + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe('/a/b'); + }); + + it('allows routes that contain *', () => { + const req = { originalUrl: '/a/b' } as unknown as ExpressRequest; + storeLayer(req, 'a'); + storeLayer(req, '/:boo'); + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe('/a/:boo'); + }); + + it('returns undefined if invalid', () => { + const req = { originalUrl: '/a/b' } as unknown as ExpressRequest; + storeLayer(req, '/a'); + storeLayer(req, '/c'); + expect(getActualMatchedRoute(req, getConstructedRoute(req))).toBe(undefined); + }); +}); + +describe('getConstructedRoute', () => { + it('returns * when the only meaningful path', () => { + const req = {} as unknown as ExpressRequest; + storeLayer(req, '*'); + // not-meaningful paths + storeLayer(req, '/'); + storeLayer(req, '/*'); + expect(getConstructedRoute(req)).toBe('*'); + }); + + it('joins meaningful paths together', () => { + const req = {} as unknown as ExpressRequest; + storeLayer(req, '/a'); + storeLayer(req, '/b/'); + storeLayer(req, '/*'); + storeLayer(req, '/c'); + expect(getConstructedRoute(req)).toBe('/a/b/c'); + }); +}); + +describe('hasDefaultProp', () => { + it('returns detects the presence of a default function prop', () => { + expect(hasDefaultProp({ default: function express() {} })).toBe(true); + expect(hasDefaultProp({ default: 'other thing' })).toBe(false); + expect(hasDefaultProp({})).toBe(false); + }); +}); + +describe('isExpressWith(out)RouterPrototype', () => { + it('detects what kind of express this is', () => { + expect(isExpressWithoutRouterPrototype({})).toBe(false); + expect( + isExpressWithoutRouterPrototype( + Object.assign(function express() {}, { + Router: Object.assign(function Router() {}, { + route() {}, + }), + }), + ), + ).toBe(true); + expect( + isExpressWithoutRouterPrototype( + Object.assign(function express() {}, { + Router: class Router { + route() {} + }, + }), + ), + ).toBe(false); + expect(isExpressWithRouterPrototype({})).toBe(false); + expect( + isExpressWithRouterPrototype({ + Router: Object.assign(function Router() {}, { + route() {}, + }), + }), + ).toBe(false); + expect( + isExpressWithRouterPrototype({ + Router: class Router { + route() {} + }, + }), + ).toBe(true); + }); +}); + +describe('getLayerPath', () => { + it('extracts the layer path segment from first arg', () => { + expect(getLayerPath(['/x'])).toBe('/x'); + expect(getLayerPath([['/x', '/y']])).toBe('/x,/y'); + expect(getLayerPath([['/x', null, 1, /z/i, '/y']])).toBe('/x,,1,/z/i,/y'); + }); +}); + +describe('defaultShouldHandleError', () => { + it('returns true if the response status code is 500', () => { + // just a wrapper to not have to type this out each time. + const _ = (o: unknown): MiddlewareError => o as MiddlewareError; + expect(defaultShouldHandleError(_({ status: 500 }))).toBe(true); + expect(defaultShouldHandleError(_({ statusCode: 500 }))).toBe(true); + expect(defaultShouldHandleError(_({ status_code: 500 }))).toBe(true); + expect(defaultShouldHandleError(_({ output: { statusCode: 500 } }))).toBe(true); + expect(defaultShouldHandleError(_({}))).toBe(true); + expect(defaultShouldHandleError(_({ status: 200 }))).toBe(false); + expect(defaultShouldHandleError(_({ statusCode: 200 }))).toBe(false); + expect(defaultShouldHandleError(_({ status_code: 200 }))).toBe(false); + expect(defaultShouldHandleError(_({ output: { statusCode: 200 } }))).toBe(false); + }); +}); + +let transactionName = ''; +const notDefaultIsolationScope = { + setTransactionName(name: string) { + transactionName = name; + }, +}; +let isolationScope = notDefaultIsolationScope; +const defaultIsolationScope = { + setTransactionName(_: string) { + throw new Error('should not set tx name in default isolation scope'); + }, +}; +vi.mock('../../../../src/debug-build', () => ({ + DEBUG_BUILD: true, +})); +const debugWarnings: string[] = []; +vi.mock('../../../../src/utils/debug-logger', () => ({ + debug: { + warn: (msg: string) => { + debugWarnings.push(msg); + }, + }, +})); +vi.mock('../../../../src/currentScopes', () => ({ + getIsolationScope() { + return isolationScope; + }, +})); +vi.mock('../../../../src/defaultScopes', () => ({ + getDefaultIsolationScope() { + return defaultIsolationScope; + }, +})); +describe('getSpanName', () => { + it('no setting tx name when in default isolation scope', () => { + isolationScope = defaultIsolationScope; + expect(getSpanName({} as unknown as ExpressRequestInfo, 'default')).toBe('default'); + expect(debugWarnings).toStrictEqual([ + 'Isolation scope is still default isolation scope - skipping setting transactionName', + ]); + expect(transactionName).toBe(''); + debugWarnings.length = 0; + }); + + it('no setting tx name when not a request_handler', () => { + isolationScope = notDefaultIsolationScope; + expect(getSpanName({} as unknown as ExpressRequestInfo, 'default')).toBe('default'); + expect(debugWarnings).toStrictEqual([]); + expect(transactionName).toBe(''); + debugWarnings.length = 0; + }); + + it('sets tx name for request_handler in isolation scope', () => { + isolationScope = notDefaultIsolationScope; + expect( + getSpanName( + { + layerType: 'request_handler', + request: { + method: 'put', + }, + route: '/a/:boo', + } as unknown as ExpressRequestInfo, + 'default', + ), + ).toBe('default'); + expect(debugWarnings).toStrictEqual([]); + expect(transactionName).toBe('PUT /a/:boo'); + debugWarnings.length = 0; + }); + + it('method defaults to GET', () => { + isolationScope = notDefaultIsolationScope; + expect( + getSpanName( + { + layerType: 'request_handler', + request: {}, + route: '/a/:boo', + } as unknown as ExpressRequestInfo, + 'default', + ), + ).toBe('default'); + expect(debugWarnings).toStrictEqual([]); + expect(transactionName).toBe('GET /a/:boo'); + debugWarnings.length = 0; + }); +}); diff --git a/packages/node-core/src/utils/ensureIsWrapped.ts b/packages/node-core/src/utils/ensureIsWrapped.ts index 921d01da8207..7c10c1c9cfe7 100644 --- a/packages/node-core/src/utils/ensureIsWrapped.ts +++ b/packages/node-core/src/utils/ensureIsWrapped.ts @@ -1,5 +1,13 @@ import { isWrapped } from '@opentelemetry/instrumentation'; -import { consoleSandbox, getClient, getGlobalScope, hasSpansEnabled, isEnabled } from '@sentry/core'; +import { + consoleSandbox, + getClient, + getOriginalFunction, + getGlobalScope, + hasSpansEnabled, + isEnabled, + type WrappedFunction, +} from '@sentry/core'; import type { NodeClient } from '../sdk/client'; import { createMissingInstrumentationContext } from './createMissingInstrumentationContext'; import { isCjs } from './detection'; @@ -14,7 +22,10 @@ export function ensureIsWrapped( const clientOptions = getClient()?.getOptions(); if ( !clientOptions?.disableInstrumentationWarnings && - !isWrapped(maybeWrappedFunction) && + !( + isWrapped(maybeWrappedFunction) || + typeof getOriginalFunction(maybeWrappedFunction as WrappedFunction) === 'function' + ) && isEnabled() && hasSpansEnabled(clientOptions) ) { diff --git a/packages/node-core/test/utils/ensureIsWrapped.test.ts b/packages/node-core/test/utils/ensureIsWrapped.test.ts index 890b913a4cb2..5fe92218d837 100644 --- a/packages/node-core/test/utils/ensureIsWrapped.test.ts +++ b/packages/node-core/test/utils/ensureIsWrapped.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { ensureIsWrapped } from '../../src/utils/ensureIsWrapped'; import { cleanupOtel, mockSdkInit, resetGlobals } from '../helpers/mockSdkInit'; +import { markFunctionWrapped } from '@sentry/core'; const unwrappedFunction = () => {}; @@ -69,4 +70,24 @@ describe('ensureIsWrapped', () => { expect(spyWarn).toHaveBeenCalledTimes(0); }); + + it('does not warn when the method is wrapped by @sentry/core', () => { + const spyWarn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + mockSdkInit({ tracesSampleRate: 1 }); + + function originalFunction() { + return 'i am original'; + } + + function wrappedFunction() { + return originalFunction(); + } + + markFunctionWrapped(wrappedFunction, originalFunction); + + ensureIsWrapped(wrappedfunction, 'express'); + + expect(spyWarn).toHaveBeenCalledTimes(0); + }); }); diff --git a/packages/node/package.json b/packages/node/package.json index 8100f9fd342a..f6dd7f2a5349 100644 --- a/packages/node/package.json +++ b/packages/node/package.json @@ -72,7 +72,6 @@ "@opentelemetry/instrumentation-amqplib": "0.60.0", "@opentelemetry/instrumentation-connect": "0.56.0", "@opentelemetry/instrumentation-dataloader": "0.30.0", - "@opentelemetry/instrumentation-express": "0.61.0", "@opentelemetry/instrumentation-fs": "0.32.0", "@opentelemetry/instrumentation-generic-pool": "0.56.0", "@opentelemetry/instrumentation-graphql": "0.61.0", diff --git a/packages/node/src/integrations/tracing/express.ts b/packages/node/src/integrations/tracing/express.ts index e645cc24d31e..21d6c3654c0a 100644 --- a/packages/node/src/integrations/tracing/express.ts +++ b/packages/node/src/integrations/tracing/express.ts @@ -1,197 +1,93 @@ -import type * as http from 'node:http'; -import type { Span } from '@opentelemetry/api'; -import type { ExpressRequestInfo } from '@opentelemetry/instrumentation-express'; -import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express'; -import type { IntegrationFn } from '@sentry/core'; +// Automatic istrumentation for Express using OTel +import type { InstrumentationConfig } from '@opentelemetry/instrumentation'; +import { InstrumentationBase, InstrumentationNodeModuleDefinition } from '@opentelemetry/instrumentation'; +import { context } from '@opentelemetry/api'; +import { getRPCMetadata, RPCType } from '@opentelemetry/core'; + +import { ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core'; import { - captureException, + type ExpressIntegrationOptions, + type IntegrationFn, debug, + patchExpressModule, + unpatchExpressModule, + SDK_VERSION, defineIntegration, - getDefaultIsolationScope, - getIsolationScope, - httpRequestToRequestData, - SEMANTIC_ATTRIBUTE_SENTRY_OP, - spanToJSON, + setupExpressErrorHandler as coreSetupExpressErrorHandler, + type ExpressHandlerOptions, } from '@sentry/core'; -import { addOriginToSpan, ensureIsWrapped, generateInstrumentOnce } from '@sentry/node-core'; +export { expressErrorHandler } from '@sentry/core'; import { DEBUG_BUILD } from '../../debug-build'; const INTEGRATION_NAME = 'Express'; +const SUPPORTED_VERSIONS = ['>=4.0.0 <6']; -function requestHook(span: Span): void { - addOriginToSpan(span, 'auto.http.otel.express'); - - const attributes = spanToJSON(span).data; - // this is one of: middleware, request_handler, router - const type = attributes['express.type']; - - if (type) { - span.setAttribute(SEMANTIC_ATTRIBUTE_SENTRY_OP, `${type}.express`); - } - - // Also update the name, we don't need to "middleware - " prefix - const name = attributes['express.name']; - if (typeof name === 'string') { - span.updateName(name); - } +export function setupExpressErrorHandler( + //oxlint-disable-next-line no-explicit-any + app: { use: (middleware: any) => unknown }, + options?: ExpressHandlerOptions, +): void { + coreSetupExpressErrorHandler(app, options); + ensureIsWrapped(app.use, 'express'); } -function spanNameHook(info: ExpressRequestInfo, defaultName: string): string { - if (getIsolationScope() === getDefaultIsolationScope()) { - DEBUG_BUILD && debug.warn('Isolation scope is still default isolation scope - skipping setting transactionName'); - return defaultName; - } - if (info.layerType === 'request_handler') { - // type cast b/c Otel unfortunately types info.request as any :( - const req = info.request as { method?: string }; - const method = req.method ? req.method.toUpperCase() : 'GET'; - getIsolationScope().setTransactionName(`${method} ${info.route}`); - } - return defaultName; -} +export type ExpressInstrumentationConfig = InstrumentationConfig & + Omit; export const instrumentExpress = generateInstrumentOnce( INTEGRATION_NAME, - () => + (options?: ExpressInstrumentationConfig) => new ExpressInstrumentation({ - requestHook: span => requestHook(span), - spanNameHook: (info, defaultName) => spanNameHook(info, defaultName), + ignoreLayers: options?.ignoreLayers, + ignoreLayersType: options?.ignoreLayersType, }), ); -const _expressIntegration = (() => { +export class ExpressInstrumentation extends InstrumentationBase { + public constructor(config: ExpressInstrumentationConfig) { + super('sentry-express', SDK_VERSION, config); + } + public init(): InstrumentationNodeModuleDefinition { + const module = new InstrumentationNodeModuleDefinition( + 'express', + SUPPORTED_VERSIONS, + express => { + try { + patchExpressModule({ + ...this.getConfig(), + express, + onRouteResolved(route) { + const rpcMetadata = getRPCMetadata(context.active()); + if (route && rpcMetadata?.type === RPCType.HTTP) { + rpcMetadata.route = route; + } + }, + }); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to patch express module:', e); + } + return express; + }, + express => { + try { + unpatchExpressModule({ express }); + } catch (e) { + DEBUG_BUILD && debug.error('Failed to unpatch express module:', e); + } + return express; + }, + ); + return module; + } +} + +const _expressInstrumentation = ((options?: ExpressInstrumentationConfig) => { return { name: INTEGRATION_NAME, setupOnce() { - instrumentExpress(); + instrumentExpress(options); }, }; }) satisfies IntegrationFn; -/** - * Adds Sentry tracing instrumentation for [Express](https://expressjs.com/). - * - * If you also want to capture errors, you need to call `setupExpressErrorHandler(app)` after you set up your Express server. - * - * For more information, see the [express documentation](https://docs.sentry.io/platforms/javascript/guides/express/). - * - * @example - * ```javascript - * const Sentry = require('@sentry/node'); - * - * Sentry.init({ - * integrations: [Sentry.expressIntegration()], - * }) - * ``` - */ -export const expressIntegration = defineIntegration(_expressIntegration); - -interface MiddlewareError extends Error { - status?: number | string; - statusCode?: number | string; - status_code?: number | string; - output?: { - statusCode?: number | string; - }; -} - -type ExpressMiddleware = (req: http.IncomingMessage, res: http.ServerResponse, next: () => void) => void; - -type ExpressErrorMiddleware = ( - error: MiddlewareError, - req: http.IncomingMessage, - res: http.ServerResponse, - next: (error: MiddlewareError) => void, -) => void; - -interface ExpressHandlerOptions { - /** - * Callback method deciding whether error should be captured and sent to Sentry - * @param error Captured middleware error - */ - shouldHandleError?(this: void, error: MiddlewareError): boolean; -} - -/** - * An Express-compatible error handler. - */ -export function expressErrorHandler(options?: ExpressHandlerOptions): ExpressErrorMiddleware { - return function sentryErrorMiddleware( - error: MiddlewareError, - request: http.IncomingMessage, - res: http.ServerResponse, - next: (error: MiddlewareError) => void, - ): void { - const normalizedRequest = httpRequestToRequestData(request); - // Ensure we use the express-enhanced request here, instead of the plain HTTP one - // When an error happens, the `expressRequestHandler` middleware does not run, so we set it here too - getIsolationScope().setSDKProcessingMetadata({ normalizedRequest }); - - const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError; - - if (shouldHandleError(error)) { - const eventId = captureException(error, { mechanism: { type: 'auto.middleware.express', handled: false } }); - (res as { sentry?: string }).sentry = eventId; - } - - next(error); - }; -} - -function expressRequestHandler(): ExpressMiddleware { - return function sentryRequestMiddleware( - request: http.IncomingMessage, - _res: http.ServerResponse, - next: () => void, - ): void { - const normalizedRequest = httpRequestToRequestData(request); - // Ensure we use the express-enhanced request here, instead of the plain HTTP one - getIsolationScope().setSDKProcessingMetadata({ normalizedRequest }); - - next(); - }; -} - -/** - * Add an Express error handler to capture errors to Sentry. - * - * The error handler must be before any other middleware and after all controllers. - * - * @param app The Express instances - * @param options {ExpressHandlerOptions} Configuration options for the handler - * - * @example - * ```javascript - * const Sentry = require('@sentry/node'); - * const express = require("express"); - * - * const app = express(); - * - * // Add your routes, etc. - * - * // Add this after all routes, - * // but before any and other error-handling middlewares are defined - * Sentry.setupExpressErrorHandler(app); - * - * app.listen(3000); - * ``` - */ -export function setupExpressErrorHandler( - app: { use: (middleware: ExpressMiddleware | ExpressErrorMiddleware) => unknown }, - options?: ExpressHandlerOptions, -): void { - app.use(expressRequestHandler()); - app.use(expressErrorHandler(options)); - ensureIsWrapped(app.use, 'express'); -} - -function getStatusCodeFromResponse(error: MiddlewareError): number { - const statusCode = error.status || error.statusCode || error.status_code || error.output?.statusCode; - return statusCode ? parseInt(statusCode as string, 10) : 500; -} - -/** Returns true if response code is internal server error */ -function defaultShouldHandleError(error: MiddlewareError): boolean { - const status = getStatusCodeFromResponse(error); - return status >= 500; -} +export const expressIntegration = defineIntegration(_expressInstrumentation); diff --git a/yarn.lock b/yarn.lock index 92512f7c5cc3..0115389ea804 100644 --- a/yarn.lock +++ b/yarn.lock @@ -6251,15 +6251,6 @@ dependencies: "@opentelemetry/instrumentation" "^0.213.0" -"@opentelemetry/instrumentation-express@0.61.0": - version "0.61.0" - resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-express/-/instrumentation-express-0.61.0.tgz#49b4d144ab6e9d6e035941a51f5e573e84e3647f" - integrity sha512-Xdmqo9RZuZlL29Flg8QdwrrX7eW1CZ7wFQPKHyXljNymgKhN1MCsYuqQ/7uxavhSKwAl7WxkTzKhnqpUApLMvQ== - dependencies: - "@opentelemetry/core" "^2.0.0" - "@opentelemetry/instrumentation" "^0.213.0" - "@opentelemetry/semantic-conventions" "^1.27.0" - "@opentelemetry/instrumentation-fs@0.32.0": version "0.32.0" resolved "https://registry.yarnpkg.com/@opentelemetry/instrumentation-fs/-/instrumentation-fs-0.32.0.tgz#2010d86da8ab3d543f8e44c8fff81b94f904d91d" @@ -27485,7 +27476,7 @@ socket.io-parser@~4.2.4: integrity sha512-asJqbVBDsBCJx0pTqw3WfesSY0iRX+2xzWEWzrpcH7L6fLzrhyF8WPI8UaeM4YCuDfpwA/cgsdugMsmtz8EJeg== dependencies: "@socket.io/component-emitter" "~3.1.0" - debug "~4.3.1" + debug "~4.4.1" socket.io@^4.5.4: version "4.8.1" @@ -28288,7 +28279,6 @@ stylus@0.59.0, stylus@^0.59.0: sucrase@^3.27.0, sucrase@^3.35.0, sucrase@getsentry/sucrase#es2020-polyfills: version "3.36.0" - uid fd682f6129e507c00bb4e6319cc5d6b767e36061 resolved "https://codeload.github.com/getsentry/sucrase/tar.gz/fd682f6129e507c00bb4e6319cc5d6b767e36061" dependencies: "@jridgewell/gen-mapping" "^0.3.2"