Skip to content

Commit

Permalink
Better errors handling (twentyhq#8835)
Browse files Browse the repository at this point in the history
- [ ] Catch this specific `500` error
- [ ] Make sure catched `500` errors are sent to sentry for the Cloud
version
- [ ] Hide the option to sync email with google in this situation where
the according env var is missing
- [x] Add Worskpace information to all catched errors for better
debugging

fix twentyhq#8607
  • Loading branch information
guillim authored and mdrazak2001 committed Dec 4, 2024
1 parent dd13a53 commit 5f90db9
Show file tree
Hide file tree
Showing 28 changed files with 120 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,13 @@ export class GraphQLConfigService
email: user.email,
firstName: user.firstName,
lastName: user.lastName,
workspaceId: workspace?.id,
workspaceDisplayName: workspace?.displayName,
}
: undefined,
workspace
? {
id: workspace.id,
displayName: workspace.displayName,
activationStatus: workspace.activationStatus,
}
: undefined,
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { ExecutionContext, Injectable } from '@nestjs/common';
import { AuthGuard } from '@nestjs/passport';

import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import { GoogleAPIsOauthExchangeCodeForTokenStrategy } from 'src/engine/core-modules/auth/strategies/google-apis-oauth-exchange-code-for-token.auth.strategy';
import { TransientTokenService } from 'src/engine/core-modules/auth/token/services/transient-token.service';
import { setRequestExtraParams } from 'src/engine/core-modules/auth/utils/google-apis-set-request-extra-params.util';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum';
import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service';
Expand Down Expand Up @@ -41,9 +41,9 @@ export class GoogleAPIsOauthExchangeCodeForTokenGuard extends AuthGuard(
!this.environmentService.get('MESSAGING_PROVIDER_GMAIL_ENABLED') &&
!this.environmentService.get('CALENDAR_PROVIDER_GOOGLE_ENABLED')
) {
throw new AuthException(
throw new EnvironmentException(
'Google apis auth is not enabled',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
import { ExecutionContext, Injectable } from '@nestjs/common';
import { AuthGuard } from '@nestjs/passport';

import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import { GoogleAPIsOauthRequestCodeStrategy } from 'src/engine/core-modules/auth/strategies/google-apis-oauth-request-code.auth.strategy';
import { TransientTokenService } from 'src/engine/core-modules/auth/token/services/transient-token.service';
import { setRequestExtraParams } from 'src/engine/core-modules/auth/utils/google-apis-set-request-extra-params.util';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { FeatureFlagKey } from 'src/engine/core-modules/feature-flag/enums/feature-flag-key.enum';
import { FeatureFlagService } from 'src/engine/core-modules/feature-flag/services/feature-flag.service';
Expand Down Expand Up @@ -41,9 +41,9 @@ export class GoogleAPIsOauthRequestCodeGuard extends AuthGuard('google-apis') {
!this.environmentService.get('MESSAGING_PROVIDER_GMAIL_ENABLED') &&
!this.environmentService.get('CALENDAR_PROVIDER_GOOGLE_ENABLED')
) {
throw new AuthException(
throw new EnvironmentException(
'Google apis auth is not enabled',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { CanActivate, Injectable } from '@nestjs/common';

import { Observable } from 'rxjs';

import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import { GoogleStrategy } from 'src/engine/core-modules/auth/strategies/google.auth.strategy';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';

@Injectable()
Expand All @@ -15,9 +15,9 @@ export class GoogleProviderEnabledGuard implements CanActivate {

canActivate(): boolean | Promise<boolean> | Observable<boolean> {
if (!this.environmentService.get('AUTH_GOOGLE_ENABLED')) {
throw new AuthException(
throw new EnvironmentException(
'Google auth is not enabled',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ import { CanActivate, Injectable } from '@nestjs/common';

import { Observable } from 'rxjs';

import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import { MicrosoftStrategy } from 'src/engine/core-modules/auth/strategies/microsoft.auth.strategy';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';

@Injectable()
Expand All @@ -15,9 +15,9 @@ export class MicrosoftProviderEnabledGuard implements CanActivate {

canActivate(): boolean | Promise<boolean> | Observable<boolean> {
if (!this.environmentService.get('AUTH_MICROSOFT_ENABLED')) {
throw new AuthException(
throw new EnvironmentException(
'Microsoft auth is not enabled',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ import { CanActivate, Injectable } from '@nestjs/common';
import { Observable } from 'rxjs';

import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';

@Injectable()
Expand All @@ -16,9 +16,9 @@ export class SSOProviderEnabledGuard implements CanActivate {

canActivate(): boolean | Promise<boolean> | Observable<boolean> {
if (!this.environmentService.get('ENTERPRISE_KEY')) {
throw new AuthException(
throw new EnvironmentException(
'Enterprise key must be defined to use SSO',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,14 @@ import { HttpService } from '@nestjs/axios';
import { Injectable } from '@nestjs/common';
import { InjectRepository } from '@nestjs/typeorm';

import { isDefined } from 'class-validator';
import FileType from 'file-type';
import { Repository } from 'typeorm';
import { v4 } from 'uuid';
import { isDefined } from 'class-validator';

import { FileFolder } from 'src/engine/core-modules/file/interfaces/file-folder.interface';

import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';
import {
AuthException,
AuthExceptionCode,
Expand All @@ -18,6 +19,11 @@ import {
compareHash,
hashPassword,
} from 'src/engine/core-modules/auth/auth.util';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { FileUploadService } from 'src/engine/core-modules/file/file-upload/services/file-upload.service';
import { OnboardingService } from 'src/engine/core-modules/onboarding/onboarding.service';
import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service';
Expand All @@ -26,9 +32,7 @@ import {
Workspace,
WorkspaceActivationStatus,
} from 'src/engine/core-modules/workspace/workspace.entity';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { getImageBufferFromUrl } from 'src/utils/image';
import { AppToken } from 'src/engine/core-modules/app-token/app-token.entity';

export type SignInUpServiceInput = {
email: string;
Expand Down Expand Up @@ -298,9 +302,9 @@ export class SignInUpService {
picture: SignInUpServiceInput['picture'];
}) {
if (this.environmentService.get('IS_SIGN_UP_DISABLED')) {
throw new AuthException(
throw new EnvironmentException(
'Sign up is disabled',
AuthExceptionCode.FORBIDDEN_EXCEPTION,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ import {
AuthContext,
JwtPayload,
} from 'src/engine/core-modules/auth/types/auth-context.type';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
import { User } from 'src/engine/core-modules/user/user.entity';
Expand All @@ -42,9 +46,9 @@ export class AccessTokenService {
const expiresIn = this.environmentService.get('ACCESS_TOKEN_EXPIRES_IN');

if (!expiresIn) {
throw new AuthException(
throw new EnvironmentException(
'Expiration time for access token is not set',
AuthExceptionCode.INTERNAL_SERVER_ERROR,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Test, TestingModule } from '@nestjs/testing';

import { AuthException } from 'src/engine/core-modules/auth/auth.exception';
import { EnvironmentException } from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';

Expand Down Expand Up @@ -76,7 +76,7 @@ describe('LoginTokenService', () => {

await expect(
service.generateLoginToken('[email protected]'),
).rejects.toThrow(AuthException);
).rejects.toThrow(EnvironmentException);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { Injectable } from '@nestjs/common';
import { addMilliseconds } from 'date-fns';
import ms from 'ms';

import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import { AuthToken } from 'src/engine/core-modules/auth/dto/token.entity';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';

Expand All @@ -23,9 +23,9 @@ export class LoginTokenService {
const expiresIn = this.environmentService.get('LOGIN_TOKEN_EXPIRES_IN');

if (!expiresIn) {
throw new AuthException(
throw new EnvironmentException(
'Expiration time for access token is not set',
AuthExceptionCode.INTERNAL_SERVER_ERROR,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Test, TestingModule } from '@nestjs/testing';

import { AuthException } from 'src/engine/core-modules/auth/auth.exception';
import { EnvironmentException } from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';

Expand Down Expand Up @@ -88,7 +88,7 @@ describe('TransientTokenService', () => {

await expect(
service.generateTransientToken('member-id', 'user-id', 'workspace-id'),
).rejects.toThrow(AuthException);
).rejects.toThrow(EnvironmentException);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@ import { Injectable } from '@nestjs/common';
import { addMilliseconds } from 'date-fns';
import ms from 'ms';

import {
AuthException,
AuthExceptionCode,
} from 'src/engine/core-modules/auth/auth.exception';
import { AuthToken } from 'src/engine/core-modules/auth/dto/token.entity';
import {
EnvironmentException,
EnvironmentExceptionCode,
} from 'src/engine/core-modules/environment/environment.exception';
import { EnvironmentService } from 'src/engine/core-modules/environment/environment.service';
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';

Expand All @@ -32,9 +32,9 @@ export class TransientTokenService {
);

if (!expiresIn) {
throw new AuthException(
throw new EnvironmentException(
'Expiration time for access token is not set',
AuthExceptionCode.INTERNAL_SERVER_ERROR,
EnvironmentExceptionCode.ENVIRONMENT_VARIABLES_NOT_FOUND,
);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { CustomException } from 'src/utils/custom-exception';

export class EnvironmentException extends CustomException {
code: EnvironmentExceptionCode;
constructor(message: string, code: EnvironmentExceptionCode) {
super(message, code);
}
}

export enum EnvironmentExceptionCode {
ENVIRONMENT_VARIABLES_NOT_FOUND = 'ENVIRONMENT_VARIABLES_NOT_FOUND',
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/* eslint-disable no-console */
import { ExceptionHandlerUser } from 'src/engine/core-modules/exception-handler/interfaces/exception-handler-user.interface';
import { ExceptionHandlerOptions } from 'src/engine/core-modules/exception-handler/interfaces/exception-handler-options.interface';

import { ExceptionHandlerDriverInterface } from 'src/engine/core-modules/exception-handler/interfaces';
Expand All @@ -18,11 +17,4 @@ export class ExceptionHandlerConsoleDriver

return [];
}

captureMessage(message: string, user?: ExceptionHandlerUser): void {
console.group('Message Captured');
console.info(user);
console.info(message);
console.groupEnd();
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as Sentry from '@sentry/node';

import { ExceptionHandlerOptions } from 'src/engine/core-modules/exception-handler/interfaces/exception-handler-options.interface';
import { ExceptionHandlerUser } from 'src/engine/core-modules/exception-handler/interfaces/exception-handler-user.interface';

import { ExceptionHandlerDriverInterface } from 'src/engine/core-modules/exception-handler/interfaces';

Expand All @@ -24,14 +23,16 @@ export class ExceptionHandlerSentryDriver
scope.setExtra('document', options.document);
}

if (options?.workspace) {
scope.setExtra('workspace', options.workspace);
}

if (options?.user) {
scope.setUser({
id: options.user.id,
email: options.user.email,
firstName: options.user.firstName,
lastName: options.user.lastName,
workspaceId: options.user.workspaceId,
workspaceDisplayName: options.user.workspaceDisplayName,
});
}

Expand Down Expand Up @@ -69,21 +70,4 @@ export class ExceptionHandlerSentryDriver

return eventIds;
}

captureMessage(message: string, user?: ExceptionHandlerUser) {
Sentry.captureMessage(message, (scope) => {
if (user) {
scope.setUser({
id: user.id,
email: user.email,
firstName: user.firstName,
lastName: user.lastName,
workspaceId: user.workspaceId,
workspaceDisplayName: user.workspaceDisplayName,
});
}

return scope;
});
}
}
Loading

0 comments on commit 5f90db9

Please sign in to comment.