Skip to content

Commit d045bcb

Browse files
authored
Add http status to graphql errors (#5896)
Graphql errors are not properly filtered by our handler. We still receive errors like `NOT_FOUND` in sentry while they should be filtered. Example [here](https://twenty-v7.sentry.io/issues/5490383016/?environment=prod&project=4507072499810304&query=is%3Aunresolved+issue.priority%3A%5Bhigh%2C+medium%5D&referrer=issue-stream&statsPeriod=7d&stream_index=6). We associate statuses with errors in our map `graphQLPredefinedExceptions` but we cannot retrieve the status from the error. This PR lists the codes that should be filtered. To test: - call `findDuplicates` with an invalid id - before, server would breaks - now the error is simply returned
1 parent 6fd8dab commit d045bcb

File tree

2 files changed

+63
-22
lines changed

2 files changed

+63
-22
lines changed

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

+26-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
ConflictError,
1414
MethodNotAllowedError,
1515
TimeoutError,
16+
ErrorCode,
1617
} from 'src/engine/utils/graphql-errors.util';
1718
import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
1819

@@ -26,6 +27,17 @@ const graphQLPredefinedExceptions = {
2627
409: ConflictError,
2728
};
2829

30+
export const graphQLErrorCodesToFilter = [
31+
ErrorCode.GRAPHQL_VALIDATION_FAILED,
32+
ErrorCode.UNAUTHENTICATED,
33+
ErrorCode.FORBIDDEN,
34+
ErrorCode.NOT_FOUND,
35+
ErrorCode.METHOD_NOT_ALLOWED,
36+
ErrorCode.TIMEOUT,
37+
ErrorCode.CONFLICT,
38+
ErrorCode.BAD_USER_INPUT,
39+
];
40+
2941
export const handleExceptionAndConvertToGraphQLError = (
3042
exception: Error,
3143
exceptionHandlerService: ExceptionHandlerService,
@@ -43,14 +55,22 @@ export const shouldFilterException = (exception: Error): boolean => {
4355
) {
4456
return true;
4557
}
58+
59+
if (
60+
exception instanceof BaseGraphQLError &&
61+
graphQLErrorCodesToFilter.includes(exception?.extensions?.code)
62+
) {
63+
return true;
64+
}
65+
4666
if (exception instanceof HttpException && exception.getStatus() < 500) {
4767
return true;
4868
}
4969

5070
return false;
5171
};
5272

53-
export const handleException = (
73+
const handleException = (
5474
exception: Error,
5575
exceptionHandlerService: ExceptionHandlerService,
5676
user?: ExceptionHandlerUser,
@@ -72,7 +92,7 @@ export const convertExceptionToGraphQLError = (
7292
return convertExceptionToGraphql(exception);
7393
};
7494

75-
export const convertHttpExceptionToGraphql = (exception: HttpException) => {
95+
const convertHttpExceptionToGraphql = (exception: HttpException) => {
7696
const status = exception.getStatus();
7797
let error: BaseGraphQLError;
7898

@@ -97,7 +117,10 @@ export const convertHttpExceptionToGraphql = (exception: HttpException) => {
97117
};
98118

99119
export const convertExceptionToGraphql = (exception: Error) => {
100-
const error = new BaseGraphQLError(exception.name, 'INTERNAL_SERVER_ERROR');
120+
const error = new BaseGraphQLError(
121+
exception.name,
122+
ErrorCode.INTERNAL_SERVER_ERROR,
123+
);
101124

102125
error.stack = exception.stack;
103126
error.extensions['response'] = exception.message;

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

+37-19
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,22 @@ declare module 'graphql' {
1515
}
1616
}
1717

18-
export class BaseGraphQLError extends Error implements GraphQLError {
18+
export enum ErrorCode {
19+
GRAPHQL_PARSE_FAILED = 'GRAPHQL_PARSE_FAILED',
20+
GRAPHQL_VALIDATION_FAILED = 'GRAPHQL_VALIDATION_FAILED',
21+
UNAUTHENTICATED = 'UNAUTHENTICATED',
22+
FORBIDDEN = 'FORBIDDEN',
23+
PERSISTED_QUERY_NOT_FOUND = 'PERSISTED_QUERY_NOT_FOUND',
24+
PERSISTED_QUERY_NOT_SUPPORTED = 'PERSISTED_QUERY_NOT_SUPPORTED',
25+
BAD_USER_INPUT = 'BAD_USER_INPUT',
26+
NOT_FOUND = 'NOT_FOUND',
27+
METHOD_NOT_ALLOWED = 'METHOD_NOT_ALLOWED',
28+
CONFLICT = 'CONFLICT',
29+
TIMEOUT = 'TIMEOUT',
30+
INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR',
31+
}
32+
33+
export class BaseGraphQLError extends GraphQLError {
1934
public extensions: Record<string, any>;
2035
override readonly name!: string;
2136
readonly locations: ReadonlyArray<SourceLocation> | undefined;
@@ -76,39 +91,39 @@ function toGraphQLError(error: BaseGraphQLError): GraphQLError {
7691

7792
export class SyntaxError extends BaseGraphQLError {
7893
constructor(message: string) {
79-
super(message, 'GRAPHQL_PARSE_FAILED');
94+
super(message, ErrorCode.GRAPHQL_PARSE_FAILED);
8095

8196
Object.defineProperty(this, 'name', { value: 'SyntaxError' });
8297
}
8398
}
8499

85100
export class ValidationError extends BaseGraphQLError {
86101
constructor(message: string) {
87-
super(message, 'GRAPHQL_VALIDATION_FAILED');
102+
super(message, ErrorCode.GRAPHQL_VALIDATION_FAILED);
88103

89104
Object.defineProperty(this, 'name', { value: 'ValidationError' });
90105
}
91106
}
92107

93108
export class AuthenticationError extends BaseGraphQLError {
94-
constructor(message: string, extensions?: Record<string, any>) {
95-
super(message, 'UNAUTHENTICATED', extensions);
109+
constructor(message: string) {
110+
super(message, ErrorCode.UNAUTHENTICATED);
96111

97112
Object.defineProperty(this, 'name', { value: 'AuthenticationError' });
98113
}
99114
}
100115

101116
export class ForbiddenError extends BaseGraphQLError {
102-
constructor(message: string, extensions?: Record<string, any>) {
103-
super(message, 'FORBIDDEN', extensions);
117+
constructor(message: string) {
118+
super(message, ErrorCode.FORBIDDEN);
104119

105120
Object.defineProperty(this, 'name', { value: 'ForbiddenError' });
106121
}
107122
}
108123

109124
export class PersistedQueryNotFoundError extends BaseGraphQLError {
110125
constructor() {
111-
super('PersistedQueryNotFound', 'PERSISTED_QUERY_NOT_FOUND');
126+
super('PersistedQueryNotFound', ErrorCode.PERSISTED_QUERY_NOT_FOUND);
112127

113128
Object.defineProperty(this, 'name', {
114129
value: 'PersistedQueryNotFoundError',
@@ -118,7 +133,10 @@ export class PersistedQueryNotFoundError extends BaseGraphQLError {
118133

119134
export class PersistedQueryNotSupportedError extends BaseGraphQLError {
120135
constructor() {
121-
super('PersistedQueryNotSupported', 'PERSISTED_QUERY_NOT_SUPPORTED');
136+
super(
137+
'PersistedQueryNotSupported',
138+
ErrorCode.PERSISTED_QUERY_NOT_SUPPORTED,
139+
);
122140

123141
Object.defineProperty(this, 'name', {
124142
value: 'PersistedQueryNotSupportedError',
@@ -127,40 +145,40 @@ export class PersistedQueryNotSupportedError extends BaseGraphQLError {
127145
}
128146

129147
export class UserInputError extends BaseGraphQLError {
130-
constructor(message: string, extensions?: Record<string, any>) {
131-
super(message, 'BAD_USER_INPUT', extensions);
148+
constructor(message: string) {
149+
super(message, ErrorCode.BAD_USER_INPUT);
132150

133151
Object.defineProperty(this, 'name', { value: 'UserInputError' });
134152
}
135153
}
136154

137155
export class NotFoundError extends BaseGraphQLError {
138-
constructor(message: string, extensions?: Record<string, any>) {
139-
super(message, 'NOT_FOUND', extensions);
156+
constructor(message: string) {
157+
super(message, ErrorCode.NOT_FOUND);
140158

141159
Object.defineProperty(this, 'name', { value: 'NotFoundError' });
142160
}
143161
}
144162

145163
export class MethodNotAllowedError extends BaseGraphQLError {
146-
constructor(message: string, extensions?: Record<string, any>) {
147-
super(message, 'METHOD_NOT_ALLOWED', extensions);
164+
constructor(message: string) {
165+
super(message, ErrorCode.METHOD_NOT_ALLOWED);
148166

149167
Object.defineProperty(this, 'name', { value: 'MethodNotAllowedError' });
150168
}
151169
}
152170

153171
export class ConflictError extends BaseGraphQLError {
154-
constructor(message: string, extensions?: Record<string, any>) {
155-
super(message, 'CONFLICT', extensions);
172+
constructor(message: string) {
173+
super(message, ErrorCode.CONFLICT);
156174

157175
Object.defineProperty(this, 'name', { value: 'ConflictError' });
158176
}
159177
}
160178

161179
export class TimeoutError extends BaseGraphQLError {
162-
constructor(message: string, extensions?: Record<string, any>) {
163-
super(message, 'TIMEOUT', extensions);
180+
constructor(message: string) {
181+
super(message, ErrorCode.TIMEOUT);
164182

165183
Object.defineProperty(this, 'name', { value: 'TimeoutError' });
166184
}

0 commit comments

Comments
 (0)