Catch query timeout exceptions (#5680)

Query read timeouts happen when a remote server is not available. It breaks: - the remote server show page - the record table page of imported remote tables This PR will catch the exception so it does not go to Sentry in both cases. Also did 2 renaming.
2024-12-27 14:24:06 +03:00 · 2024-05-31 10:39:35 +02:00 · 2024-05-31 10:39:35 +02:00 · c60a3e49cd
commit c60a3e49cd
parent 5e1dfde3e4
11 changed files with 101 additions and 63 deletions
--- a/packages/twenty-server/src/command/command.ts
+++ b/packages/twenty-server/src/command/command.ts
@ -1,6 +1,6 @@
 import { CommandFactory } from 'nest-commander';

-import { filterException } from 'src/engine/utils/global-exception-handler.util';
+import { shouldFilterException } from 'src/engine/utils/global-exception-handler.util';
 import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
 import { LoggerService } from 'src/engine/integrations/logger/logger.service';

@ -10,7 +10,7 @@ async function bootstrap() {
  const errorHandler = (err: Error) => {
    loggerService.error(err?.message, err?.name);

-    if (filterException(err)) {
+    if (shouldFilterException(err)) {
      return;
    }

--- a/packages/twenty-server/src/database/typeorm/metadata/metadata.datasource.ts
+++ b/packages/twenty-server/src/database/typeorm/metadata/metadata.datasource.ts
@ -20,6 +20,9 @@ export const typeORMMetadataModuleOptions: TypeOrmModuleOptions = {
          rejectUnauthorized: false,
        }
      : undefined,
+  extra: {
+    query_timeout: 10000,
+  },
 };
 export const connectionSource = new DataSource(
  typeORMMetadataModuleOptions as DataSourceOptions,
--- a/packages/twenty-server/src/database/typeorm/typeorm.service.ts
+++ b/packages/twenty-server/src/database/typeorm/typeorm.service.ts
@ -38,6 +38,9 @@ export class TypeORMService implements OnModuleInit, OnModuleDestroy {
            rejectUnauthorized: false,
          }
        : undefined,
+      extra: {
+        query_timeout: 10000,
+      },
    });
  }

--- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/assert-is-valid-uuid.util.ts
+++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/utils/assert-is-valid-uuid.util.ts
--- a/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.service.ts
+++ b/packages/twenty-server/src/engine/api/graphql/workspace-query-runner/workspace-query-runner.service.ts
@ -3,6 +3,7 @@ import {
  Inject,
  Injectable,
  Logger,
+  RequestTimeoutException,
 } from '@nestjs/common';
 import { EventEmitter2 } from '@nestjs/event-emitter';

@ -49,7 +50,8 @@ import { QueryRunnerArgsFactory } from 'src/engine/api/graphql/workspace-query-r
 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/assertIsValidUuid.util';
+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 { WorkspaceQueryRunnerOptions } from './interfaces/query-runner-option.interface';
 import {
@ -577,28 +579,30 @@ export class WorkspaceQueryRunnerService {
        workspaceId,
      );

-    await workspaceDataSource?.query(`
-        SET search_path TO ${this.workspaceDataSourceService.getSchemaName(
-          workspaceId,
-        )};
-      `);
+    try {
+      return await workspaceDataSource?.transaction(
+        async (transactionManager) => {
+          await transactionManager.query(`
+          SET search_path TO ${this.workspaceDataSourceService.getSchemaName(
+            workspaceId,
+          )};
+        `);

-    return await workspaceDataSource?.transaction(
-      async (transactionManager) => {
-        await transactionManager.query(`
-        SET search_path TO ${this.workspaceDataSourceService.getSchemaName(
-          workspaceId,
-        )};
-      `);
+          const results = transactionManager.query<PGGraphQLResult>(
+            `SELECT graphql.resolve($1);`,
+            [query],
+          );

-        const results = transactionManager.query<PGGraphQLResult>(
-          `SELECT graphql.resolve($1);`,
-          [query],
-        );
+          return results;
+        },
+      );
+    } catch (error) {
+      if (isQueryTimeoutError(error)) {
+        throw new RequestTimeoutException(error.message);
+      }

-        return results;
-      },
-    );
+      throw error;
+    }
  }

  private async parseResult<Result>(
--- a/packages/twenty-server/src/engine/integrations/exception-handler/hooks/use-exception-handler.hook.ts
+++ b/packages/twenty-server/src/engine/integrations/exception-handler/hooks/use-exception-handler.hook.ts
@ -11,7 +11,7 @@ import { GraphQLContext } from 'src/engine/api/graphql/graphql-config/interfaces
 import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
 import {
  convertExceptionToGraphQLError,
-  filterException,
+  shouldFilterException,
 } from 'src/engine/utils/global-exception-handler.util';

 export type ExceptionHandlerPluginOptions = {
@ -69,7 +69,7 @@ export const useExceptionHandler = <PluginContext extends GraphQLContext>(
              }>(
                (acc, err) => {
                  // Filter out exceptions that we don't want to be captured by exception handler
-                  if (filterException(err?.originalError ?? err)) {
+                  if (shouldFilterException(err?.originalError ?? err)) {
                    acc.filtered.push(err);
                  } else {
                    acc.unfiltered.push(err);
--- a/packages/twenty-server/src/engine/metadata-modules/remote-server/remote-table/distant-table/distant-table.service.ts
+++ b/packages/twenty-server/src/engine/metadata-modules/remote-server/remote-table/distant-table/distant-table.service.ts
@ -1,4 +1,8 @@
-import { BadRequestException, Injectable } from '@nestjs/common';
+import {
+  BadRequestException,
+  Injectable,
+  RequestTimeoutException,
+} from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';

 import { EntityManager, Repository } from 'typeorm';
@ -12,6 +16,7 @@ import { WorkspaceDataSourceService } from 'src/engine/workspace-datasource/work
 import { DistantTables } from 'src/engine/metadata-modules/remote-server/remote-table/distant-table/types/distant-table';
 import { STRIPE_DISTANT_TABLES } from 'src/engine/metadata-modules/remote-server/remote-table/distant-table/utils/stripe-distant-tables.util';
 import { PostgresTableSchemaColumn } from 'src/engine/metadata-modules/remote-server/types/postgres-table-schema-column';
+import { isQueryTimeoutError } from 'src/engine/utils/query-timeout.util';

@Injectable()
 export class DistantTableService {
@ -70,44 +75,54 @@ export class DistantTableService {
        workspaceId,
      );

-    const distantTables = await workspaceDataSource.transaction(
-      async (entityManager: EntityManager) => {
-        await entityManager.query(`CREATE SCHEMA "${tmpSchemaName}"`);
+    try {
+      const distantTables = await workspaceDataSource.transaction(
+        async (entityManager: EntityManager) => {
+          await entityManager.query(`CREATE SCHEMA "${tmpSchemaName}"`);

-        const tableLimitationsOptions = tableName
-          ? ` LIMIT TO ("${tableName}")`
-          : '';
+          const tableLimitationsOptions = tableName
+            ? ` LIMIT TO ("${tableName}")`
+            : '';

-        await entityManager.query(
-          `IMPORT FOREIGN SCHEMA "${remoteServer.schema}"${tableLimitationsOptions} FROM SERVER "${remoteServer.foreignDataWrapperId}" INTO "${tmpSchemaName}"`,
+          await entityManager.query(
+            `IMPORT FOREIGN SCHEMA "${remoteServer.schema}"${tableLimitationsOptions} FROM SERVER "${remoteServer.foreignDataWrapperId}" INTO "${tmpSchemaName}"`,
+          );
+
+          const createdForeignTableNames = await entityManager.query(
+            `SELECT table_name, column_name, data_type, udt_name FROM information_schema.columns WHERE table_schema = '${tmpSchemaName}'`,
+          );
+
+          await entityManager.query(`DROP SCHEMA "${tmpSchemaName}" CASCADE`);
+
+          return createdForeignTableNames.reduce(
+            (acc, { table_name, column_name, data_type, udt_name }) => {
+              if (!acc[table_name]) {
+                acc[table_name] = [];
+              }
+
+              acc[table_name].push({
+                columnName: column_name,
+                dataType: data_type,
+                udtName: udt_name,
+              });
+
+              return acc;
+            },
+            {},
+          );
+        },
+      );
+
+      return distantTables;
+    } catch (error) {
+      if (isQueryTimeoutError(error)) {
+        throw new RequestTimeoutException(
+          `Could not find distant tables: ${error.message}`,
        );
+      }

-        const createdForeignTableNames = await entityManager.query(
-          `SELECT table_name, column_name, data_type, udt_name FROM information_schema.columns WHERE table_schema = '${tmpSchemaName}'`,
-        );
-
-        await entityManager.query(`DROP SCHEMA "${tmpSchemaName}" CASCADE`);
-
-        return createdForeignTableNames.reduce(
-          (acc, { table_name, column_name, data_type, udt_name }) => {
-            if (!acc[table_name]) {
-              acc[table_name] = [];
-            }
-
-            acc[table_name].push({
-              columnName: column_name,
-              dataType: data_type,
-              udtName: udt_name,
-            });
-
-            return acc;
-          },
-          {},
-        );
-      },
-    );
-
-    return distantTables;
+      throw error;
+    }
  }

  private getDistantTablesFromStaticSchema(
--- a/packages/twenty-server/src/engine/utils/global-exception-handler.util.ts
+++ b/packages/twenty-server/src/engine/utils/global-exception-handler.util.ts
@ -10,6 +10,7 @@ import {
  NotFoundError,
  ConflictError,
  MethodNotAllowedError,
+  TimeoutError,
 } from 'src/engine/utils/graphql-errors.util';
 import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';

@ -19,6 +20,7 @@ const graphQLPredefinedExceptions = {
  403: ForbiddenError,
  404: NotFoundError,
  405: MethodNotAllowedError,
+  408: TimeoutError,
  409: ConflictError,
 };

@ -32,7 +34,7 @@ export const handleExceptionAndConvertToGraphQLError = (
  return convertExceptionToGraphQLError(exception);
 };

-export const filterException = (exception: Error): boolean => {
+export const shouldFilterException = (exception: Error): boolean => {
  if (exception instanceof HttpException && exception.getStatus() < 500) {
    return true;
  }
@ -45,7 +47,7 @@ export const handleException = (
  exceptionHandlerService: ExceptionHandlerService,
  user?: ExceptionHandlerUser,
 ): void => {
-  if (filterException(exception)) {
+  if (shouldFilterException(exception)) {
    return;
  }

--- a/packages/twenty-server/src/engine/utils/graphql-errors.util.ts
+++ b/packages/twenty-server/src/engine/utils/graphql-errors.util.ts
@ -157,3 +157,11 @@ export class ConflictError extends BaseGraphQLError {
    Object.defineProperty(this, 'name', { value: 'ConflictError' });
  }
 }
+
+export class TimeoutError extends BaseGraphQLError {
+  constructor(message: string, extensions?: Record<string, any>) {
+    super(message, 'TIMEOUT', extensions);
+
+    Object.defineProperty(this, 'name', { value: 'TimeoutError' });
+  }
+}
--- a/packages/twenty-server/src/engine/utils/query-timeout.util.ts
+++ b/packages/twenty-server/src/engine/utils/query-timeout.util.ts
@ -0,0 +1,3 @@
+export const isQueryTimeoutError = (error: Error) => {
+  return error.message.includes('Query read timeout');
+};
--- a/packages/twenty-server/src/queue-worker/queue-worker.ts
+++ b/packages/twenty-server/src/queue-worker/queue-worker.ts
@ -5,7 +5,7 @@ import {
  MessageQueueJobData,
 } from 'src/engine/integrations/message-queue/interfaces/message-queue-job.interface';

-import { filterException } from 'src/engine/utils/global-exception-handler.util';
+import { shouldFilterException } from 'src/engine/utils/global-exception-handler.util';
 import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
 import { LoggerService } from 'src/engine/integrations/logger/logger.service';
 import { JobsModule } from 'src/engine/integrations/message-queue/jobs.module';
@ -54,7 +54,7 @@ async function bootstrap() {
  } catch (err) {
    loggerService?.error(err?.message, err?.name);

-    if (!filterException(err)) {
+    if (!shouldFilterException(err)) {
      exceptionHandlerService?.captureExceptions([err]);
    }