Fix custom errors thrown as 500 (#6238)

We call convertExceptionToGraphQLError in the exception handler for http
exceptions but we don't take into account those that already are
graphqlErrors and because of that the logic of convertExceptionToGraphql
is to fallback to a 500.
Now if the exception is a BaseGraphqlError (custom graphql error we
throw in the code), we throw them directly.

BEFORE
<img width="957" alt="Screenshot 2024-07-12 at 15 33 03"
src="https://github.com/user-attachments/assets/22ddae13-4996-4ad3-8f86-dd17c2922ca8">


AFTER
<img width="923" alt="Screenshot 2024-07-12 at 15 32 01"
src="https://github.com/user-attachments/assets/d3d6db93-6d28-495c-a4b4-ba4e47d45abd">

---------

Co-authored-by: Charles Bochet <charles@twenty.com>

packages/twenty-server/src/engine/api/graphql/metadata.module-factory.ts

@@ -1,15 +1,15 @@
 import { YogaDriverConfig } from '@graphql-yoga/nestjs';
 import GraphQLJSON from 'graphql-type-json';
 
-import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
+import { useCachedMetadata } from 'src/engine/api/graphql/graphql-config/hooks/use-cached-metadata';
-import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
-import { useExceptionHandler } from 'src/engine/integrations/exception-handler/hooks/use-exception-handler.hook';
 import { useThrottler } from 'src/engine/api/graphql/graphql-config/hooks/use-throttler';
 import { MetadataGraphQLApiModule } from 'src/engine/api/graphql/metadata-graphql-api.module';
-import { renderApolloPlayground } from 'src/engine/utils/render-apollo-playground.util';
+import { useGraphQLErrorHandlerHook } from 'src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook';
 import { DataloaderService } from 'src/engine/dataloaders/dataloader.service';
-import { useCachedMetadata } from 'src/engine/api/graphql/graphql-config/hooks/use-cached-metadata';
 import { CacheStorageService } from 'src/engine/integrations/cache-storage/cache-storage.service';
+import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
+import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
+import { renderApolloPlayground } from 'src/engine/utils/render-apollo-playground.util';
 
 export const metadataModuleFactory = async (
   environmentService: EnvironmentService,
@@ -32,7 +32,7 @@ export const metadataModuleFactory = async (
           return context.req.user?.id ?? context.req.ip ?? 'anonymous';
         },
       }),
-      useExceptionHandler({
+      useGraphQLErrorHandlerHook({
         exceptionHandlerService,
       }),
       useCachedMetadata({

packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.service.ts

@@ -9,12 +9,12 @@ import { EventEmitter2 } from '@nestjs/event-emitter';
 import isEmpty from 'lodash.isempty';
 import { DataSource } from 'typeorm';
 
-import { IConnection } from 'src/engine/api/graphql/workspace-query-runner/interfaces/connection.interface';
 import {
   Record as IRecord,
   RecordFilter,
   RecordOrderBy,
 } from 'src/engine/api/graphql/workspace-query-builder/interfaces/record.interface';
+import { IConnection } from 'src/engine/api/graphql/workspace-query-runner/interfaces/connection.interface';
 import {
   CreateManyResolverArgs,
   CreateOneResolverArgs,
@@ -30,36 +30,36 @@ import {
 import { ObjectMetadataInterface } from 'src/engine/metadata-modules/field-metadata/interfaces/object-metadata.interface';
 
 import { WorkspaceQueryBuilderFactory } from 'src/engine/api/graphql/workspace-query-builder/workspace-query-builder.factory';
-import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service';
+import { QueryResultGettersFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters.factory';
-import { MessageQueueService } from 'src/engine/integrations/message-queue/services/message-queue.service';
+import { QueryRunnerArgsFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory';
-import { MessageQueue } from 'src/engine/integrations/message-queue/message-queue.constants';
 import {
   CallWebhookJobsJob,
   CallWebhookJobsJobData,
   CallWebhookJobsJobOperation,
 } from 'src/engine/api/graphql/workspace-query-runner/jobs/call-webhook-jobs.job';
-import { parseResult } from 'src/engine/api/graphql/workspace-query-runner/utils/parse-result.util';
-import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util';
-import { ObjectRecordDeleteEvent } from 'src/engine/integrations/event-emitter/types/object-record-delete.event';
-import { ObjectRecordCreateEvent } from 'src/engine/integrations/event-emitter/types/object-record-create.event';
-import { ObjectRecordUpdateEvent } from 'src/engine/integrations/event-emitter/types/object-record-update.event';
-import { WorkspaceQueryHookService } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/workspace-query-hook.service';
-import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
-import { NotFoundError } from 'src/engine/utils/graphql-errors.util';
-import { QueryRunnerArgsFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/query-runner-args.factory';
-import { QueryResultGettersFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters.factory';
-import { assertMutationNotOnRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/assert-mutation-not-on-remote-object.util';
-import { STANDARD_OBJECT_IDS } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids';
 import { assertIsValidUuid } from 'src/engine/api/graphql/workspace-query-runner/utils/assert-is-valid-uuid.util';
-import { isQueryTimeoutError } from 'src/engine/utils/query-timeout.util';
+import { parseResult } from 'src/engine/api/graphql/workspace-query-runner/utils/parse-result.util';
-import { InjectMessageQueue } from 'src/engine/integrations/message-queue/decorators/message-queue.decorator';
+import { WorkspaceQueryHookService } from 'src/engine/api/graphql/workspace-query-runner/workspace-query-hook/workspace-query-hook.service';
 import { DuplicateService } from 'src/engine/core-modules/duplicate/duplicate.service';
+import { NotFoundError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
+import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
+import { ObjectRecordCreateEvent } from 'src/engine/integrations/event-emitter/types/object-record-create.event';
+import { ObjectRecordDeleteEvent } from 'src/engine/integrations/event-emitter/types/object-record-delete.event';
+import { ObjectRecordUpdateEvent } from 'src/engine/integrations/event-emitter/types/object-record-update.event';
+import { InjectMessageQueue } from 'src/engine/integrations/message-queue/decorators/message-queue.decorator';
+import { MessageQueue } from 'src/engine/integrations/message-queue/message-queue.constants';
+import { MessageQueueService } from 'src/engine/integrations/message-queue/services/message-queue.service';
+import { assertMutationNotOnRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/assert-mutation-not-on-remote-object.util';
+import { computeObjectTargetTable } from 'src/engine/utils/compute-object-target-table.util';
+import { isQueryTimeoutError } from 'src/engine/utils/query-timeout.util';
+import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/workspace-datasource.service';
+import { STANDARD_OBJECT_IDS } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids';
 
-import { WorkspaceQueryRunnerOptions } from './interfaces/query-runner-option.interface';
 import {
   PGGraphQLMutation,
   PGGraphQLResult,
 } from './interfaces/pg-graphql.interface';
+import { WorkspaceQueryRunnerOptions } from './interfaces/query-runner-option.interface';
 import {
   PgGraphQLConfig,
   computePgGraphQLError,

packages/twenty-server/src/engine/api/rest/errors/RestApiException.ts

@@ -1,6 +1,6 @@
 import { BadRequestException } from '@nestjs/common';
 
-import { BaseGraphQLError } from 'src/engine/utils/graphql-errors.util';
+import { BaseGraphQLError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
 
 const formatMessage = (message: BaseGraphQLError) => {
   if (message.extensions) {

packages/twenty-server/src/engine/core-modules/graphql/engine-graphql.module.ts

@@ -0,0 +1,11 @@
+import { Module } from '@nestjs/common';
+
+import { useGraphQLErrorHandlerHook } from 'src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook';
+import { ExceptionHandlerModule } from 'src/engine/integrations/exception-handler/exception-handler.module';
+
+@Module({
+  imports: [ExceptionHandlerModule],
+  exports: [useGraphQLErrorHandlerHook],
+  providers: [],
+})
+export class EngineGraphQLModule {}

packages/twenty-server/src/engine/core-modules/graphql/hooks/use-graphql-error-handler.hook.ts

@@ -0,0 +1,133 @@
+import {
+  OnExecuteDoneHookResultOnNextHook,
+  Plugin,
+  getDocumentString,
+  handleStreamOrSingleExecutionResult,
+} from '@envelop/core';
+import { GraphQLError, Kind, OperationDefinitionNode, print } from 'graphql';
+
+import { GraphQLContext } from 'src/engine/api/graphql/graphql-config/interfaces/graphql-context.interface';
+
+import { generateGraphQLErrorFromError } from 'src/engine/core-modules/graphql/utils/generate-graphql-error-from-error.util';
+import { BaseGraphQLError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
+import { shouldCaptureException } from 'src/engine/core-modules/graphql/utils/should-capture-exception.util';
+import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
+
+type GraphQLErrorHandlerHookOptions = {
+  /**
+   * The exception handler service to use.
+   */
+  exceptionHandlerService: ExceptionHandlerService;
+  /**
+   * The key of the event id in the error's extension. `null` to disable.
+   * @default exceptionEventId
+   */
+  eventIdKey?: string | null;
+};
+
+export const useGraphQLErrorHandlerHook = <
+  PluginContext extends GraphQLContext,
+>(
+  options: GraphQLErrorHandlerHookOptions,
+): Plugin<PluginContext> => {
+  const eventIdKey = options.eventIdKey === null ? null : 'exceptionEventId';
+
+  function addEventId(
+    err: GraphQLError,
+    eventId: string | undefined | null,
+  ): GraphQLError {
+    if (eventIdKey !== null && eventId) {
+      err.extensions[eventIdKey] = eventId;
+    }
+
+    return err;
+  }
+
+  return {
+    async onExecute({ args }) {
+      const exceptionHandlerService = options.exceptionHandlerService;
+      const rootOperation = args.document.definitions.find(
+        (o) => o.kind === Kind.OPERATION_DEFINITION,
+      ) as OperationDefinitionNode;
+      const operationType = rootOperation.operation;
+      const user = args.contextValue.req.user;
+      const document = getDocumentString(args.document, print);
+      const opName =
+        args.operationName ||
+        rootOperation.name?.value ||
+        'Anonymous Operation';
+
+      return {
+        onExecuteDone(payload) {
+          const handleResult: OnExecuteDoneHookResultOnNextHook<object> = ({
+            result,
+            setResult,
+          }) => {
+            if (result.errors && result.errors.length > 0) {
+              const errorsToCapture = result.errors.reduce<BaseGraphQLError[]>(
+                (acc, error) => {
+                  if (!(error instanceof BaseGraphQLError)) {
+                    error = generateGraphQLErrorFromError(error);
+                  }
+
+                  if (shouldCaptureException(error)) {
+                    acc.push(error);
+                  }
+
+                  return acc;
+                },
+                [],
+              );
+
+              if (errorsToCapture.length > 0) {
+                const eventIds = exceptionHandlerService.captureExceptions(
+                  errorsToCapture,
+                  {
+                    operation: {
+                      name: opName,
+                      type: operationType,
+                    },
+                    document,
+                    user,
+                  },
+                );
+
+                errorsToCapture.map((err, i) => addEventId(err, eventIds?.[i]));
+              }
+
+              const nonCapturedErrors = result.errors.filter(
+                (error) => !errorsToCapture.includes(error),
+              );
+
+              setResult({
+                ...result,
+                errors: [...nonCapturedErrors, ...errorsToCapture],
+              });
+            }
+          };
+
+          return handleStreamOrSingleExecutionResult(payload, handleResult);
+        },
+      };
+    },
+    onValidate: ({ context, validateFn, params: { documentAST, schema } }) => {
+      const errors = validateFn(schema, documentAST);
+
+      if (Array.isArray(errors) && errors.length > 0) {
+        const headers = context.req.headers;
+        const currentSchemaVersion = context.req.cacheVersion;
+
+        const requestSchemaVersion = headers['x-schema-version'];
+
+        if (
+          requestSchemaVersion &&
+          requestSchemaVersion !== currentSchemaVersion
+        ) {
+          throw new GraphQLError(
+            `Schema version mismatch, please refresh the page.`,
+          );
+        }
+      }
+    },
+  };
+};

packages/twenty-server/src/engine/core-modules/graphql/utils/generate-graphql-error-from-error.util.ts

@@ -0,0 +1,18 @@
+import {
+  BaseGraphQLError,
+  ErrorCode,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
+
+export const generateGraphQLErrorFromError = (error: Error) => {
+  const graphqlError = new BaseGraphQLError(
+    error.name,
+    ErrorCode.INTERNAL_SERVER_ERROR,
+  );
+
+  if (process.env.NODE_ENV === 'development') {
+    graphqlError.stack = error.stack;
+    graphqlError.extensions['response'] = error.message;
+  }
+
+  return error;
+};

packages/twenty-server/src/engine/core-modules/graphql/utils/should-capture-exception.util.ts

@@ -0,0 +1,26 @@
+import {
+  BaseGraphQLError,
+  ErrorCode,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
+
+export const graphQLErrorCodesToFilter = [
+  ErrorCode.GRAPHQL_VALIDATION_FAILED,
+  ErrorCode.UNAUTHENTICATED,
+  ErrorCode.FORBIDDEN,
+  ErrorCode.NOT_FOUND,
+  ErrorCode.METHOD_NOT_ALLOWED,
+  ErrorCode.TIMEOUT,
+  ErrorCode.CONFLICT,
+  ErrorCode.BAD_USER_INPUT,
+];
+
+export const shouldCaptureException = (exception: Error): boolean => {
+  if (
+    exception instanceof BaseGraphQLError &&
+    graphQLErrorCodesToFilter.includes(exception?.extensions?.code)
+  ) {
+    return true;
+  }
+
+  return false;
+};

packages/twenty-server/src/engine/core-modules/postgres-credentials/postgres-credentials.service.ts

@@ -1,5 +1,5 @@
-import { InjectRepository } from '@nestjs/typeorm';
 import { BadRequestException } from '@nestjs/common';
+import { InjectRepository } from '@nestjs/typeorm';
 
 import { randomBytes } from 'crypto';
 
@@ -9,10 +9,10 @@ import {
   decryptText,
   encryptText,
 } from 'src/engine/core-modules/auth/auth.util';
-import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
+import { NotFoundError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
-import { PostgresCredentials } from 'src/engine/core-modules/postgres-credentials/postgres-credentials.entity';
-import { NotFoundError } from 'src/engine/utils/graphql-errors.util';
 import { PostgresCredentialsDTO } from 'src/engine/core-modules/postgres-credentials/dtos/postgres-credentials.dto';
+import { PostgresCredentials } from 'src/engine/core-modules/postgres-credentials/postgres-credentials.entity';
+import { EnvironmentService } from 'src/engine/integrations/environment/environment.service';
 
 export class PostgresCredentialsService {
   constructor(

packages/twenty-server/src/engine/integrations/exception-handler/hooks/use-exception-handler.hook.ts

@@ -1,10 +1,10 @@
-import { GraphQLError, Kind, OperationDefinitionNode, print } from 'graphql';
 import {
-  getDocumentString,
-  handleStreamOrSingleExecutionResult,
   OnExecuteDoneHookResultOnNextHook,
   Plugin,
+  getDocumentString,
+  handleStreamOrSingleExecutionResult,
 } from '@envelop/core';
+import { GraphQLError, Kind, OperationDefinitionNode, print } from 'graphql';
 
 import { GraphQLContext } from 'src/engine/api/graphql/graphql-config/interfaces/graphql-context.interface';
 
@@ -26,6 +26,9 @@ export type ExceptionHandlerPluginOptions = {
   eventIdKey?: string | null;
 };
 
+// This hook is deprecated.
+// We should either handle exception in the context of graphql, controller or command
+// @deprecated
 export const useExceptionHandler = <PluginContext extends GraphQLContext>(
   options: ExceptionHandlerPluginOptions,
 ): Plugin<PluginContext> => {

packages/twenty-server/src/engine/metadata-modules/field-metadata/utils/field-metadata-graphql-api-exception-handler.util.ts

@@ -1,14 +1,14 @@
+import {
+  ConflictError,
+  ForbiddenError,
+  InternalServerError,
+  NotFoundError,
+  UserInputError,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
 import {
   FieldMetadataException,
   FieldMetadataExceptionCode,
 } from 'src/engine/metadata-modules/field-metadata/field-metadata.exception';
-import {
-  UserInputError,
-  ForbiddenError,
-  ConflictError,
-  InternalServerError,
-  NotFoundError,
-} from 'src/engine/utils/graphql-errors.util';
 
 export const fieldMetadataGraphqlApiExceptionHandler = (error: Error) => {
   if (error instanceof FieldMetadataException) {

packages/twenty-server/src/engine/metadata-modules/object-metadata/utils/object-metadata-graphql-api-exception-handler.util.ts

@@ -1,14 +1,14 @@
+import {
+  ConflictError,
+  ForbiddenError,
+  InternalServerError,
+  NotFoundError,
+  UserInputError,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
 import {
   ObjectMetadataException,
   ObjectMetadataExceptionCode,
 } from 'src/engine/metadata-modules/object-metadata/object-metadata.exception';
-import {
-  UserInputError,
-  ForbiddenError,
-  ConflictError,
-  InternalServerError,
-  NotFoundError,
-} from 'src/engine/utils/graphql-errors.util';
 
 export const objectMetadataGraphqlApiExceptionHandler = (error: Error) => {
   if (error instanceof ObjectMetadataException) {

packages/twenty-server/src/engine/metadata-modules/relation-metadata/utils/relation-metadata-graphql-api-exception-handler.util.ts

@@ -1,13 +1,13 @@
+import {
+  ConflictError,
+  InternalServerError,
+  NotFoundError,
+  UserInputError,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
 import {
   RelationMetadataException,
   RelationMetadataExceptionCode,
 } from 'src/engine/metadata-modules/relation-metadata/relation-metadata.exception';
-import {
-  UserInputError,
-  ConflictError,
-  InternalServerError,
-  NotFoundError,
-} from 'src/engine/utils/graphql-errors.util';
 
 export const relationMetadataGraphqlApiExceptionHandler = (error: Error) => {
   if (error instanceof RelationMetadataException) {

packages/twenty-server/src/engine/metadata-modules/remote-server/remote-table/utils/remote-table-graphql-api-exception-handler.util.ts

@@ -1,13 +1,13 @@
+import {
+  ConflictError,
+  InternalServerError,
+  NotFoundError,
+  UserInputError,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
 import {
   RemoteTableException,
   RemoteTableExceptionCode,
 } from 'src/engine/metadata-modules/remote-server/remote-table/remote-table.exception';
-import {
-  UserInputError,
-  ConflictError,
-  InternalServerError,
-  NotFoundError,
-} from 'src/engine/utils/graphql-errors.util';
 
 export const remoteTableGraphqlApiExceptionHandler = (error: Error) => {
   if (error instanceof RemoteTableException) {

packages/twenty-server/src/engine/metadata-modules/remote-server/utils/remote-server-graphql-api-exception-handler.util.ts

@@ -1,14 +1,14 @@
+import {
+  ConflictError,
+  ForbiddenError,
+  InternalServerError,
+  NotFoundError,
+  UserInputError,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
 import {
   RemoteServerException,
   RemoteServerExceptionCode,
 } from 'src/engine/metadata-modules/remote-server/remote-server.exception';
-import {
-  UserInputError,
-  ForbiddenError,
-  ConflictError,
-  InternalServerError,
-  NotFoundError,
-} from 'src/engine/utils/graphql-errors.util';
 
 export const remoteServerGraphqlApiExceptionHandler = (error: any) => {
   if (error instanceof RemoteServerException) {

packages/twenty-server/src/engine/utils/global-exception-handler.util.ts

@@ -7,14 +7,14 @@ import { ExceptionHandlerUser } from 'src/engine/integrations/exception-handler/
 import {
   AuthenticationError,
   BaseGraphQLError,
-  ForbiddenError,
-  ValidationError,
-  NotFoundError,
   ConflictError,
-  MethodNotAllowedError,
-  TimeoutError,
   ErrorCode,
-} from 'src/engine/utils/graphql-errors.util';
+  ForbiddenError,
+  MethodNotAllowedError,
+  NotFoundError,
+  TimeoutError,
+  ValidationError,
+} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
 import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
 
 const graphQLPredefinedExceptions = {
@@ -88,6 +88,9 @@ export const convertExceptionToGraphQLError = (
   if (exception instanceof HttpException) {
     return convertHttpExceptionToGraphql(exception);
   }
+  if (exception instanceof BaseGraphQLError) {
+    return exception;
+  }
 
   return convertExceptionToGraphql(exception);
 };