diff --git a/packages/node/src/handlers.ts b/packages/node/src/handlers.ts index bcdb94467353..de2945c7ca33 100644 --- a/packages/node/src/handlers.ts +++ b/packages/node/src/handlers.ts @@ -2,7 +2,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import { captureException, getCurrentHub, startTransaction, withScope } from '@sentry/core'; import { extractTraceparentData, Span } from '@sentry/tracing'; -import { Event } from '@sentry/types'; +import { Event, Transaction } from '@sentry/types'; import { extractNodeRequestData, forget, @@ -21,6 +21,16 @@ import { flush } from './sdk'; const DEFAULT_SHUTDOWN_TIMEOUT = 2000; +interface ExpressRequest { + route?: { + path: string; + }; + method: string; + originalUrl: string; + baseUrl: string; + query: string; +} + /** * Express-compatible tracing handler. * @see Exposed as `Handlers.tracingHandler` @@ -65,6 +75,7 @@ export function tracingHandler(): ( res.once('finish', () => { // We schedule the immediate execution of the `finish` to let all the spans being closed first. setImmediate(() => { + addExpressReqToTransaction(transaction, (req as unknown) as ExpressRequest); transaction.setHttpStatus(res.statusCode); transaction.finish(); }); @@ -74,6 +85,20 @@ export function tracingHandler(): ( }; } +/** + * Set parameterized as transaction name e.g.: `GET /users/:id` + * Also adds more context data on the transaction from the request + */ +function addExpressReqToTransaction(transaction: Transaction | undefined, req: ExpressRequest): void { + if (!transaction) return; + if (req.route) { + transaction.name = `${req.method} ${req.baseUrl}${req.route.path}`; + } + transaction.setData('url', req.originalUrl); + transaction.setData('baseUrl', req.baseUrl); + transaction.setData('query', req.query); +} + type TransactionTypes = 'path' | 'methodPath' | 'handler'; /** JSDoc */ diff --git a/packages/tracing/src/integrations/express.ts b/packages/tracing/src/integrations/express.ts index b9f6f4beeb75..1c844d41be51 100644 --- a/packages/tracing/src/integrations/express.ts +++ b/packages/tracing/src/integrations/express.ts @@ -1,8 +1,6 @@ -/* eslint-disable @typescript-eslint/no-explicit-any */ import { Integration, Transaction } from '@sentry/types'; import { logger } from '@sentry/utils'; -// Have to manually set types because we are using package-alias type Method = | 'all' | 'get' @@ -27,17 +25,14 @@ type Method = | 'subscribe' | 'trace' | 'unlock' - | 'unsubscribe'; + | 'unsubscribe' + | 'use'; -type Application = { - [method in Method | 'use']: (...args: any) => any; +type Router = { + [method in Method]: (...args: unknown[]) => unknown; }; -type ErrorRequestHandler = (...args: any) => any; -type RequestHandler = (...args: any) => any; -type NextFunction = (...args: any) => any; - -interface Response { +interface ExpressResponse { once(name: string, callback: () => void): void; } @@ -52,8 +47,7 @@ interface SentryTracingResponse { /** * Express integration * - * Provides an request and error handler for Express framework - * as well as tracing capabilities + * Provides an request and error handler for Express framework as well as tracing capabilities */ export class Express implements Integration { /** @@ -69,27 +63,26 @@ export class Express implements Integration { /** * Express App instance */ - private readonly _app?: Application; + private readonly _router?: Router; private readonly _methods?: Method[]; /** * @inheritDoc */ - public constructor(options: { app?: Application; methods?: Method[] } = {}) { - this._app = options.app; - this._methods = options.methods; + public constructor(options: { app?: Router; router?: Router; methods?: Method[] } = {}) { + this._router = options.router || options.app; + this._methods = (Array.isArray(options.methods) ? options.methods : []).concat('use'); } /** * @inheritDoc */ public setupOnce(): void { - if (!this._app) { + if (!this._router) { logger.error('ExpressIntegration is missing an Express instance'); return; } - instrumentMiddlewares(this._app); - routeMiddlewares(this._app, this._methods); + instrumentMiddlewares(this._router, this._methods); } } @@ -104,75 +97,63 @@ export class Express implements Integration { * app.use(function (req, res, next) { ... }) * // error handler * app.use(function (err, req, res, next) { ... }) + * + * They all internally delegate to the `router[method]` of the given application instance. */ -// eslint-disable-next-line @typescript-eslint/ban-types -function wrap(fn: Function): RequestHandler | ErrorRequestHandler { +// eslint-disable-next-line @typescript-eslint/ban-types, @typescript-eslint/no-explicit-any +function wrap(fn: Function, method: Method): (...args: any[]) => void { const arity = fn.length; switch (arity) { case 2: { - return function(this: NodeJS.Global, req: Request, res: Response & SentryTracingResponse): any { + return function(this: NodeJS.Global, req: unknown, res: ExpressResponse & SentryTracingResponse): void { const transaction = res.__sentry_transaction; - addExpressReqToTransaction(transaction, req); if (transaction) { const span = transaction.startChild({ description: fn.name, - op: 'middleware', + op: `middleware.${method}`, }); res.once('finish', () => { span.finish(); }); } - // eslint-disable-next-line prefer-rest-params - return fn.apply(this, arguments); + return fn.call(this, req, res); }; } case 3: { return function( this: NodeJS.Global, - req: Request, - res: Response & SentryTracingResponse, - next: NextFunction, - ): any { + req: unknown, + res: ExpressResponse & SentryTracingResponse, + next: () => void, + ): void { const transaction = res.__sentry_transaction; - addExpressReqToTransaction(transaction, req); - const span = - transaction && - transaction.startChild({ - description: fn.name, - op: 'middleware', - }); - fn.call(this, req, res, function(this: NodeJS.Global): any { - if (span) { - span.finish(); - } - // eslint-disable-next-line prefer-rest-params - return next.apply(this, arguments); + const span = transaction?.startChild({ + description: fn.name, + op: `middleware.${method}`, + }); + fn.call(this, req, res, function(this: NodeJS.Global, ...args: unknown[]): void { + span?.finish(); + next.call(this, ...args); }); }; } case 4: { return function( this: NodeJS.Global, - err: any, + err: Error, req: Request, res: Response & SentryTracingResponse, - next: NextFunction, - ): any { + next: () => void, + ): void { const transaction = res.__sentry_transaction; - addExpressReqToTransaction(transaction, req); - const span = - transaction && - transaction.startChild({ - description: fn.name, - op: 'middleware', - }); - fn.call(this, err, req, res, function(this: NodeJS.Global): any { - if (span) { - span.finish(); - } - // eslint-disable-next-line prefer-rest-params - return next.apply(this, arguments); + const span = transaction?.startChild({ + description: fn.name, + op: `middleware.${method}`, + }); + fn.call(this, err, req, res, function(this: NodeJS.Global, ...args: unknown[]): void { + span?.finish(); + next.call(this, ...args); }); }; } @@ -183,24 +164,7 @@ function wrap(fn: Function): RequestHandler | ErrorRequestHandler { } /** - * Set parameterized as transaction name e.g.: `GET /users/:id` - * Also adds more context data on the transaction from the request - */ -function addExpressReqToTransaction(transaction: Transaction | undefined, req: any): void { - /* eslint-disable @typescript-eslint/no-unsafe-member-access */ - if (transaction) { - if (req.route && req.route.path) { - transaction.name = `${req.method} ${req.route.path}`; - } - transaction.setData('url', req.originalUrl); - transaction.setData('baseUrl', req.baseUrl); - transaction.setData('query', req.query); - } - /* eslint-enable @typescript-eslint/no-unsafe-member-access */ -} - -/** - * Takes all the function arguments passed to the original `app.use` call + * Takes all the function arguments passed to the original `app` or `router` method, eg. `app.use` or `router.use` * and wraps every function, as well as array of functions with a call to our `wrap` method. * We have to take care of the arrays as well as iterate over all of the arguments, * as `app.use` can accept middlewares in few various forms. @@ -209,16 +173,16 @@ function addExpressReqToTransaction(transaction: Transaction | undefined, req: a * app.use([], , ...) * app.use([], ...[]) */ -function wrapUseArgs(args: IArguments): unknown[] { - return Array.from(args).map((arg: unknown) => { +function wrapMiddlewareArgs(args: unknown[], method: Method): unknown[] { + return args.map((arg: unknown) => { if (typeof arg === 'function') { - return wrap(arg); + return wrap(arg, method); } if (Array.isArray(arg)) { return arg.map((a: unknown) => { if (typeof a === 'function') { - return wrap(a); + return wrap(a, method); } return a; }); @@ -229,31 +193,21 @@ function wrapUseArgs(args: IArguments): unknown[] { } /** - * Patches original App to utilize our tracing functionality + * Patches original router to utilize our tracing functionality */ -function patchMiddleware(app: Application, method: Method | 'use'): Application { - const originalAppCallback = app[method]; +function patchMiddleware(router: Router, method: Method): Router { + const originalCallback = router[method]; - app[method] = function(): any { - // eslint-disable-next-line prefer-rest-params - return originalAppCallback.apply(this, wrapUseArgs(arguments)); + router[method] = function(...args: unknown[]): void { + return originalCallback.call(this, ...wrapMiddlewareArgs(args, method)); }; - return app; + return router; } /** - * Patches original app.use + * Patches original router methods */ -function instrumentMiddlewares(app: Application): void { - patchMiddleware(app, 'use'); -} - -/** - * Patches original app.METHOD - */ -function routeMiddlewares(app: Application, methods: Method[] = []): void { - methods.forEach(function(method: Method) { - patchMiddleware(app, method); - }); +function instrumentMiddlewares(router: Router, methods: Method[] = []): void { + methods.forEach((method: Method) => patchMiddleware(router, method)); }