From 6c00aa92a48ad6971f499f88b75f346ea71b5692 Mon Sep 17 00:00:00 2001 From: Weiko Date: Tue, 9 Jan 2024 17:46:16 +0100 Subject: [PATCH] Fix capture exception for metadata and core (#3335) --- packages/twenty-server/src/app.module.ts | 11 ---- .../src/filters/global-exception.filter.ts | 17 ------ .../utils/global-exception-handler.util.ts | 57 ++++++++++--------- .../src/graphql-config.service.ts | 27 ++++++++- .../field-metadata/field-metadata.resolver.ts | 12 ++-- .../field-metadata/field-metadata.service.ts | 2 +- .../src/metadata/metadata.module-factory.ts | 32 +++++++++++ .../src/metadata/metadata.module.ts | 12 ++-- .../workspace-query-runner.service.ts | 16 +++--- 9 files changed, 107 insertions(+), 79 deletions(-) delete mode 100644 packages/twenty-server/src/filters/global-exception.filter.ts create mode 100644 packages/twenty-server/src/metadata/metadata.module-factory.ts diff --git a/packages/twenty-server/src/app.module.ts b/packages/twenty-server/src/app.module.ts index a447c1e5f..42bc8505f 100644 --- a/packages/twenty-server/src/app.module.ts +++ b/packages/twenty-server/src/app.module.ts @@ -1,14 +1,10 @@ import { Module } from '@nestjs/common'; import { GraphQLModule } from '@nestjs/graphql'; import { ConfigModule } from '@nestjs/config'; -import { APP_FILTER } from '@nestjs/core'; import { YogaDriver, YogaDriverConfig } from '@graphql-yoga/nestjs'; import { GraphQLConfigService } from 'src/graphql-config.service'; -import { GlobalExceptionFilter } from 'src/filters/global-exception.filter'; - -import { AppService } from './app.service'; import { CoreModule } from './core/core.module'; import { IntegrationsModule } from './integrations/integrations.module'; @@ -30,12 +26,5 @@ import { WorkspaceModule } from './workspace/workspace.module'; CoreModule, WorkspaceModule, ], - providers: [ - AppService, - { - provide: APP_FILTER, - useValue: GlobalExceptionFilter, - }, - ], }) export class AppModule {} diff --git a/packages/twenty-server/src/filters/global-exception.filter.ts b/packages/twenty-server/src/filters/global-exception.filter.ts deleted file mode 100644 index 6e248bc46..000000000 --- a/packages/twenty-server/src/filters/global-exception.filter.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { Catch, Injectable } from '@nestjs/common'; -import { GqlExceptionFilter } from '@nestjs/graphql'; - -import { globalExceptionHandler } from 'src/filters/utils/global-exception-handler.util'; -import { ExceptionHandlerService } from 'src/integrations/exception-handler/exception-handler.service'; - -@Catch() -@Injectable() -export class GlobalExceptionFilter implements GqlExceptionFilter { - constructor( - private readonly exceptionHandlerService: ExceptionHandlerService, - ) {} - - catch(exception: unknown) { - return globalExceptionHandler(exception, this.exceptionHandlerService); - } -} diff --git a/packages/twenty-server/src/filters/utils/global-exception-handler.util.ts b/packages/twenty-server/src/filters/utils/global-exception-handler.util.ts index 736f6c70d..d5f733b5e 100644 --- a/packages/twenty-server/src/filters/utils/global-exception-handler.util.ts +++ b/packages/twenty-server/src/filters/utils/global-exception-handler.util.ts @@ -6,42 +6,50 @@ import { AuthenticationError, BaseGraphQLError, ForbiddenError, + ValidationError, } from 'src/filters/utils/graphql-errors.util'; import { ExceptionHandlerService } from 'src/integrations/exception-handler/exception-handler.service'; const graphQLPredefinedExceptions = { + 400: ValidationError, 401: AuthenticationError, 403: ForbiddenError, }; -export const globalExceptionHandler = ( - exception: unknown, +export const handleExceptionAndConvertToGraphQLError = ( + exception: Error, exceptionHandlerService: ExceptionHandlerService, -) => { - if (exception instanceof HttpException) { - return httpExceptionHandler(exception, exceptionHandlerService); - } +): BaseGraphQLError => { + handleException(exception, exceptionHandlerService); - if (exception instanceof TypeORMError) { - return typeOrmExceptionHandler(exception, exceptionHandlerService); - } - - exceptionHandlerService.captureException(exception); - - return exception; + return convertExceptionToGraphQLError(exception); }; -export const httpExceptionHandler = ( - exception: HttpException, +export const handleException = ( + exception: Error, exceptionHandlerService: ExceptionHandlerService, -) => { - const status = exception.getStatus(); - let error: BaseGraphQLError; - - // Capture all 5xx errors and send them to exception handler - if (status >= 500) { +): void => { + if ( + exception instanceof TypeORMError || + (exception instanceof HttpException && exception.getStatus() >= 500) + ) { exceptionHandlerService.captureException(exception); } +}; + +export const convertExceptionToGraphQLError = ( + exception: Error, +): BaseGraphQLError => { + if (exception instanceof HttpException) { + return convertHttpExceptionToGraphql(exception); + } + + return convertExceptionToGraphql(exception); +}; + +export const convertHttpExceptionToGraphql = (exception: HttpException) => { + const status = exception.getStatus(); + let error: BaseGraphQLError; if (status in graphQLPredefinedExceptions) { error = new graphQLPredefinedExceptions[exception.getStatus()]( @@ -60,12 +68,7 @@ export const httpExceptionHandler = ( return error; }; -export const typeOrmExceptionHandler = ( - exception: TypeORMError, - exceptionHandlerService: ExceptionHandlerService, -) => { - exceptionHandlerService.captureException(exception); - +export const convertExceptionToGraphql = (exception: Error) => { const error = new BaseGraphQLError(exception.name, 'INTERNAL_SERVER_ERROR'); error.stack = exception.stack; diff --git a/packages/twenty-server/src/graphql-config.service.ts b/packages/twenty-server/src/graphql-config.service.ts index e122ce01c..40b36198f 100644 --- a/packages/twenty-server/src/graphql-config.service.ts +++ b/packages/twenty-server/src/graphql-config.service.ts @@ -9,14 +9,18 @@ import { import { GraphQLSchema, GraphQLError } from 'graphql'; import GraphQLJSON from 'graphql-type-json'; import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken'; -import { GraphQLSchemaWithContext, YogaInitialContext } from 'graphql-yoga'; +import { + GraphQLSchemaWithContext, + YogaInitialContext, + maskError, +} from 'graphql-yoga'; import { TokenService } from 'src/core/auth/services/token.service'; import { CoreModule } from 'src/core/core.module'; import { Workspace } from 'src/core/workspace/workspace.entity'; import { WorkspaceFactory } from 'src/workspace/workspace.factory'; import { ExceptionHandlerService } from 'src/integrations/exception-handler/exception-handler.service'; -import { globalExceptionHandler } from 'src/filters/utils/global-exception-handler.util'; +import { handleExceptionAndConvertToGraphQLError } from 'src/filters/utils/global-exception-handler.util'; @Injectable() export class GraphQLConfigService @@ -29,10 +33,24 @@ export class GraphQLConfigService ) {} createGqlOptions(): YogaDriverConfig { + const exceptionHandlerService = this.exceptionHandlerService; + return { context: ({ req }) => ({ req }), autoSchemaFile: true, include: [CoreModule], + maskedErrors: { + maskError(error: GraphQLError, message, isDev) { + if (error.originalError) { + return handleExceptionAndConvertToGraphQLError( + error.originalError, + exceptionHandlerService, + ); + } + + return maskError(error, message, isDev); + }, + }, conditionalSchema: async (context) => { try { let workspace: Workspace; @@ -63,7 +81,10 @@ export class GraphQLConfigService }); } - throw globalExceptionHandler(error, this.exceptionHandlerService); + throw handleExceptionAndConvertToGraphQLError( + error, + this.exceptionHandlerService, + ); } }, resolvers: { JSON: GraphQLJSON }, diff --git a/packages/twenty-server/src/metadata/field-metadata/field-metadata.resolver.ts b/packages/twenty-server/src/metadata/field-metadata/field-metadata.resolver.ts index 3b712968e..a8b8f5342 100644 --- a/packages/twenty-server/src/metadata/field-metadata/field-metadata.resolver.ts +++ b/packages/twenty-server/src/metadata/field-metadata/field-metadata.resolver.ts @@ -19,10 +19,14 @@ export class FieldMetadataResolver { @Args('input') input: CreateOneFieldMetadataInput, @AuthWorkspace() { id: workspaceId }: Workspace, ) { - return this.fieldMetadataService.createOne({ - ...input.field, - workspaceId, - }); + try { + return this.fieldMetadataService.createOne({ + ...input.field, + workspaceId, + }); + } catch (error) { + console.log(error); + } } @Mutation(() => FieldMetadataDTO) diff --git a/packages/twenty-server/src/metadata/field-metadata/field-metadata.service.ts b/packages/twenty-server/src/metadata/field-metadata/field-metadata.service.ts index ecf89cdb0..659c751f5 100644 --- a/packages/twenty-server/src/metadata/field-metadata/field-metadata.service.ts +++ b/packages/twenty-server/src/metadata/field-metadata/field-metadata.service.ts @@ -56,7 +56,7 @@ export class FieldMetadataService extends TypeOrmQueryService => ({ + context: ({ req }) => ({ req }), + driver: YogaDriver, + autoSchemaFile: true, + include: [MetadataModule], + resolvers: { JSON: GraphQLJSON }, + plugins: [], + path: '/metadata', + maskedErrors: { + maskError(error: GraphQLError, message, isDev) { + if (error.originalError) { + return handleExceptionAndConvertToGraphQLError( + error.originalError, + exceptionHandlerService, + ); + } else { + return maskError(error, message, isDev); + } + }, + }, +}); diff --git a/packages/twenty-server/src/metadata/metadata.module.ts b/packages/twenty-server/src/metadata/metadata.module.ts index 3e5c43bef..8f7bd58f1 100644 --- a/packages/twenty-server/src/metadata/metadata.module.ts +++ b/packages/twenty-server/src/metadata/metadata.module.ts @@ -2,10 +2,10 @@ import { Module } from '@nestjs/common'; import { GraphQLModule } from '@nestjs/graphql'; import { YogaDriverConfig, YogaDriver } from '@graphql-yoga/nestjs'; -import GraphQLJSON from 'graphql-type-json'; import { WorkspaceMigrationRunnerModule } from 'src/workspace/workspace-migration-runner/workspace-migration-runner.module'; import { WorkspaceMigrationModule } from 'src/metadata/workspace-migration/workspace-migration.module'; +import { metadataModuleFactory } from 'src/metadata/metadata.module-factory'; import { DataSourceModule } from './data-source/data-source.module'; import { FieldMetadataModule } from './field-metadata/field-metadata.module'; @@ -13,14 +13,10 @@ import { ObjectMetadataModule } from './object-metadata/object-metadata.module'; import { RelationMetadataModule } from './relation-metadata/relation-metadata.module'; @Module({ imports: [ - GraphQLModule.forRoot({ - context: ({ req }) => ({ req }), + GraphQLModule.forRootAsync({ driver: YogaDriver, - autoSchemaFile: true, - include: [MetadataModule], - resolvers: { JSON: GraphQLJSON }, - plugins: [], - path: '/metadata', + imports: [], + useFactory: metadataModuleFactory, }), DataSourceModule, FieldMetadataModule, diff --git a/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts b/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts index 415f7a038..d3a41cbc7 100644 --- a/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts +++ b/packages/twenty-server/src/workspace/workspace-query-runner/workspace-query-runner.service.ts @@ -34,7 +34,7 @@ import { } from 'src/workspace/workspace-query-runner/jobs/call-webhook-jobs.job'; import { parseResult } from 'src/workspace/workspace-query-runner/utils/parse-result.util'; import { ExceptionHandlerService } from 'src/integrations/exception-handler/exception-handler.service'; -import { globalExceptionHandler } from 'src/filters/utils/global-exception-handler.util'; +import { handleExceptionAndConvertToGraphQLError } from 'src/filters/utils/global-exception-handler.util'; import { WorkspaceQueryRunnerOptions } from './interfaces/query-runner-optionts.interface'; import { @@ -72,7 +72,7 @@ export class WorkspaceQueryRunnerService { return this.parseResult>(result, targetTableName, ''); } catch (exception) { - const error = globalExceptionHandler( + const error = handleExceptionAndConvertToGraphQLError( exception, this.exceptionHandlerService, ); @@ -106,7 +106,7 @@ export class WorkspaceQueryRunnerService { return parsedResult?.edges?.[0]?.node; } catch (exception) { - const error = globalExceptionHandler( + const error = handleExceptionAndConvertToGraphQLError( exception, this.exceptionHandlerService, ); @@ -141,7 +141,7 @@ export class WorkspaceQueryRunnerService { return parsedResults; } catch (exception) { - const error = globalExceptionHandler( + const error = handleExceptionAndConvertToGraphQLError( exception, this.exceptionHandlerService, ); @@ -185,7 +185,7 @@ export class WorkspaceQueryRunnerService { return parsedResults?.[0]; } catch (exception) { - const error = globalExceptionHandler( + const error = handleExceptionAndConvertToGraphQLError( exception, this.exceptionHandlerService, ); @@ -220,7 +220,7 @@ export class WorkspaceQueryRunnerService { return parsedResults?.[0]; } catch (exception) { - const error = globalExceptionHandler( + const error = handleExceptionAndConvertToGraphQLError( exception, this.exceptionHandlerService, ); @@ -255,7 +255,7 @@ export class WorkspaceQueryRunnerService { return parsedResults; } catch (exception) { - const error = globalExceptionHandler( + const error = handleExceptionAndConvertToGraphQLError( exception, this.exceptionHandlerService, ); @@ -293,7 +293,7 @@ export class WorkspaceQueryRunnerService { return parsedResults; } catch (exception) { - const error = globalExceptionHandler( + const error = handleExceptionAndConvertToGraphQLError( exception, this.exceptionHandlerService, );