diff --git a/dev-packages/node-integration-tests/suites/express/tracing-experimental/server.js b/dev-packages/node-integration-tests/suites/express/tracing-experimental/server.js index 0f9136789b12..207d11d3aa0e 100644 --- a/dev-packages/node-integration-tests/suites/express/tracing-experimental/server.js +++ b/dev-packages/node-integration-tests/suites/express/tracing-experimental/server.js @@ -34,6 +34,6 @@ app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/ res.send({ response: 'response 4' }); }); -app.use(Sentry.errorHandler()); +Sentry.setupExpressErrorHandler(app); startExpressServerAndSendPortToRunner(app); diff --git a/packages/node-experimental/src/index.ts b/packages/node-experimental/src/index.ts index ef9605afabe7..282b703d1194 100644 --- a/packages/node-experimental/src/index.ts +++ b/packages/node-experimental/src/index.ts @@ -1,5 +1,3 @@ -export { errorHandler } from './sdk/handlers/errorHandler'; - export { httpIntegration } from './integrations/http'; export { nativeNodeFetchIntegration } from './integrations/node-fetch'; @@ -11,7 +9,7 @@ export { modulesIntegration } from './integrations/modules'; export { onUncaughtExceptionIntegration } from './integrations/onuncaughtexception'; export { onUnhandledRejectionIntegration } from './integrations/onunhandledrejection'; -export { expressIntegration } from './integrations/tracing/express'; +export { expressIntegration, expressErrorHandler, setupExpressErrorHandler } from './integrations/tracing/express'; export { fastifyIntegration } from './integrations/tracing/fastify'; export { graphqlIntegration } from './integrations/tracing/graphql'; export { mongoIntegration } from './integrations/tracing/mongo'; diff --git a/packages/node-experimental/src/integrations/tracing/express.ts b/packages/node-experimental/src/integrations/tracing/express.ts index 0606702d8220..12e44199e16d 100644 --- a/packages/node-experimental/src/integrations/tracing/express.ts +++ b/packages/node-experimental/src/integrations/tracing/express.ts @@ -1,8 +1,11 @@ +import type * as http from 'http'; import { registerInstrumentations } from '@opentelemetry/instrumentation'; import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express'; import { defineIntegration } from '@sentry/core'; +import { captureException, getClient, getIsolationScope } from '@sentry/core'; import type { IntegrationFn } from '@sentry/types'; +import type { NodeClient } from '../../sdk/client'; import { addOriginToSpan } from '../../utils/addOriginToSpan'; const _expressIntegration = (() => { @@ -26,5 +29,89 @@ const _expressIntegration = (() => { * Express integration * * Capture tracing data for express. + * In order to capture exceptions, you have to call `setupExpressErrorHandler(app)` before any other middleware and after all controllers. */ 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 = ( + error: MiddlewareError, + req: http.IncomingMessage, + res: http.ServerResponse, + next: (error: MiddlewareError) => void, +) => void; + +/** + * An Express-compatible error handler. + */ +export function expressErrorHandler(options?: { + /** + * Callback method deciding whether error should be captured and sent to Sentry + * @param error Captured middleware error + */ + shouldHandleError?(this: void, error: MiddlewareError): boolean; +}): ExpressMiddleware { + return function sentryErrorMiddleware( + error: MiddlewareError, + _req: http.IncomingMessage, + res: http.ServerResponse, + next: (error: MiddlewareError) => void, + ): void { + const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError; + + if (shouldHandleError(error)) { + const client = getClient(); + if (client && client.getOptions().autoSessionTracking) { + // Check if the `SessionFlusher` is instantiated on the client to go into this branch that marks the + // `requestSession.status` as `Crashed`, and this check is necessary because the `SessionFlusher` is only + // instantiated when the the`requestHandler` middleware is initialised, which indicates that we should be + // running in SessionAggregates mode + const isSessionAggregatesMode = client['_sessionFlusher'] !== undefined; + if (isSessionAggregatesMode) { + const requestSession = getIsolationScope().getRequestSession(); + // If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a + // Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within + // the bounds of a request, and if so the status is updated + if (requestSession && requestSession.status !== undefined) { + requestSession.status = 'crashed'; + } + } + } + + const eventId = captureException(error, { mechanism: { type: 'middleware', handled: false } }); + (res as { sentry?: string }).sentry = eventId; + next(error); + + return; + } + + next(error); + }; +} + +/** + * Setup an error handler for Express. + * The error handler must be before any other middleware and after all controllers. + */ +export function setupExpressErrorHandler(app: { use: (middleware: ExpressMiddleware) => unknown }): void { + app.use(expressErrorHandler()); +} + +function getStatusCodeFromResponse(error: MiddlewareError): number { + const statusCode = error.status || error.statusCode || error.status_code || (error.output && 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; +} diff --git a/packages/node-experimental/src/sdk/handlers/errorHandler.ts b/packages/node-experimental/src/sdk/handlers/errorHandler.ts deleted file mode 100644 index 2460ea11a0e4..000000000000 --- a/packages/node-experimental/src/sdk/handlers/errorHandler.ts +++ /dev/null @@ -1,76 +0,0 @@ -import type * as http from 'http'; -import { captureException, getClient, getIsolationScope } from '@sentry/core'; -import type { NodeClient } from '../client'; - -interface MiddlewareError extends Error { - status?: number | string; - statusCode?: number | string; - status_code?: number | string; - output?: { - statusCode?: number | string; - }; -} - -/** - * An Express-compatible error handler. - */ -export function errorHandler(options?: { - /** - * Callback method deciding whether error should be captured and sent to Sentry - * @param error Captured middleware error - */ - shouldHandleError?(this: void, error: MiddlewareError): boolean; -}): ( - error: MiddlewareError, - req: http.IncomingMessage, - res: http.ServerResponse, - next: (error: MiddlewareError) => void, -) => void { - return function sentryErrorMiddleware( - error: MiddlewareError, - _req: http.IncomingMessage, - res: http.ServerResponse, - next: (error: MiddlewareError) => void, - ): void { - const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError; - - if (shouldHandleError(error)) { - const client = getClient(); - if (client && client.getOptions().autoSessionTracking) { - // Check if the `SessionFlusher` is instantiated on the client to go into this branch that marks the - // `requestSession.status` as `Crashed`, and this check is necessary because the `SessionFlusher` is only - // instantiated when the the`requestHandler` middleware is initialised, which indicates that we should be - // running in SessionAggregates mode - const isSessionAggregatesMode = client['_sessionFlusher'] !== undefined; - if (isSessionAggregatesMode) { - const requestSession = getIsolationScope().getRequestSession(); - // If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a - // Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within - // the bounds of a request, and if so the status is updated - if (requestSession && requestSession.status !== undefined) { - requestSession.status = 'crashed'; - } - } - } - - const eventId = captureException(error, { mechanism: { type: 'middleware', handled: false } }); - (res as { sentry?: string }).sentry = eventId; - next(error); - - return; - } - - next(error); - }; -} - -function getStatusCodeFromResponse(error: MiddlewareError): number { - const statusCode = error.status || error.statusCode || error.status_code || (error.output && 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; -} diff --git a/packages/node-experimental/test/sdk/handlers/errorHandler.test.ts b/packages/node-experimental/test/integrations/express.test.ts similarity index 93% rename from packages/node-experimental/test/sdk/handlers/errorHandler.test.ts rename to packages/node-experimental/test/integrations/express.test.ts index 58c0cf934316..592ab7677db0 100644 --- a/packages/node-experimental/test/sdk/handlers/errorHandler.test.ts +++ b/packages/node-experimental/test/integrations/express.test.ts @@ -1,11 +1,11 @@ import * as http from 'http'; import { getCurrentScope, getIsolationScope, setAsyncContextStrategy, setCurrentClient, withScope } from '@sentry/core'; import type { Scope } from '@sentry/types'; -import { NodeClient } from '../../../src/sdk/client'; -import { errorHandler } from '../../../src/sdk/handlers/errorHandler'; -import { getDefaultNodeClientOptions } from '../../helpers/getDefaultNodeClientOptions'; +import { expressErrorHandler } from '../../src/integrations/tracing/express'; +import { NodeClient } from '../../src/sdk/client'; +import { getDefaultNodeClientOptions } from '../helpers/getDefaultNodeClientOptions'; -describe('errorHandler()', () => { +describe('expressErrorHandler()', () => { beforeEach(() => { getCurrentScope().clear(); getIsolationScope().clear(); @@ -20,7 +20,7 @@ describe('errorHandler()', () => { const path = '/by/the/trees/'; const queryString = 'chase=me&please=thankyou'; - const sentryErrorMiddleware = errorHandler(); + const sentryErrorMiddleware = expressErrorHandler(); let req: http.IncomingMessage, res: http.ServerResponse, next: () => undefined; let client: NodeClient;