Browse Source

feat(sentry-plugin): Use ErrorHandlerStrategy for better error coverage

Using the new ErrorHandlerStrategy allows us to collect errors from both the server and the
worker. It also prevents the default exception filter from being overridden.
Michael Bromley 2 years ago
parent
commit
82ddf943f0

+ 42 - 0
packages/sentry-plugin/src/sentry-error-handler-strategy.ts

@@ -0,0 +1,42 @@
+import { ArgumentsHost, ExecutionContext } from '@nestjs/common';
+import { GqlContextType, GqlExecutionContext } from '@nestjs/graphql';
+import { setContext } from '@sentry/node';
+import { ErrorHandlerStrategy, I18nError, Injector, Job, LogLevel } from '@vendure/core';
+
+import { SentryService } from './sentry.service';
+
+export class SentryErrorHandlerStrategy implements ErrorHandlerStrategy {
+    private sentryService: SentryService;
+
+    init(injector: Injector) {
+        this.sentryService = injector.get(SentryService);
+    }
+
+    handleServerError(exception: Error, { host }: { host: ArgumentsHost }) {
+        // We only care about errors which have at least a Warn log level
+        const shouldLogError = exception instanceof I18nError ? exception.logLevel <= LogLevel.Warn : true;
+        if (shouldLogError) {
+            if (host?.getType<GqlContextType>() === 'graphql') {
+                const gqlContext = GqlExecutionContext.create(host as ExecutionContext);
+                const info = gqlContext.getInfo();
+                setContext('GraphQL Error Context', {
+                    fieldName: info.fieldName,
+                    path: info.path,
+                });
+            }
+            const variables = (exception as any).variables;
+            if (variables) {
+                setContext('GraphQL Error Variables', variables);
+            }
+            this.sentryService.captureException(exception);
+        }
+    }
+
+    handleWorkerError(exception: Error, { job }: { job: Job }) {
+        setContext('Worker Context', {
+            queueName: job.queueName,
+            jobId: job.id,
+        });
+        this.sentryService.captureException(exception);
+    }
+}

+ 4 - 12
packages/sentry-plugin/src/sentry-plugin.ts

@@ -1,5 +1,4 @@
 import { MiddlewareConsumer, NestModule } from '@nestjs/common';
-import { APP_FILTER } from '@nestjs/core';
 import { PluginCommonModule, VendurePlugin } from '@vendure/core';
 
 import { SentryAdminTestResolver } from './api/admin-test.resolver';
@@ -8,7 +7,7 @@ import { ErrorTestService } from './api/error-test.service';
 import { SENTRY_PLUGIN_OPTIONS } from './constants';
 import { SentryApolloPlugin } from './sentry-apollo-plugin';
 import { SentryContextMiddleware } from './sentry-context.middleware';
-import { SentryExceptionsFilter } from './sentry.filter';
+import { SentryErrorHandlerStrategy } from './sentry-error-handler-strategy';
 import { SentryService } from './sentry.service';
 import { SentryPluginOptions } from './types';
 
@@ -108,21 +107,14 @@ const SentryOptionsProvider = {
  */
 @VendurePlugin({
     imports: [PluginCommonModule],
-    providers: [
-        SentryOptionsProvider,
-        SentryService,
-        ErrorTestService,
-        {
-            provide: APP_FILTER,
-            useClass: SentryExceptionsFilter,
-        },
-    ],
+    providers: [SentryOptionsProvider, SentryService, ErrorTestService],
     configuration: config => {
         config.apiOptions.apolloServerPlugins.push(
             new SentryApolloPlugin({
                 enableTracing: !!SentryPlugin.options.enableTracing,
             }),
         );
+        config.systemOptions.errorHandlers.push(new SentryErrorHandlerStrategy());
         return config;
     },
     adminApiExtensions: {
@@ -130,7 +122,7 @@ const SentryOptionsProvider = {
         resolvers: () => (SentryPlugin.options.includeErrorTestMutation ? [SentryAdminTestResolver] : []),
     },
     exports: [SentryService],
-    compatibility: '^2.0.0',
+    compatibility: '^2.2.0',
 })
 export class SentryPlugin implements NestModule {
     static options: SentryPluginOptions = {} as any;

+ 22 - 16
packages/sentry-plugin/src/sentry.filter.ts

@@ -2,26 +2,32 @@ import type { ArgumentsHost, ExceptionFilter } from '@nestjs/common';
 import { Catch, ExecutionContext } from '@nestjs/common';
 import { GqlContextType, GqlExecutionContext } from '@nestjs/graphql';
 import { setContext } from '@sentry/node';
+import { ExceptionLoggerFilter, ForbiddenError, I18nError, LogLevel } from '@vendure/core';
 
 import { SentryService } from './sentry.service';
 
-@Catch()
-export class SentryExceptionsFilter implements ExceptionFilter {
-    constructor(private readonly sentryService: SentryService) {}
+export class SentryExceptionsFilter extends ExceptionLoggerFilter {
+    constructor(private readonly sentryService: SentryService) {
+        super();
+    }
 
-    catch(exception: Error, host: ArgumentsHost): void {
-        if (host.getType<GqlContextType>() === 'graphql') {
-            const gqlContext = GqlExecutionContext.create(host as ExecutionContext);
-            const info = gqlContext.getInfo();
-            setContext('GraphQL Error Context', {
-                fieldName: info.fieldName,
-                path: info.path,
-            });
-        }
-        const variables = (exception as any).variables;
-        if (variables) {
-            setContext('GraphQL Error Variables', variables);
+    catch(exception: Error, host: ArgumentsHost) {
+        const shouldLogError = exception instanceof I18nError ? exception.logLevel <= LogLevel.Warn : true;
+        if (shouldLogError) {
+            if (host.getType<GqlContextType>() === 'graphql') {
+                const gqlContext = GqlExecutionContext.create(host as ExecutionContext);
+                const info = gqlContext.getInfo();
+                setContext('GraphQL Error Context', {
+                    fieldName: info.fieldName,
+                    path: info.path,
+                });
+            }
+            const variables = (exception as any).variables;
+            if (variables) {
+                setContext('GraphQL Error Variables', variables);
+            }
+            this.sentryService.captureException(exception);
         }
-        this.sentryService.captureException(exception);
+        return super.catch(exception, host);
     }
 }