Skip to content
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

WIP POC: Improving Nest Error Handler #12832

Closed
wants to merge 22 commits into from
Closed
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 @@ -54,31 +54,29 @@ test('Does not send expected exception to Sentry', async ({ baseURL }) => {
expect(errorEventOccurred).toBe(false);
});

test('Does not handle expected exception if exception is thrown in module', async ({ baseURL }) => {
const errorEventPromise = waitForError('nestjs', event => {
return !event.type && event.exception?.values?.[0]?.value === 'Something went wrong in the test module!';
});

const response = await fetch(`${baseURL}/test-module`);
expect(response.status).toBe(500); // should be 400
test('Handles exception correctly and does not send to Sentry if exception is thrown in module', async ({ baseURL }) => {
let errorEventOccurred = false;

// should never arrive, but does because the exception is not handled properly
const errorEvent = await errorEventPromise;
waitForError('nestjs', event => {
if (!event.type && event.exception?.values?.[0]?.value === 'Something went wrong in the test module!') {
errorEventOccurred = true;
}

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('Something went wrong in the test module!');
return event?.transaction === 'GET /test-module';
});

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
headers: expect.any(Object),
url: 'http://localhost:3030/test-module',
const transactionEventPromise = waitForTransaction('nestjs', transactionEvent => {
return transactionEvent?.transaction === 'GET /test-module';
});

expect(errorEvent.transaction).toEqual('GET /test-module');
console.log("fetch");
const response = await fetch(`${baseURL}/test-module`);
expect(response.status).toBe(400);
console.log("waiting for response");

expect(errorEvent.contexts?.trace).toEqual({
trace_id: expect.any(String),
span_id: expect.any(String),
});
await transactionEventPromise;

await new Promise(resolve => setTimeout(resolve, 10000));

expect(errorEventOccurred).toBe(false);
});
13 changes: 12 additions & 1 deletion packages/nestjs/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,18 @@
},
"dependencies": {
"@sentry/core": "8.17.0",
"@sentry/node": "8.17.0"
"@sentry/node": "8.17.0",
"@sentry/types": "8.17.0",
"@sentry/utils": "8.17.0",
"@opentelemetry/instrumentation-nestjs-core": "0.39.0"
},
"devDependencies": {
"@nestjs/core": "^10.3.10",
"@nestjs/common": "^10.3.10"
},
"peerDependencies": {
"@nestjs/core": "^10.3.10",
"@nestjs/common": "^10.3.10"
},
"scripts": {
"build": "run-p build:transpile build:types",
Expand Down
1 change: 1 addition & 0 deletions packages/nestjs/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export * from '@sentry/node';

export { init } from './sdk';
export { nestIntegration, setupNestErrorHandler } from './setup';

export { SentryTraced } from './span-decorator';
export { SentryCron } from './cron-decorator';
182 changes: 182 additions & 0 deletions packages/nestjs/src/setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
import type { ArgumentsHost } from '@nestjs/common';
import { DiscoveryService } from '@nestjs/core';
import { NestInstrumentation } from '@opentelemetry/instrumentation-nestjs-core';
import {
SEMANTIC_ATTRIBUTE_SENTRY_OP,
SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN,
captureException,
defineIntegration,
getClient,
getDefaultIsolationScope,
getIsolationScope,
spanToJSON,
} from '@sentry/core';
import { generateInstrumentOnce } from '@sentry/node';
import type { IntegrationFn, Span } from '@sentry/types';
import { logger } from '@sentry/utils';

interface MinimalNestJsExecutionContext {
getType: () => string;

switchToHttp: () => {
// minimal request object
// according to official types, all properties are required but
// let's play it safe and assume they're optional
getRequest: () => {
route?: {
path?: string;
};
method?: string;
};
};
}

interface NestJsErrorFilter {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
catch(exception: any, host: any): void;
}

interface MinimalNestJsApp {
useGlobalFilters: (arg0: NestJsErrorFilter) => void;
useGlobalInterceptors: (interceptor: {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
intercept: (context: MinimalNestJsExecutionContext, next: { handle: () => any }) => any;
}) => void;

get: <T>(type: new (...args: any[]) => T) => T;
}

interface ExceptionFilterInstance {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
catch: (exception: unknown, host: ArgumentsHost) => any;
}

interface Provider {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
metatype: any;
instance: ExceptionFilterInstance;
}

const INTEGRATION_NAME = 'Nest';
const CATCH_WATERMARK = '__catch__';

export const instrumentNest = generateInstrumentOnce(INTEGRATION_NAME, () => new NestInstrumentation());

const _nestIntegration = (() => {
return {
name: INTEGRATION_NAME,
setupOnce() {
instrumentNest();
},
};
}) satisfies IntegrationFn;

/**
* Nest framework integration
*
* Capture tracing data for nest.
*/
export const nestIntegration = defineIntegration(_nestIntegration);

/**
* Setup an error handler for Nest.
*/
export function setupNestErrorHandler(app: MinimalNestJsApp, baseFilter: NestJsErrorFilter): void {
// Sadly, NestInstrumentation has no requestHook, so we need to add the attributes here
// We register this hook in this method, because if we register it in the integration `setup`,
// it would always run even for users that are not even using Nest.js
const client = getClient();
if (client) {
client.on('spanStart', span => {
addNestSpanAttributes(span);
});
}

app.useGlobalInterceptors({
intercept(context, next) {
if (getIsolationScope() === getDefaultIsolationScope()) {
logger.warn('Isolation scope is still the default isolation scope, skipping setting transactionName.');
return next.handle();
}

if (context.getType() === 'http') {
const req = context.switchToHttp().getRequest();
if (req.route) {
// eslint-disable-next-line @sentry-internal/sdk/no-optional-chaining
getIsolationScope().setTransactionName(`${req.method?.toUpperCase() || 'GET'} ${req.route.path}`);
}
}

return next.handle();
},
});

const wrappedFilter = new Proxy(baseFilter, {
get(target, prop, receiver) {
if (prop === 'catch') {
const originalCatch = Reflect.get(target, prop, receiver);

return (exception: unknown, host: unknown) => {
const status_code = (exception as { status?: number }).status;

// don't report expected errors
if (status_code !== undefined && status_code >= 400 && status_code < 500) {
return originalCatch.apply(target, [exception, host]);
}

captureException(exception);
return originalCatch.apply(target, [exception, host]);
};
}
return Reflect.get(target, prop, receiver);
},
});

app.useGlobalFilters(wrappedFilter);

checkinExceptionFilters(app);
}

function addNestSpanAttributes(span: Span): void {
const attributes = spanToJSON(span).data || {};

// this is one of: app_creation, request_context, handler
const type = attributes['nestjs.type'];

// If this is already set, or we have no nest.js span, no need to process again...
if (attributes[SEMANTIC_ATTRIBUTE_SENTRY_OP] || !type) {
return;
}

span.setAttributes({
[SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.http.otel.nestjs',
[SEMANTIC_ATTRIBUTE_SENTRY_OP]: `${type}.nestjs`,
});
}

function checkinExceptionFilters(app: MinimalNestJsApp): void {
const discoveryService = app.get(DiscoveryService);
const providers = discoveryService.getProviders() as Provider[];
const exceptionFilters = providers.filter(
({ metatype }) => metatype && Reflect.getMetadata(CATCH_WATERMARK, metatype),
);

exceptionFilters.map(mod => {
const instance = mod.instance;
const originalCatch = instance.catch;

instance.catch = function (exception: unknown, host) {
const status_code = (exception as { status?: number }).status;

// don't report expected errors
if (status_code !== undefined && status_code >= 400 && status_code < 500) {
return originalCatch.apply(this, [exception, host]);
}

captureException(exception);
return originalCatch.apply(this, [exception, host]);
};

return mod;
});
}
26 changes: 26 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5882,6 +5882,15 @@
semver "^7.3.5"
tar "^6.1.11"

"@nestjs/common@^10.3.10":
version "10.3.10"
resolved "https://registry.yarnpkg.com/@nestjs/common/-/common-10.3.10.tgz#d8825d55a50a04e33080c9188e6a5b03235d19f2"
integrity sha512-H8k0jZtxk1IdtErGDmxFRy0PfcOAUg41Prrqpx76DQusGGJjsaovs1zjXVD1rZWaVYchfT1uczJ6L4Kio10VNg==
dependencies:
uid "2.0.2"
iterare "1.2.1"
tslib "2.6.3"

"@nestjs/common@^10.3.7":
version "10.3.7"
resolved "https://registry.yarnpkg.com/@nestjs/common/-/common-10.3.7.tgz#38ab5ff92277cf1f26f4749c264524e76962cfff"
Expand All @@ -5891,6 +5900,18 @@
iterare "1.2.1"
tslib "2.6.2"

"@nestjs/core@^10.3.10":
version "10.3.10"
resolved "https://registry.yarnpkg.com/@nestjs/core/-/core-10.3.10.tgz#508090c3ca36488a8e24a9e5939c2f37426e48f4"
integrity sha512-ZbQ4jovQyzHtCGCrzK5NdtW1SYO2fHSsgSY1+/9WdruYCUra+JDkWEXgZ4M3Hv480Dl3OXehAmY1wCOojeMyMQ==
dependencies:
uid "2.0.2"
"@nuxtjs/opencollective" "0.3.2"
fast-safe-stringify "2.1.1"
iterare "1.2.1"
path-to-regexp "3.2.0"
tslib "2.6.3"

"@nestjs/core@^10.3.3":
version "10.3.3"
resolved "https://registry.yarnpkg.com/@nestjs/core/-/core-10.3.3.tgz#f957068ddda59252b7c36fcdb07a0fb323b52bcf"
Expand Down Expand Up @@ -32677,6 +32698,11 @@ tslib@2.6.2, tslib@^2.6.2:
resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.6.2.tgz#703ac29425e7b37cd6fd456e92404d46d1f3e4ae"
integrity sha512-AEYxH93jGFPn/a2iVAwW87VuUIkR1FVUKB77NwMF7nBTDkDrrT/Hpt/IrCJ0QXhW27jTBDcf5ZY7w6RiqTMw2Q==

tslib@2.6.3:
version "2.6.3"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.6.3.tgz#0438f810ad7a9edcde7a241c3d80db693c8cbfe0"
integrity sha512-xNvxJEOUiWPGhUuUdQgAJPKOOJfGnIyKySOc09XkKsgdUV/3E2zvwZYdejjmRgPCgcym1juLH3226yA7sEFJKQ==

tslib@^1.11.1, tslib@^1.8.1, tslib@^1.9.0:
version "1.14.1"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.14.1.tgz#cf2d38bdc34a134bcaf1091c41f6619e2f672d00"
Expand Down
Loading