Skip to content

Commit ab3aaae

Browse files
thomtrpWeiko
authored andcommitted
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.
1 parent 5bed1a3 commit ab3aaae

File tree

11 files changed

+104
-66
lines changed

11 files changed

+104
-66
lines changed

packages/twenty-server/src/command/command.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { CommandFactory } from 'nest-commander';
22

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

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

13-
if (filterException(err)) {
13+
if (shouldFilterException(err)) {
1414
return;
1515
}
1616

packages/twenty-server/src/database/typeorm/metadata/metadata.datasource.ts

+3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@ export const typeORMMetadataModuleOptions: TypeOrmModuleOptions = {
2020
rejectUnauthorized: false,
2121
}
2222
: undefined,
23+
extra: {
24+
query_timeout: 10000,
25+
},
2326
};
2427
export const connectionSource = new DataSource(
2528
typeORMMetadataModuleOptions as DataSourceOptions,

packages/twenty-server/src/database/typeorm/typeorm.service.ts

+3
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,9 @@ export class TypeORMService implements OnModuleInit, OnModuleDestroy {
3838
rejectUnauthorized: false,
3939
}
4040
: undefined,
41+
extra: {
42+
query_timeout: 10000,
43+
},
4144
});
4245
}
4346

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

+26-22
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import {
33
Inject,
44
Injectable,
55
Logger,
6+
RequestTimeoutException,
67
} from '@nestjs/common';
78
import { EventEmitter2 } from '@nestjs/event-emitter';
89

@@ -49,7 +50,8 @@ import { QueryRunnerArgsFactory } from 'src/engine/api/graphql/workspace-query-r
4950
import { QueryResultGettersFactory } from 'src/engine/api/graphql/workspace-query-runner/factories/query-result-getters.factory';
5051
import { assertMutationNotOnRemoteObject } from 'src/engine/metadata-modules/object-metadata/utils/assert-mutation-not-on-remote-object.util';
5152
import { STANDARD_OBJECT_IDS } from 'src/engine/workspace-manager/workspace-sync-metadata/constants/standard-object-ids';
52-
import { assertIsValidUuid } from 'src/engine/api/graphql/workspace-query-runner/utils/assertIsValidUuid.util';
53+
import { assertIsValidUuid } from 'src/engine/api/graphql/workspace-query-runner/utils/assert-is-valid-uuid.util';
54+
import { isQueryTimeoutError } from 'src/engine/utils/query-timeout.util';
5355

5456
import { WorkspaceQueryRunnerOptions } from './interfaces/query-runner-option.interface';
5557
import {
@@ -577,28 +579,30 @@ export class WorkspaceQueryRunnerService {
577579
workspaceId,
578580
);
579581

580-
await workspaceDataSource?.query(`
581-
SET search_path TO ${this.workspaceDataSourceService.getSchemaName(
582-
workspaceId,
583-
)};
584-
`);
585-
586-
return await workspaceDataSource?.transaction(
587-
async (transactionManager) => {
588-
await transactionManager.query(`
589-
SET search_path TO ${this.workspaceDataSourceService.getSchemaName(
590-
workspaceId,
591-
)};
592-
`);
593-
594-
const results = transactionManager.query<PGGraphQLResult>(
595-
`SELECT graphql.resolve($1);`,
596-
[query],
597-
);
582+
try {
583+
return await workspaceDataSource?.transaction(
584+
async (transactionManager) => {
585+
await transactionManager.query(`
586+
SET search_path TO ${this.workspaceDataSourceService.getSchemaName(
587+
workspaceId,
588+
)};
589+
`);
590+
591+
const results = transactionManager.query<PGGraphQLResult>(
592+
`SELECT graphql.resolve($1);`,
593+
[query],
594+
);
595+
596+
return results;
597+
},
598+
);
599+
} catch (error) {
600+
if (isQueryTimeoutError(error)) {
601+
throw new RequestTimeoutException(error.message);
602+
}
598603

599-
return results;
600-
},
601-
);
604+
throw error;
605+
}
602606
}
603607

604608
private async parseResult<Result>(

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { GraphQLContext } from 'src/engine/api/graphql/graphql-config/interfaces
1111
import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
1212
import {
1313
convertExceptionToGraphQLError,
14-
filterException,
14+
shouldFilterException,
1515
} from 'src/engine/utils/global-exception-handler.util';
1616

1717
export type ExceptionHandlerPluginOptions = {
@@ -69,7 +69,7 @@ export const useExceptionHandler = <PluginContext extends GraphQLContext>(
6969
}>(
7070
(acc, err) => {
7171
// Filter out exceptions that we don't want to be captured by exception handler
72-
if (filterException(err?.originalError ?? err)) {
72+
if (shouldFilterException(err?.originalError ?? err)) {
7373
acc.filtered.push(err);
7474
} else {
7575
acc.unfiltered.push(err);

packages/twenty-server/src/engine/metadata-modules/remote-server/remote-table/distant-table/distant-table.service.ts

+51-36
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1-
import { BadRequestException, Injectable } from '@nestjs/common';
1+
import {
2+
BadRequestException,
3+
Injectable,
4+
RequestTimeoutException,
5+
} from '@nestjs/common';
26
import { InjectRepository } from '@nestjs/typeorm';
37

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

1621
@Injectable()
1722
export class DistantTableService {
@@ -70,44 +75,54 @@ export class DistantTableService {
7075
workspaceId,
7176
);
7277

73-
const distantTables = await workspaceDataSource.transaction(
74-
async (entityManager: EntityManager) => {
75-
await entityManager.query(`CREATE SCHEMA "${tmpSchemaName}"`);
76-
77-
const tableLimitationsOptions = tableName
78-
? ` LIMIT TO ("${tableName}")`
79-
: '';
80-
81-
await entityManager.query(
82-
`IMPORT FOREIGN SCHEMA "${remoteServer.schema}"${tableLimitationsOptions} FROM SERVER "${remoteServer.foreignDataWrapperId}" INTO "${tmpSchemaName}"`,
83-
);
84-
85-
const createdForeignTableNames = await entityManager.query(
86-
`SELECT table_name, column_name, data_type, udt_name FROM information_schema.columns WHERE table_schema = '${tmpSchemaName}'`,
87-
);
88-
89-
await entityManager.query(`DROP SCHEMA "${tmpSchemaName}" CASCADE`);
90-
91-
return createdForeignTableNames.reduce(
92-
(acc, { table_name, column_name, data_type, udt_name }) => {
93-
if (!acc[table_name]) {
94-
acc[table_name] = [];
95-
}
96-
97-
acc[table_name].push({
98-
columnName: column_name,
99-
dataType: data_type,
100-
udtName: udt_name,
101-
});
78+
try {
79+
const distantTables = await workspaceDataSource.transaction(
80+
async (entityManager: EntityManager) => {
81+
await entityManager.query(`CREATE SCHEMA "${tmpSchemaName}"`);
82+
83+
const tableLimitationsOptions = tableName
84+
? ` LIMIT TO ("${tableName}")`
85+
: '';
86+
87+
await entityManager.query(
88+
`IMPORT FOREIGN SCHEMA "${remoteServer.schema}"${tableLimitationsOptions} FROM SERVER "${remoteServer.foreignDataWrapperId}" INTO "${tmpSchemaName}"`,
89+
);
90+
91+
const createdForeignTableNames = await entityManager.query(
92+
`SELECT table_name, column_name, data_type, udt_name FROM information_schema.columns WHERE table_schema = '${tmpSchemaName}'`,
93+
);
94+
95+
await entityManager.query(`DROP SCHEMA "${tmpSchemaName}" CASCADE`);
96+
97+
return createdForeignTableNames.reduce(
98+
(acc, { table_name, column_name, data_type, udt_name }) => {
99+
if (!acc[table_name]) {
100+
acc[table_name] = [];
101+
}
102+
103+
acc[table_name].push({
104+
columnName: column_name,
105+
dataType: data_type,
106+
udtName: udt_name,
107+
});
108+
109+
return acc;
110+
},
111+
{},
112+
);
113+
},
114+
);
102115

103-
return acc;
104-
},
105-
{},
116+
return distantTables;
117+
} catch (error) {
118+
if (isQueryTimeoutError(error)) {
119+
throw new RequestTimeoutException(
120+
`Could not find distant tables: ${error.message}`,
106121
);
107-
},
108-
);
122+
}
109123

110-
return distantTables;
124+
throw error;
125+
}
111126
}
112127

113128
private getDistantTablesFromStaticSchema(

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

+4-2
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
NotFoundError,
1111
ConflictError,
1212
MethodNotAllowedError,
13+
TimeoutError,
1314
} from 'src/engine/utils/graphql-errors.util';
1415
import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
1516

@@ -19,6 +20,7 @@ const graphQLPredefinedExceptions = {
1920
403: ForbiddenError,
2021
404: NotFoundError,
2122
405: MethodNotAllowedError,
23+
408: TimeoutError,
2224
409: ConflictError,
2325
};
2426

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

35-
export const filterException = (exception: Error): boolean => {
37+
export const shouldFilterException = (exception: Error): boolean => {
3638
if (exception instanceof HttpException && exception.getStatus() < 500) {
3739
return true;
3840
}
@@ -45,7 +47,7 @@ export const handleException = (
4547
exceptionHandlerService: ExceptionHandlerService,
4648
user?: ExceptionHandlerUser,
4749
): void => {
48-
if (filterException(exception)) {
50+
if (shouldFilterException(exception)) {
4951
return;
5052
}
5153

packages/twenty-server/src/engine/utils/graphql-errors.util.ts

+8
Original file line numberDiff line numberDiff line change
@@ -157,3 +157,11 @@ export class ConflictError extends BaseGraphQLError {
157157
Object.defineProperty(this, 'name', { value: 'ConflictError' });
158158
}
159159
}
160+
161+
export class TimeoutError extends BaseGraphQLError {
162+
constructor(message: string, extensions?: Record<string, any>) {
163+
super(message, 'TIMEOUT', extensions);
164+
165+
Object.defineProperty(this, 'name', { value: 'TimeoutError' });
166+
}
167+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
export const isQueryTimeoutError = (error: Error) => {
2+
return error.message.includes('Query read timeout');
3+
};

packages/twenty-server/src/queue-worker/queue-worker.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
MessageQueueJobData,
66
} from 'src/engine/integrations/message-queue/interfaces/message-queue-job.interface';
77

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

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

0 commit comments

Comments
 (0)