Skip to content

Commit

Permalink
feat(nestjs): Change nest sdk setup (#12920)
Browse files Browse the repository at this point in the history
- Adds a new nest root module that can be used to setup the Nest SDK as
a replacement for the existing setup (with a function). Instead of
calling `setupNestErrorHandler` in the main.ts file, users can now add
`SentryModule.forRoot()` (feedback about the name is definitely welcome)
as an import in their main app module. This approach is much more native
to nest than what we used so far. This root module is introduced in the
setup.ts file.
- This root module is exported with a submodule export
`@sentry/nestjs/setup`, because the SDK now depends on nestjs directly
and without this the nest instrumentation does not work anymore, since
nest gets imported before Sentry.init gets called, which disables the
otel nest instrumentation.
- Judging from the e2e tests it seems that this new approach also
resolves some issues the previous implementation had, specifically [this
issue](#12351)
seems to be resolved. The e2e test that was in place, just documented
the current (wrong) behavior. So I updated the test to reflect the new
(correct) behavior.
- I updated all the test applications to use the new approach but kept a
copy of the nestjs-basic and nestjs-distributed-tracing with the old
setup (now named node-nestjs-basic and node-nestjs-distributed-tracing
respectively) so we can still verify that the old setup (which a lot of
people use) still keeps working going forward.
- Updated/New tests in this PR: 
    - Sends unexpected exception to Sentry if thrown in Submodule
- Does not send expected exception to Sentry if thrown in Submodule and
caught by a global exception filter
- Does not send expected exception to Sentry if thrown in Submodule and
caught by a local exception filter
- Sends expected exception to Sentry if thrown from submodule registered
before Sentry
- To accomodate the new tests I added several submodules in the
nestjs-with-submodules test-application. These are overall similarly but
have important distinctions:
- example-module-local-filter: Submodule with a local filter registered
using `@UseFilters` on the controller.
- example-module-global-filter: Submodule with a global filter
registered using APP_FILTER in the submodule definition.
- example-module-global-filter-wrong-registration-order: Also has a
global filter set with APP_FILTER, but is registered in the root module
as first submodule, even before the SentryIntegration is initialized.
This case does not work properly in the new setup (Sentry should be set
first), so this module is used for tests documenting this behavior.
- Also set "moduleResolution": "Node16" in the nestjs-basic sample app
to ensure our submodule-export workaround works in both, default and
sub-path-export-compatible TS configs as was suggested
[here](#12948 (comment)).
  • Loading branch information
nicohrubec authored Jul 23, 2024
1 parent f867cc0 commit b577079
Show file tree
Hide file tree
Showing 67 changed files with 1,953 additions and 59 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1010,6 +1010,8 @@ jobs:
'generic-ts3.8',
'node-fastify',
'node-hapi',
'node-nestjs-basic',
'node-nestjs-distributed-tracing',
'nestjs-basic',
'nestjs-distributed-tracing',
'nestjs-with-submodules',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { Module } from '@nestjs/common';
import { ScheduleModule } from '@nestjs/schedule';
import { SentryModule } from '@sentry/nestjs/setup';
import { AppController } from './app.controller';
import { AppService } from './app.service';

@Module({
imports: [ScheduleModule.forRoot()],
imports: [SentryModule.forRoot(), ScheduleModule.forRoot()],
controllers: [AppController],
providers: [AppService],
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,13 @@
import './instrument';

// Import other modules
import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
import * as Sentry from '@sentry/nestjs';
import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';

const PORT = 3030;

async function bootstrap() {
const app = await NestFactory.create(AppModule);

const { httpAdapter } = app.get(HttpAdapterHost);
Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter));

await app.listen(PORT);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
"noImplicitAny": false,
"strictBindCallApply": false,
"forceConsistentCasingInFileNames": false,
"noFallthroughCasesInSwitch": false
"noFallthroughCasesInSwitch": false,
"moduleResolution": "Node16"
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@
import './instrument';

// Import other modules
import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
import * as Sentry from '@sentry/nestjs';
import { NestFactory } from '@nestjs/core';
import { TraceInitiatorModule } from './trace-initiator.module';
import { TraceReceiverModule } from './trace-receiver.module';

Expand All @@ -12,10 +11,6 @@ const TRACE_RECEIVER_PORT = 3040;

async function bootstrap() {
const trace_initiator_app = await NestFactory.create(TraceInitiatorModule);

const { httpAdapter } = trace_initiator_app.get(HttpAdapterHost);
Sentry.setupNestErrorHandler(trace_initiator_app, new BaseExceptionFilter(httpAdapter));

await trace_initiator_app.listen(TRACE_INITIATOR_PORT);

const trace_receiver_app = await NestFactory.create(TraceReceiverModule);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import { Module } from '@nestjs/common';
import { SentryModule } from '@sentry/nestjs/setup';
import { TraceInitiatorController } from './trace-initiator.controller';
import { TraceInitiatorService } from './trace-initiator.service';

@Module({
imports: [],
imports: [SentryModule.forRoot()],
controllers: [TraceInitiatorController],
providers: [TraceInitiatorService],
})
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
import { Module } from '@nestjs/common';
import { SentryModule } from '@sentry/nestjs/setup';
import { AppController } from './app.controller';
import { AppService } from './app.service';
import { ExampleModule } from './example-module/example.module';
import { ExampleModuleGlobalFilterWrongRegistrationOrder } from './example-module-global-filter-wrong-registration-order/example.module';
import { ExampleModuleGlobalFilter } from './example-module-global-filter/example.module';
import { ExampleModuleLocalFilter } from './example-module-local-filter/example.module';

@Module({
imports: [ExampleModule],
imports: [
ExampleModuleGlobalFilterWrongRegistrationOrder,
SentryModule.forRoot(),
ExampleModuleGlobalFilter,
ExampleModuleLocalFilter,
],
controllers: [AppController],
providers: [AppService],
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { Controller, Get } from '@nestjs/common';
import { ExampleExceptionWrongRegistrationOrder } from './example.exception';

@Controller('example-module-wrong-order')
export class ExampleController {
constructor() {}

@Get('/expected-exception')
getCaughtException(): string {
throw new ExampleExceptionWrongRegistrationOrder();
}

@Get('/unexpected-exception')
getUncaughtException(): string {
throw new Error(`This is an uncaught exception!`);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export class ExampleExceptionWrongRegistrationOrder extends Error {
constructor() {
super('Something went wrong in the example module!');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { ArgumentsHost, BadRequestException, Catch } from '@nestjs/common';
import { BaseExceptionFilter } from '@nestjs/core';
import { ExampleExceptionWrongRegistrationOrder } from './example.exception';

@Catch(ExampleExceptionWrongRegistrationOrder)
export class ExampleExceptionFilterWrongRegistrationOrder extends BaseExceptionFilter {
catch(exception: unknown, host: ArgumentsHost) {
if (exception instanceof ExampleExceptionWrongRegistrationOrder) {
return super.catch(new BadRequestException(exception.message), host);
}
return super.catch(exception, host);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { Module } from '@nestjs/common';
import { APP_FILTER } from '@nestjs/core';
import { ExampleController } from './example.controller';
import { ExampleExceptionFilterWrongRegistrationOrder } from './example.filter';

@Module({
imports: [],
controllers: [ExampleController],
providers: [
{
provide: APP_FILTER,
useClass: ExampleExceptionFilterWrongRegistrationOrder,
},
],
})
export class ExampleModuleGlobalFilterWrongRegistrationOrder {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { Controller, Get } from '@nestjs/common';
import * as Sentry from '@sentry/nestjs';
import { ExampleException } from './example.exception';

@Controller('example-module')
export class ExampleController {
constructor() {}

@Get('/expected-exception')
getCaughtException(): string {
throw new ExampleException();
}

@Get('/unexpected-exception')
getUncaughtException(): string {
throw new Error(`This is an uncaught exception!`);
}

@Get('/transaction')
testTransaction() {
Sentry.startSpan({ name: 'test-span' }, () => {
Sentry.startSpan({ name: 'child-span' }, () => {});
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,4 @@ import { ExampleExceptionFilter } from './example.filter';
},
],
})
export class ExampleModule {}
export class ExampleModuleGlobalFilter {}
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { Controller, Get, UseFilters } from '@nestjs/common';
import { LocalExampleException } from './example.exception';
import { LocalExampleExceptionFilter } from './example.filter';

@Controller('example-module-local-filter')
@UseFilters(LocalExampleExceptionFilter)
export class ExampleControllerLocalFilter {
constructor() {}

@Get('/expected-exception')
getCaughtException() {
throw new LocalExampleException();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export class LocalExampleException extends Error {
constructor() {
super('Something went wrong in the example module with local filter!');
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { ArgumentsHost, BadRequestException, Catch } from '@nestjs/common';
import { BaseExceptionFilter } from '@nestjs/core';
import { LocalExampleException } from './example.exception';

@Catch(LocalExampleException)
export class LocalExampleExceptionFilter extends BaseExceptionFilter {
catch(exception: unknown, host: ArgumentsHost) {
if (exception instanceof LocalExampleException) {
return super.catch(new BadRequestException(exception.message), host);
}
return super.catch(exception, host);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { Module } from '@nestjs/common';
import { ExampleControllerLocalFilter } from './example.controller';

@Module({
imports: [],
controllers: [ExampleControllerLocalFilter],
providers: [],
})
export class ExampleModuleLocalFilter {}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,13 @@
import './instrument';

// Import other modules
import { BaseExceptionFilter, HttpAdapterHost, NestFactory } from '@nestjs/core';
import * as Sentry from '@sentry/nestjs';
import { NestFactory } from '@nestjs/core';
import { AppModule } from './app.module';

const PORT = 3030;

async function bootstrap() {
const app = await NestFactory.create(AppModule);

const { httpAdapter } = app.get(HttpAdapterHost);
Sentry.setupNestErrorHandler(app, new BaseExceptionFilter(httpAdapter));

await app.listen(PORT);
}

Expand Down
Loading

0 comments on commit b577079

Please sign in to comment.