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
This commit is contained in:
Thomas Trompette
2024-06-19 10:39:09 +02:00
committed by GitHub
parent 6fd8dab552
commit d045bcbb94
2 changed files with 63 additions and 22 deletions

View File

@ -13,6 +13,7 @@ import {
ConflictError, ConflictError,
MethodNotAllowedError, MethodNotAllowedError,
TimeoutError, TimeoutError,
ErrorCode,
} from 'src/engine/utils/graphql-errors.util'; } from 'src/engine/utils/graphql-errors.util';
import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service'; import { ExceptionHandlerService } from 'src/engine/integrations/exception-handler/exception-handler.service';
@ -26,6 +27,17 @@ const graphQLPredefinedExceptions = {
409: ConflictError, 409: ConflictError,
}; };
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 handleExceptionAndConvertToGraphQLError = ( export const handleExceptionAndConvertToGraphQLError = (
exception: Error, exception: Error,
exceptionHandlerService: ExceptionHandlerService, exceptionHandlerService: ExceptionHandlerService,
@ -43,6 +55,14 @@ export const shouldFilterException = (exception: Error): boolean => {
) { ) {
return true; return true;
} }
if (
exception instanceof BaseGraphQLError &&
graphQLErrorCodesToFilter.includes(exception?.extensions?.code)
) {
return true;
}
if (exception instanceof HttpException && exception.getStatus() < 500) { if (exception instanceof HttpException && exception.getStatus() < 500) {
return true; return true;
} }
@ -50,7 +70,7 @@ export const shouldFilterException = (exception: Error): boolean => {
return false; return false;
}; };
export const handleException = ( const handleException = (
exception: Error, exception: Error,
exceptionHandlerService: ExceptionHandlerService, exceptionHandlerService: ExceptionHandlerService,
user?: ExceptionHandlerUser, user?: ExceptionHandlerUser,
@ -72,7 +92,7 @@ export const convertExceptionToGraphQLError = (
return convertExceptionToGraphql(exception); return convertExceptionToGraphql(exception);
}; };
export const convertHttpExceptionToGraphql = (exception: HttpException) => { const convertHttpExceptionToGraphql = (exception: HttpException) => {
const status = exception.getStatus(); const status = exception.getStatus();
let error: BaseGraphQLError; let error: BaseGraphQLError;
@ -97,7 +117,10 @@ export const convertHttpExceptionToGraphql = (exception: HttpException) => {
}; };
export const convertExceptionToGraphql = (exception: Error) => { export const convertExceptionToGraphql = (exception: Error) => {
const error = new BaseGraphQLError(exception.name, 'INTERNAL_SERVER_ERROR'); const error = new BaseGraphQLError(
exception.name,
ErrorCode.INTERNAL_SERVER_ERROR,
);
error.stack = exception.stack; error.stack = exception.stack;
error.extensions['response'] = exception.message; error.extensions['response'] = exception.message;

View File

@ -15,7 +15,22 @@ declare module 'graphql' {
} }
} }
export class BaseGraphQLError extends Error implements GraphQLError { export enum ErrorCode {
GRAPHQL_PARSE_FAILED = 'GRAPHQL_PARSE_FAILED',
GRAPHQL_VALIDATION_FAILED = 'GRAPHQL_VALIDATION_FAILED',
UNAUTHENTICATED = 'UNAUTHENTICATED',
FORBIDDEN = 'FORBIDDEN',
PERSISTED_QUERY_NOT_FOUND = 'PERSISTED_QUERY_NOT_FOUND',
PERSISTED_QUERY_NOT_SUPPORTED = 'PERSISTED_QUERY_NOT_SUPPORTED',
BAD_USER_INPUT = 'BAD_USER_INPUT',
NOT_FOUND = 'NOT_FOUND',
METHOD_NOT_ALLOWED = 'METHOD_NOT_ALLOWED',
CONFLICT = 'CONFLICT',
TIMEOUT = 'TIMEOUT',
INTERNAL_SERVER_ERROR = 'INTERNAL_SERVER_ERROR',
}
export class BaseGraphQLError extends GraphQLError {
public extensions: Record<string, any>; public extensions: Record<string, any>;
override readonly name!: string; override readonly name!: string;
readonly locations: ReadonlyArray<SourceLocation> | undefined; readonly locations: ReadonlyArray<SourceLocation> | undefined;
@ -76,7 +91,7 @@ function toGraphQLError(error: BaseGraphQLError): GraphQLError {
export class SyntaxError extends BaseGraphQLError { export class SyntaxError extends BaseGraphQLError {
constructor(message: string) { constructor(message: string) {
super(message, 'GRAPHQL_PARSE_FAILED'); super(message, ErrorCode.GRAPHQL_PARSE_FAILED);
Object.defineProperty(this, 'name', { value: 'SyntaxError' }); Object.defineProperty(this, 'name', { value: 'SyntaxError' });
} }
@ -84,23 +99,23 @@ export class SyntaxError extends BaseGraphQLError {
export class ValidationError extends BaseGraphQLError { export class ValidationError extends BaseGraphQLError {
constructor(message: string) { constructor(message: string) {
super(message, 'GRAPHQL_VALIDATION_FAILED'); super(message, ErrorCode.GRAPHQL_VALIDATION_FAILED);
Object.defineProperty(this, 'name', { value: 'ValidationError' }); Object.defineProperty(this, 'name', { value: 'ValidationError' });
} }
} }
export class AuthenticationError extends BaseGraphQLError { export class AuthenticationError extends BaseGraphQLError {
constructor(message: string, extensions?: Record<string, any>) { constructor(message: string) {
super(message, 'UNAUTHENTICATED', extensions); super(message, ErrorCode.UNAUTHENTICATED);
Object.defineProperty(this, 'name', { value: 'AuthenticationError' }); Object.defineProperty(this, 'name', { value: 'AuthenticationError' });
} }
} }
export class ForbiddenError extends BaseGraphQLError { export class ForbiddenError extends BaseGraphQLError {
constructor(message: string, extensions?: Record<string, any>) { constructor(message: string) {
super(message, 'FORBIDDEN', extensions); super(message, ErrorCode.FORBIDDEN);
Object.defineProperty(this, 'name', { value: 'ForbiddenError' }); Object.defineProperty(this, 'name', { value: 'ForbiddenError' });
} }
@ -108,7 +123,7 @@ export class ForbiddenError extends BaseGraphQLError {
export class PersistedQueryNotFoundError extends BaseGraphQLError { export class PersistedQueryNotFoundError extends BaseGraphQLError {
constructor() { constructor() {
super('PersistedQueryNotFound', 'PERSISTED_QUERY_NOT_FOUND'); super('PersistedQueryNotFound', ErrorCode.PERSISTED_QUERY_NOT_FOUND);
Object.defineProperty(this, 'name', { Object.defineProperty(this, 'name', {
value: 'PersistedQueryNotFoundError', value: 'PersistedQueryNotFoundError',
@ -118,7 +133,10 @@ export class PersistedQueryNotFoundError extends BaseGraphQLError {
export class PersistedQueryNotSupportedError extends BaseGraphQLError { export class PersistedQueryNotSupportedError extends BaseGraphQLError {
constructor() { constructor() {
super('PersistedQueryNotSupported', 'PERSISTED_QUERY_NOT_SUPPORTED'); super(
'PersistedQueryNotSupported',
ErrorCode.PERSISTED_QUERY_NOT_SUPPORTED,
);
Object.defineProperty(this, 'name', { Object.defineProperty(this, 'name', {
value: 'PersistedQueryNotSupportedError', value: 'PersistedQueryNotSupportedError',
@ -127,40 +145,40 @@ export class PersistedQueryNotSupportedError extends BaseGraphQLError {
} }
export class UserInputError extends BaseGraphQLError { export class UserInputError extends BaseGraphQLError {
constructor(message: string, extensions?: Record<string, any>) { constructor(message: string) {
super(message, 'BAD_USER_INPUT', extensions); super(message, ErrorCode.BAD_USER_INPUT);
Object.defineProperty(this, 'name', { value: 'UserInputError' }); Object.defineProperty(this, 'name', { value: 'UserInputError' });
} }
} }
export class NotFoundError extends BaseGraphQLError { export class NotFoundError extends BaseGraphQLError {
constructor(message: string, extensions?: Record<string, any>) { constructor(message: string) {
super(message, 'NOT_FOUND', extensions); super(message, ErrorCode.NOT_FOUND);
Object.defineProperty(this, 'name', { value: 'NotFoundError' }); Object.defineProperty(this, 'name', { value: 'NotFoundError' });
} }
} }
export class MethodNotAllowedError extends BaseGraphQLError { export class MethodNotAllowedError extends BaseGraphQLError {
constructor(message: string, extensions?: Record<string, any>) { constructor(message: string) {
super(message, 'METHOD_NOT_ALLOWED', extensions); super(message, ErrorCode.METHOD_NOT_ALLOWED);
Object.defineProperty(this, 'name', { value: 'MethodNotAllowedError' }); Object.defineProperty(this, 'name', { value: 'MethodNotAllowedError' });
} }
} }
export class ConflictError extends BaseGraphQLError { export class ConflictError extends BaseGraphQLError {
constructor(message: string, extensions?: Record<string, any>) { constructor(message: string) {
super(message, 'CONFLICT', extensions); super(message, ErrorCode.CONFLICT);
Object.defineProperty(this, 'name', { value: 'ConflictError' }); Object.defineProperty(this, 'name', { value: 'ConflictError' });
} }
} }
export class TimeoutError extends BaseGraphQLError { export class TimeoutError extends BaseGraphQLError {
constructor(message: string, extensions?: Record<string, any>) { constructor(message: string) {
super(message, 'TIMEOUT', extensions); super(message, ErrorCode.TIMEOUT);
Object.defineProperty(this, 'name', { value: 'TimeoutError' }); Object.defineProperty(this, 'name', { value: 'TimeoutError' });
} }