Skip to content

Commit

Permalink
Merge branch 'develop' into nh/improve-nestjs-error-handling
Browse files Browse the repository at this point in the history
  • Loading branch information
nicohrubec authored Jul 11, 2024
2 parents 2795344 + 9bb9c99 commit 72e4f8e
Show file tree
Hide file tree
Showing 16 changed files with 232 additions and 88 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ import { Module } from '@nestjs/common';
import { ScheduleModule } from '@nestjs/schedule';
import { AppController1, AppController2 } from './app.controller';
import { AppService1, AppService2 } from './app.service';
import { TestModule } from './test-module/test.module';

@Module({
imports: [ScheduleModule.forRoot()],
imports: [ScheduleModule.forRoot(), TestModule],
controllers: [AppController1],
providers: [AppService1],
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { Controller, Get } from '@nestjs/common';
import { TestException } from './test.exception';

@Controller('test-module')
export class TestController {
constructor() {}

@Get()
getTest(): string {
throw new TestException();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export class TestException extends Error {
constructor() {
super('Something went wrong in the test 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 { TestException } from './test.exception';

@Catch(TestException)
export class TestExceptionFilter extends BaseExceptionFilter {
catch(exception: unknown, host: ArgumentsHost) {
if (exception instanceof TestException) {
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 { TestController } from './test.controller';
import { TestExceptionFilter } from './test.filter';

@Module({
imports: [],
controllers: [TestController],
providers: [
{
provide: APP_FILTER,
useClass: TestExceptionFilter,
},
],
})
export class TestModule {}
Original file line number Diff line number Diff line change
Expand Up @@ -53,3 +53,32 @@ 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

// should never arrive, but does because the exception is not handled properly
const errorEvent = await errorEventPromise;

expect(errorEvent.exception?.values).toHaveLength(1);
expect(errorEvent.exception?.values?.[0]?.value).toBe('Something went wrong in the test module!');

expect(errorEvent.request).toEqual({
method: 'GET',
cookies: {},
headers: expect.any(Object),
url: 'http://localhost:3030/test-module',
});

expect(errorEvent.transaction).toEqual('GET /test-module');

expect(errorEvent.contexts?.trace).toEqual({
trace_id: expect.any(String),
span_id: expect.any(String),
});
});
2 changes: 0 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"clean:deps": "lerna clean --yes && rm -rf node_modules && yarn",
"clean:tarballs": "rimraf **/*.tgz",
"clean:all": "run-s clean:build clean:tarballs clean:caches clean:deps",
"codecov": "codecov",
"fix": "run-s fix:biome fix:prettier fix:lerna",
"fix:lerna": "lerna run fix",
"fix:biome": "biome check --apply .",
Expand Down Expand Up @@ -109,7 +108,6 @@
"@types/jsdom": "^21.1.6",
"@types/node": "^14.18.0",
"@vitest/coverage-v8": "^1.6.0",
"codecov": "^3.6.5",
"deepmerge": "^4.2.2",
"downlevel-dts": "~0.11.0",
"eslint": "7.32.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/nestjs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ Note that it is necessary to initialize Sentry **before you import any package t
## Span Decorator

Use the @SentryTraced() decorator to gain additional performance insights for any function within your NestJS
application.
applications.

```js
import { Injectable } from '@nestjs/common';
Expand Down
4 changes: 3 additions & 1 deletion packages/nextjs/src/common/wrapRouteHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@ import {
import { vercelWaitUntil } from './utils/vercelWaitUntil';

/**
* Wraps a Next.js route handler with performance and error instrumentation.
* Wraps a Next.js App Router Route handler with Sentry error and performance instrumentation.
*
* NOTICE: This wrapper is for App Router API routes. If you are looking to wrap Pages Router API routes use `wrapApiHandlerWithSentry` instead.
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function wrapRouteHandlerWithSentry<F extends (...args: any[]) => any>(
Expand Down
3 changes: 2 additions & 1 deletion packages/nextjs/src/config/loaders/wrappingLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,8 @@ export default function wrappingLoader(

const componentTypeMatch = path.posix
.normalize(path.relative(appDir, this.resourcePath))
.match(/\/?([^/]+)\.(?:js|ts|jsx|tsx)$/);
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
.match(new RegExp(`/\\/?([^/]+)\\.(?:${pageExtensionRegex})$`));

if (componentTypeMatch && componentTypeMatch[1]) {
let componentType: ServerComponentContext['componentType'];
Expand Down
22 changes: 15 additions & 7 deletions packages/nextjs/src/config/webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ export function constructWebpackConfigFunction(
);
};

const possibleMiddlewareLocations = ['js', 'jsx', 'ts', 'tsx'].map(middlewareFileEnding => {
const possibleMiddlewareLocations = pageExtensions.map(middlewareFileEnding => {
return path.join(middlewareLocationFolder, `middleware.${middlewareFileEnding}`);
});
const isMiddlewareResource = (resourcePath: string): boolean => {
Expand All @@ -163,7 +163,10 @@ export function constructWebpackConfigFunction(
return (
appDirPath !== undefined &&
normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) &&
!!normalizedAbsoluteResourcePath.match(/[\\/](page|layout|loading|head|not-found)\.(js|jsx|tsx)$/)
!!normalizedAbsoluteResourcePath.match(
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
new RegExp(`[\\\\/](page|layout|loading|head|not-found)\\.(${pageExtensionRegex})$`),
)
);
};

Expand All @@ -172,7 +175,10 @@ export function constructWebpackConfigFunction(
return (
appDirPath !== undefined &&
normalizedAbsoluteResourcePath.startsWith(appDirPath + path.sep) &&
!!normalizedAbsoluteResourcePath.match(/[\\/]route\.(js|jsx|ts|tsx)$/)
!!normalizedAbsoluteResourcePath.match(
// eslint-disable-next-line @sentry-internal/sdk/no-regexp-constructor
new RegExp(`[\\\\/]route\\.(${pageExtensionRegex})$`),
)
);
};

Expand Down Expand Up @@ -285,10 +291,12 @@ export function constructWebpackConfigFunction(
}

if (appDirPath) {
const hasGlobalErrorFile = ['global-error.js', 'global-error.jsx', 'global-error.ts', 'global-error.tsx'].some(
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
globalErrorFile => fs.existsSync(path.join(appDirPath!, globalErrorFile)),
);
const hasGlobalErrorFile = pageExtensions
.map(extension => `global-error.${extension}`)
.some(
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
globalErrorFile => fs.existsSync(path.join(appDirPath!, globalErrorFile)),
);

if (
!hasGlobalErrorFile &&
Expand Down
4 changes: 3 additions & 1 deletion packages/nextjs/src/index.types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ export declare const metrics: typeof clientSdk.metrics & typeof serverSdk.metric
export { withSentryConfig } from './config';

/**
* Wraps a Next.js API handler with Sentry error and performance instrumentation.
* Wraps a Next.js Pages Router API route with Sentry error and performance instrumentation.
*
* NOTICE: This wrapper is for Pages Router API routes. If you are looking to wrap App Router API routes use `wrapRouteHandlerWithSentry` instead.
*
* @param handler The handler exported from the API route file.
* @param parameterizedRoute The page's parameterized route.
Expand Down
1 change: 1 addition & 0 deletions packages/nextjs/test/config/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export const EDGE_SDK_CONFIG_FILE = 'sentry.edge.config.js';
/** Mock next config object */
export const userNextConfig: NextConfigObject = {
publicRuntimeConfig: { location: 'dogpark', activities: ['fetch', 'chasing', 'digging'] },
pageExtensions: ['jsx', 'js', 'tsx', 'ts', 'custom.jsx', 'custom.js', 'custom.tsx', 'custom.ts'],
webpack: (incomingWebpackConfig: WebpackConfigObject, _options: BuildContext) => ({
...incomingWebpackConfig,
mode: 'universal-sniffing',
Expand Down
30 changes: 27 additions & 3 deletions packages/nextjs/test/config/loaders.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ describe('webpack loaders', () => {
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/testPage.tsx',
expectedWrappingTargetKind: 'page',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/testPage.custom.tsx',
expectedWrappingTargetKind: 'page',
},
{
resourcePath: './src/pages/testPage.tsx',
expectedWrappingTargetKind: 'page',
Expand Down Expand Up @@ -133,6 +137,10 @@ describe('webpack loaders', () => {
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/middleware.js',
expectedWrappingTargetKind: 'middleware',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/middleware.custom.js',
expectedWrappingTargetKind: 'middleware',
},
{
resourcePath: './src/middleware.js',
expectedWrappingTargetKind: 'middleware',
Expand Down Expand Up @@ -162,25 +170,41 @@ describe('webpack loaders', () => {
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/api/nested/testApiRoute.js',
expectedWrappingTargetKind: 'api-route',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/pages/api/nested/testApiRoute.custom.js',
expectedWrappingTargetKind: 'api-route',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/route.ts',
expectedWrappingTargetKind: 'route-handler',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/route.custom.ts',
expectedWrappingTargetKind: 'route-handler',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/page.js',
expectedWrappingTargetKind: 'server-component',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/page.custom.js',
expectedWrappingTargetKind: 'server-component',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/page.js',
expectedWrappingTargetKind: 'server-component',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/page.ts', // ts is not a valid file ending for pages in the app dir
expectedWrappingTargetKind: undefined,
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/nested/page.ts',
expectedWrappingTargetKind: 'server-component',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/(group)/nested/page.tsx',
expectedWrappingTargetKind: 'server-component',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/(group)/nested/loading.ts',
expectedWrappingTargetKind: undefined,
expectedWrappingTargetKind: 'server-component',
},
{
resourcePath: '/Users/Maisey/projects/squirrelChasingSimulator/src/app/layout.js',
Expand Down
50 changes: 45 additions & 5 deletions packages/nuxt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,18 +28,30 @@ The minimum supported version of Nuxt is `3.0.0`.
This package is a wrapper around `@sentry/node` for the server and `@sentry/vue` for the client side, with added
functionality related to Nuxt.

What is working:
**What is working:**

- Error Reporting
- Vue
- Node
- Nitro

What is partly working:
**What is partly working:**

- Tracing by setting `tracesSampleRate`
- UI (Vue) traces
- HTTP (Node) traces

What is not yet(!) included:
**What is not yet(!) included:**

- Source Maps
- Connected Traces
- Nuxt-specific traces and connecting frontend & backend traces

**Known Issues:**

- When adding `sentry.server.config.(ts/js)`, you get this error: "Failed to register ESM hook", but the application
will still work
- When initializing Sentry on the server with `instrument.server.(js|ts)`, you get an `'import-in-the-middle'` error,
and the application won't work

## Automatic Setup

Expand Down Expand Up @@ -96,10 +108,38 @@ Add a `sentry.server.config.(js|ts)` file to the root of your project:
import * as Sentry from '@sentry/nuxt';

Sentry.init({
dsn: env.DSN,
dsn: process.env.DSN,
});
```

**Alternative Setup (ESM-compatible)**

This setup makes sure Sentry is imported on the server before any other imports. As of now, this however leads to an
import-in-the-middle error ([related reproduction](https://github.com/getsentry/sentry-javascript-examples/pull/38)).

Add an `instrument.server.mjs` file to your `public` folder:

```javascript
import * as Sentry from '@sentry/nuxt';

// Only run `init` when DSN is available
if (process.env.SENTRY_DSN) {
Sentry.init({
dsn: process.env.DSN,
});
}
```

Add an import flag to the node options, so the file loads before any other imports:

```json
{
"scripts": {
"preview": "NODE_OPTIONS='--import ./public/instrument.server.mjs' nuxt preview"
}
}
```

### 5. Vite Setup

todo: add vite setup
Expand Down
Loading

0 comments on commit 72e4f8e

Please sign in to comment.