Improve sentry grouping (#12007)
This PR attemps at improving sentry grouping and filtering by - Using the exceptionCode as the fingerprint when the error is a customException. For this to work in this PR we are now throwing customExceptions instead of internalServerError deprived of their code. They will still be converted to Internal server errors when sent back as response - Filtering 4xx issues where it was missing (for emailVerification because errors were not handled, for invalid captcha and billing errors because they are httpErrors and not graphqlErrors) --------- Co-authored-by: Félix Malfait <felix@twenty.com>
This commit is contained in:
@ -1,6 +1,7 @@
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export class AuthException extends CustomException {
|
||||
declare code: AuthExceptionCode;
|
||||
constructor(message: string, code: AuthExceptionCode) {
|
||||
super(message, code);
|
||||
}
|
||||
|
||||
@ -6,9 +6,7 @@ import {
|
||||
} from 'src/engine/core-modules/auth/auth.exception';
|
||||
import {
|
||||
AuthenticationError,
|
||||
EmailNotVerifiedError,
|
||||
ForbiddenError,
|
||||
InternalServerError,
|
||||
NotFoundError,
|
||||
UserInputError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
@ -21,18 +19,33 @@ export class AuthGraphqlApiExceptionFilter implements ExceptionFilter {
|
||||
throw new NotFoundError(exception.message);
|
||||
case AuthExceptionCode.INVALID_INPUT:
|
||||
throw new UserInputError(exception.message);
|
||||
case AuthExceptionCode.EMAIL_NOT_VERIFIED:
|
||||
throw new EmailNotVerifiedError(exception.message);
|
||||
case AuthExceptionCode.FORBIDDEN_EXCEPTION:
|
||||
throw new ForbiddenError(exception.message);
|
||||
case AuthExceptionCode.EMAIL_NOT_VERIFIED:
|
||||
throw new ForbiddenError(exception.message, {
|
||||
subCode: AuthExceptionCode.EMAIL_NOT_VERIFIED,
|
||||
});
|
||||
case AuthExceptionCode.UNAUTHENTICATED:
|
||||
case AuthExceptionCode.USER_NOT_FOUND:
|
||||
case AuthExceptionCode.WORKSPACE_NOT_FOUND:
|
||||
throw new AuthenticationError(exception.message);
|
||||
case AuthExceptionCode.INVALID_DATA:
|
||||
case AuthExceptionCode.INTERNAL_SERVER_ERROR:
|
||||
default:
|
||||
throw new InternalServerError(exception.message);
|
||||
case AuthExceptionCode.USER_WORKSPACE_NOT_FOUND:
|
||||
case AuthExceptionCode.INSUFFICIENT_SCOPES:
|
||||
case AuthExceptionCode.OAUTH_ACCESS_DENIED:
|
||||
case AuthExceptionCode.SSO_AUTH_FAILED:
|
||||
case AuthExceptionCode.USE_SSO_AUTH:
|
||||
case AuthExceptionCode.SIGNUP_DISABLED:
|
||||
case AuthExceptionCode.GOOGLE_API_AUTH_DISABLED:
|
||||
case AuthExceptionCode.MICROSOFT_API_AUTH_DISABLED:
|
||||
case AuthExceptionCode.MISSING_ENVIRONMENT_VARIABLE:
|
||||
throw exception;
|
||||
default: {
|
||||
const _exhaustiveCheck: never = exception.code;
|
||||
|
||||
throw exception;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -0,0 +1,29 @@
|
||||
import {
|
||||
EmailVerificationException,
|
||||
EmailVerificationExceptionCode,
|
||||
} from 'src/engine/core-modules/email-verification/email-verification.exception';
|
||||
import {
|
||||
ForbiddenError,
|
||||
UserInputError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
|
||||
export const emailVerificationGraphqlApiExceptionHandler = (
|
||||
error: EmailVerificationException,
|
||||
) => {
|
||||
switch (error.code) {
|
||||
case EmailVerificationExceptionCode.INVALID_TOKEN:
|
||||
case EmailVerificationExceptionCode.INVALID_APP_TOKEN_TYPE:
|
||||
case EmailVerificationExceptionCode.TOKEN_EXPIRED:
|
||||
case EmailVerificationExceptionCode.RATE_LIMIT_EXCEEDED:
|
||||
throw new ForbiddenError(error.message);
|
||||
case EmailVerificationExceptionCode.EMAIL_MISSING:
|
||||
case EmailVerificationExceptionCode.EMAIL_ALREADY_VERIFIED:
|
||||
case EmailVerificationExceptionCode.EMAIL_VERIFICATION_NOT_REQUIRED:
|
||||
throw new UserInputError(error.message);
|
||||
default: {
|
||||
const _exhaustiveCheck: never = error.code;
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
};
|
||||
@ -1,6 +1,7 @@
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export class EmailVerificationException extends CustomException {
|
||||
declare code: EmailVerificationExceptionCode;
|
||||
constructor(message: string, code: EmailVerificationExceptionCode) {
|
||||
super(message, code);
|
||||
}
|
||||
|
||||
@ -5,6 +5,8 @@ import { SOURCE_LOCALE } from 'twenty-shared/translations';
|
||||
import { DomainManagerService } from 'src/engine/core-modules/domain-manager/services/domain-manager.service';
|
||||
import { ResendEmailVerificationTokenInput } from 'src/engine/core-modules/email-verification/dtos/resend-email-verification-token.input';
|
||||
import { ResendEmailVerificationTokenOutput } from 'src/engine/core-modules/email-verification/dtos/resend-email-verification-token.output';
|
||||
import { emailVerificationGraphqlApiExceptionHandler } from 'src/engine/core-modules/email-verification/email-verification-exception-handler.util';
|
||||
import { EmailVerificationException } from 'src/engine/core-modules/email-verification/email-verification.exception';
|
||||
import { EmailVerificationService } from 'src/engine/core-modules/email-verification/services/email-verification.service';
|
||||
import { I18nContext } from 'src/engine/core-modules/i18n/types/i18n-context.type';
|
||||
import { OriginHeader } from 'src/engine/decorators/auth/origin-header.decorator';
|
||||
@ -23,15 +25,22 @@ export class EmailVerificationResolver {
|
||||
@OriginHeader() origin: string,
|
||||
@Context() context: I18nContext,
|
||||
): Promise<ResendEmailVerificationTokenOutput> {
|
||||
const workspace =
|
||||
await this.domainManagerService.getWorkspaceByOriginOrDefaultWorkspace(
|
||||
origin,
|
||||
);
|
||||
try {
|
||||
const workspace =
|
||||
await this.domainManagerService.getWorkspaceByOriginOrDefaultWorkspace(
|
||||
origin,
|
||||
);
|
||||
|
||||
return await this.emailVerificationService.resendEmailVerificationToken(
|
||||
resendEmailVerificationTokenInput.email,
|
||||
workspace,
|
||||
context.req.headers['x-locale'] ?? SOURCE_LOCALE,
|
||||
);
|
||||
return await this.emailVerificationService.resendEmailVerificationToken(
|
||||
resendEmailVerificationTokenInput.email,
|
||||
workspace,
|
||||
context.req.headers['x-locale'] ?? SOURCE_LOCALE,
|
||||
);
|
||||
} catch (error) {
|
||||
if (error instanceof EmailVerificationException) {
|
||||
return emailVerificationGraphqlApiExceptionHandler(error);
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -3,6 +3,7 @@ import * as Sentry from '@sentry/node';
|
||||
import { ExceptionHandlerOptions } from 'src/engine/core-modules/exception-handler/interfaces/exception-handler-options.interface';
|
||||
|
||||
import { ExceptionHandlerDriverInterface } from 'src/engine/core-modules/exception-handler/interfaces';
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export class ExceptionHandlerSentryDriver
|
||||
implements ExceptionHandlerDriverInterface
|
||||
@ -15,8 +16,7 @@ export class ExceptionHandlerSentryDriver
|
||||
|
||||
Sentry.withScope((scope) => {
|
||||
if (options?.operation) {
|
||||
scope.setTag('operation', options.operation.name);
|
||||
scope.setTag('operationName', options.operation.name);
|
||||
scope.setExtra('operation', options.operation.name);
|
||||
}
|
||||
|
||||
if (options?.document) {
|
||||
@ -49,6 +49,11 @@ export class ExceptionHandlerSentryDriver
|
||||
});
|
||||
}
|
||||
|
||||
if (exception instanceof CustomException) {
|
||||
scope.setTag('customExceptionCode', exception.code);
|
||||
scope.setFingerprint([exception.code]);
|
||||
}
|
||||
|
||||
const eventId = Sentry.captureException(exception, {
|
||||
contexts: {
|
||||
GraphQL: {
|
||||
|
||||
@ -24,7 +24,6 @@ export enum ErrorCode {
|
||||
PERSISTED_QUERY_NOT_SUPPORTED = 'PERSISTED_QUERY_NOT_SUPPORTED',
|
||||
BAD_USER_INPUT = 'BAD_USER_INPUT',
|
||||
NOT_FOUND = 'NOT_FOUND',
|
||||
EMAIL_NOT_VERIFIED = 'EMAIL_NOT_VERIFIED',
|
||||
METHOD_NOT_ALLOWED = 'METHOD_NOT_ALLOWED',
|
||||
CONFLICT = 'CONFLICT',
|
||||
TIMEOUT = 'TIMEOUT',
|
||||
@ -107,16 +106,16 @@ export class ValidationError extends BaseGraphQLError {
|
||||
}
|
||||
|
||||
export class AuthenticationError extends BaseGraphQLError {
|
||||
constructor(message: string) {
|
||||
super(message, ErrorCode.UNAUTHENTICATED);
|
||||
constructor(message: string, extensions?: Record<string, any>) {
|
||||
super(message, ErrorCode.UNAUTHENTICATED, extensions);
|
||||
|
||||
Object.defineProperty(this, 'name', { value: 'AuthenticationError' });
|
||||
}
|
||||
}
|
||||
|
||||
export class ForbiddenError extends BaseGraphQLError {
|
||||
constructor(message: string) {
|
||||
super(message, ErrorCode.FORBIDDEN);
|
||||
constructor(message: string, extensions?: Record<string, any>) {
|
||||
super(message, ErrorCode.FORBIDDEN, extensions);
|
||||
|
||||
Object.defineProperty(this, 'name', { value: 'ForbiddenError' });
|
||||
}
|
||||
@ -161,14 +160,6 @@ export class NotFoundError extends BaseGraphQLError {
|
||||
}
|
||||
}
|
||||
|
||||
export class EmailNotVerifiedError extends BaseGraphQLError {
|
||||
constructor(message: string) {
|
||||
super(message, ErrorCode.EMAIL_NOT_VERIFIED);
|
||||
|
||||
Object.defineProperty(this, 'name', { value: 'EmailNotVerifiedError' });
|
||||
}
|
||||
}
|
||||
|
||||
export class MethodNotAllowedError extends BaseGraphQLError {
|
||||
constructor(message: string) {
|
||||
super(message, ErrorCode.METHOD_NOT_ALLOWED);
|
||||
|
||||
@ -1,3 +1,5 @@
|
||||
import { HttpException } from '@nestjs/common';
|
||||
|
||||
import {
|
||||
BaseGraphQLError,
|
||||
ErrorCode,
|
||||
@ -12,7 +14,6 @@ export const graphQLErrorCodesToFilterOut = [
|
||||
ErrorCode.TIMEOUT,
|
||||
ErrorCode.CONFLICT,
|
||||
ErrorCode.BAD_USER_INPUT,
|
||||
ErrorCode.EMAIL_NOT_VERIFIED,
|
||||
];
|
||||
|
||||
export const shouldCaptureException = (exception: Error): boolean => {
|
||||
@ -23,5 +24,13 @@ export const shouldCaptureException = (exception: Error): boolean => {
|
||||
return false;
|
||||
}
|
||||
|
||||
if (
|
||||
exception instanceof HttpException &&
|
||||
exception.getStatus() >= 400 &&
|
||||
exception.getStatus() < 500
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
};
|
||||
|
||||
@ -1,13 +1,12 @@
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export class SearchException extends CustomException {
|
||||
declare code: SearchExceptionCode;
|
||||
constructor(message: string, code: SearchExceptionCode) {
|
||||
super(message, code);
|
||||
}
|
||||
}
|
||||
|
||||
export enum SearchExceptionCode {
|
||||
METADATA_CACHE_VERSION_NOT_FOUND = 'METADATA_CACHE_VERSION_NOT_FOUND',
|
||||
LABEL_IDENTIFIER_FIELD_NOT_FOUND = 'LABEL_IDENTIFIER_FIELD_NOT_FOUND',
|
||||
OBJECT_METADATA_MAP_NOT_FOUND = 'OBJECT_METADATA_MAP_NOT_FOUND',
|
||||
}
|
||||
|
||||
@ -1,7 +1,9 @@
|
||||
import { Catch, ExceptionFilter } from '@nestjs/common';
|
||||
|
||||
import { InternalServerError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
import { SearchException } from 'src/engine/core-modules/search/exceptions/search.exception';
|
||||
import {
|
||||
SearchException,
|
||||
SearchExceptionCode,
|
||||
} from 'src/engine/core-modules/search/exceptions/search.exception';
|
||||
|
||||
@Catch(SearchException)
|
||||
export class SearchApiExceptionFilter implements ExceptionFilter {
|
||||
@ -9,8 +11,13 @@ export class SearchApiExceptionFilter implements ExceptionFilter {
|
||||
|
||||
catch(exception: SearchException) {
|
||||
switch (exception.code) {
|
||||
default:
|
||||
throw new InternalServerError(exception.message);
|
||||
case SearchExceptionCode.LABEL_IDENTIFIER_FIELD_NOT_FOUND:
|
||||
throw exception;
|
||||
default: {
|
||||
const _exhaustiveCheck: never = exception.code;
|
||||
|
||||
throw exception;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -7,16 +7,20 @@ import { ObjectRecordFilter } from 'src/engine/api/graphql/workspace-query-build
|
||||
|
||||
import { SearchArgs } from 'src/engine/core-modules/search/dtos/search-args';
|
||||
import { SearchRecordDTO } from 'src/engine/core-modules/search/dtos/search-record-dto';
|
||||
import {
|
||||
SearchException,
|
||||
SearchExceptionCode,
|
||||
} from 'src/engine/core-modules/search/exceptions/search.exception';
|
||||
import { SearchApiExceptionFilter } from 'src/engine/core-modules/search/filters/search-api-exception.filter';
|
||||
import { SearchService } from 'src/engine/core-modules/search/services/search.service';
|
||||
import { RecordsWithObjectMetadataItem } from 'src/engine/core-modules/search/types/records-with-object-metadata-item';
|
||||
import { formatSearchTerms } from 'src/engine/core-modules/search/utils/format-search-terms';
|
||||
import { Workspace } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
|
||||
import {
|
||||
WorkspaceMetadataCacheException,
|
||||
WorkspaceMetadataCacheExceptionCode,
|
||||
} from 'src/engine/metadata-modules/workspace-metadata-cache/exceptions/workspace-metadata-cache.exception';
|
||||
import {
|
||||
WorkspaceMetadataVersionException,
|
||||
WorkspaceMetadataVersionExceptionCode,
|
||||
} from 'src/engine/metadata-modules/workspace-metadata-version/exceptions/workspace-metadata-version.exception';
|
||||
import { TwentyORMManager } from 'src/engine/twenty-orm/twenty-orm.manager';
|
||||
import { WorkspaceCacheStorageService } from 'src/engine/workspace-cache-storage/workspace-cache-storage.service';
|
||||
|
||||
@ -47,9 +51,9 @@ export class SearchResolver {
|
||||
await this.workspaceCacheStorageService.getMetadataVersion(workspace.id);
|
||||
|
||||
if (currentCacheVersion === undefined) {
|
||||
throw new SearchException(
|
||||
'Metadata cache version not found',
|
||||
SearchExceptionCode.METADATA_CACHE_VERSION_NOT_FOUND,
|
||||
throw new WorkspaceMetadataVersionException(
|
||||
`Metadata version not found for workspace ${workspace.id}`,
|
||||
WorkspaceMetadataVersionExceptionCode.METADATA_VERSION_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
|
||||
@ -60,9 +64,9 @@ export class SearchResolver {
|
||||
);
|
||||
|
||||
if (!objectMetadataMaps) {
|
||||
throw new SearchException(
|
||||
throw new WorkspaceMetadataCacheException(
|
||||
`Object metadata map not found for workspace ${workspace.id} and metadata version ${currentCacheVersion}`,
|
||||
SearchExceptionCode.OBJECT_METADATA_MAP_NOT_FOUND,
|
||||
WorkspaceMetadataCacheExceptionCode.OBJECT_METADATA_MAP_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
@ -2,7 +2,6 @@ import { Catch, ExceptionFilter } from '@nestjs/common';
|
||||
|
||||
import {
|
||||
ForbiddenError,
|
||||
InternalServerError,
|
||||
NotFoundError,
|
||||
UserInputError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
@ -25,8 +24,13 @@ export class ConfigVariableGraphqlApiExceptionFilter
|
||||
case ConfigVariableExceptionCode.VALIDATION_FAILED:
|
||||
throw new UserInputError(exception.message);
|
||||
case ConfigVariableExceptionCode.INTERNAL_ERROR:
|
||||
default:
|
||||
throw new InternalServerError(exception.message);
|
||||
case ConfigVariableExceptionCode.UNSUPPORTED_CONFIG_TYPE:
|
||||
throw exception;
|
||||
default: {
|
||||
const _exhaustiveCheck: never = exception.code;
|
||||
|
||||
throw exception;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export class ConfigVariableException extends CustomException {
|
||||
declare code: ConfigVariableExceptionCode;
|
||||
constructor(message: string, code: ConfigVariableExceptionCode) {
|
||||
super(message, code);
|
||||
}
|
||||
@ -12,6 +13,5 @@ export enum ConfigVariableExceptionCode {
|
||||
VARIABLE_NOT_FOUND = 'VARIABLE_NOT_FOUND',
|
||||
VALIDATION_FAILED = 'VALIDATION_FAILED',
|
||||
UNSUPPORTED_CONFIG_TYPE = 'UNSUPPORTED_CONFIG_TYPE',
|
||||
METADATA_NOT_FOUND = 'METADATA_NOT_FOUND',
|
||||
INTERNAL_ERROR = 'INTERNAL_ERROR',
|
||||
}
|
||||
|
||||
@ -1,7 +1,6 @@
|
||||
import { Catch, ExceptionFilter } from '@nestjs/common';
|
||||
|
||||
import {
|
||||
InternalServerError,
|
||||
NotFoundError,
|
||||
UserInputError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
@ -25,8 +24,13 @@ export class WorkflowTriggerGraphqlApiExceptionFilter
|
||||
throw new UserInputError(exception.message);
|
||||
case WorkflowTriggerExceptionCode.NOT_FOUND:
|
||||
throw new NotFoundError(exception.message);
|
||||
default:
|
||||
throw new InternalServerError(exception.message);
|
||||
case WorkflowTriggerExceptionCode.INTERNAL_ERROR:
|
||||
throw exception;
|
||||
default: {
|
||||
const _exhaustiveCheck: never = exception.code;
|
||||
|
||||
throw exception;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@ -1,6 +1,5 @@
|
||||
import {
|
||||
ConflictError,
|
||||
InternalServerError,
|
||||
NotFoundError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
import { workspaceGraphqlApiExceptionHandler } from 'src/engine/core-modules/workspace/utils/workspace-graphql-api-exception-handler.util';
|
||||
@ -8,6 +7,7 @@ import {
|
||||
WorkspaceException,
|
||||
WorkspaceExceptionCode,
|
||||
} from 'src/engine/core-modules/workspace/workspace.exception';
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
describe('workspaceGraphqlApiExceptionHandler', () => {
|
||||
it('should throw NotFoundError when WorkspaceExceptionCode is SUBDOMAIN_NOT_FOUND', () => {
|
||||
@ -48,7 +48,7 @@ describe('workspaceGraphqlApiExceptionHandler', () => {
|
||||
const error = new WorkspaceException('Unknown error', 'UNKNOWN_CODE');
|
||||
|
||||
expect(() => workspaceGraphqlApiExceptionHandler(error)).toThrow(
|
||||
InternalServerError,
|
||||
CustomException,
|
||||
);
|
||||
});
|
||||
|
||||
|
||||
@ -1,7 +1,6 @@
|
||||
import {
|
||||
ConflictError,
|
||||
ForbiddenError,
|
||||
InternalServerError,
|
||||
NotFoundError,
|
||||
} from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
import {
|
||||
@ -15,12 +14,17 @@ export const workspaceGraphqlApiExceptionHandler = (error: Error) => {
|
||||
case WorkspaceExceptionCode.SUBDOMAIN_NOT_FOUND:
|
||||
case WorkspaceExceptionCode.WORKSPACE_NOT_FOUND:
|
||||
throw new NotFoundError(error.message);
|
||||
case WorkspaceExceptionCode.DOMAIN_ALREADY_TAKEN:
|
||||
case WorkspaceExceptionCode.SUBDOMAIN_ALREADY_TAKEN:
|
||||
throw new ConflictError(error.message);
|
||||
case WorkspaceExceptionCode.ENVIRONMENT_VAR_NOT_ENABLED:
|
||||
case WorkspaceExceptionCode.WORKSPACE_CUSTOM_DOMAIN_DISABLED:
|
||||
throw new ForbiddenError(error.message);
|
||||
default:
|
||||
throw new InternalServerError(error.message);
|
||||
default: {
|
||||
const _exhaustiveCheck: never = error.code;
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@ -1,6 +1,7 @@
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export class WorkspaceException extends CustomException {
|
||||
declare code: WorkspaceExceptionCode;
|
||||
constructor(message: string, code: WorkspaceExceptionCode) {
|
||||
super(message, code);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user