Skip to content

ref(node-experimental): Rename errorHandler to expressErrorHandler #10746

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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);
4 changes: 1 addition & 3 deletions packages/node-experimental/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
export { errorHandler } from './sdk/handlers/errorHandler';

export { httpIntegration } from './integrations/http';
export { nativeNodeFetchIntegration } from './integrations/node-fetch';

Expand All @@ -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';
Expand Down
87 changes: 87 additions & 0 deletions packages/node-experimental/src/integrations/tracing/express.ts
Original file line number Diff line number Diff line change
@@ -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 = (() => {
Expand All @@ -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<NodeClient>();
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;
}
76 changes: 0 additions & 76 deletions packages/node-experimental/src/sdk/handlers/errorHandler.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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;
Expand Down