From d83a2ee02e47048160bc46d10aeef5a37e480f02 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C3=ABl=20Minelli?= <git@minelli.me> Date: Mon, 25 Mar 2024 16:44:41 +0100 Subject: [PATCH] Errors => Update error handling and sending --- ExpressAPI/prisma/seed.ts | 2 +- ExpressAPI/src/helpers/DojoValidators.ts | 4 +- ExpressAPI/src/helpers/GlobalHelper.ts | 17 +++-- ExpressAPI/src/managers/AssignmentManager.ts | 1 - ExpressAPI/src/managers/GitlabManager.ts | 42 +++++----- .../ClientVersionCheckerMiddleware.ts | 16 ++-- .../src/middlewares/SecurityMiddleware.ts | 2 +- ExpressAPI/src/routes/AssignmentRoutes.ts | 76 ++++++++++--------- ExpressAPI/src/routes/ExerciseRoutes.ts | 23 +++--- 9 files changed, 95 insertions(+), 88 deletions(-) diff --git a/ExpressAPI/prisma/seed.ts b/ExpressAPI/prisma/seed.ts index fc8dce5..74d814d 100644 --- a/ExpressAPI/prisma/seed.ts +++ b/ExpressAPI/prisma/seed.ts @@ -17,7 +17,7 @@ async function main() { main().then(async () => { await db.$disconnect(); }).catch(async e => { - logger.error(e); + logger.error(JSON.stringify(e)); await db.$disconnect(); process.exit(1); }); diff --git a/ExpressAPI/src/helpers/DojoValidators.ts b/ExpressAPI/src/helpers/DojoValidators.ts index 442f160..26f60f4 100644 --- a/ExpressAPI/src/helpers/DojoValidators.ts +++ b/ExpressAPI/src/helpers/DojoValidators.ts @@ -34,7 +34,7 @@ class DojoValidators { try { return value === 'null' || value === 'undefined' || value === '' ? null : value; } catch ( error ) { - logger.error(`null sanitizer error: ${ error }`); + logger.error(`null sanitizer error: ${ JSON.stringify(error) }`); return value; } @@ -84,7 +84,7 @@ class DojoValidators { return Config.assignment.default.template; } } catch ( error ) { - logger.error(`Template url sanitizer error: ${ error }`); + logger.error(`Template url sanitizer error: ${ JSON.stringify(error) }`); return value; } diff --git a/ExpressAPI/src/helpers/GlobalHelper.ts b/ExpressAPI/src/helpers/GlobalHelper.ts index 8d4068b..00f0933 100644 --- a/ExpressAPI/src/helpers/GlobalHelper.ts +++ b/ExpressAPI/src/helpers/GlobalHelper.ts @@ -1,16 +1,16 @@ -import express from 'express'; -import logger from '../shared/logging/WinstonLogger'; -import GitlabManager from '../managers/GitlabManager'; -import { AxiosError } from 'axios'; +import express from 'express'; +import logger from '../shared/logging/WinstonLogger'; +import GitlabManager from '../managers/GitlabManager'; +import DojoStatusCode from '../shared/types/Dojo/DojoStatusCode'; import { StatusCodes } from 'http-status-codes'; -import DojoStatusCode from '../shared/types/Dojo/DojoStatusCode'; +import { GitbeakerRequestError } from '@gitbeaker/requester-utils'; import * as Gitlab from '@gitbeaker/rest'; class GlobalHelper { async repositoryCreationError(message: string, error: unknown, req: express.Request, res: express.Response, gitlabError: DojoStatusCode, internalError: DojoStatusCode, repositoryToRemove?: Gitlab.ProjectSchema): Promise<void> { logger.error(message); - logger.error(error); + logger.error(JSON.stringify(error)); try { if ( repositoryToRemove ) { @@ -18,11 +18,12 @@ class GlobalHelper { } } catch ( deleteError ) { logger.error('Repository deletion error'); - logger.error(deleteError); + logger.error(JSON.stringify(deleteError)); } - if ( error instanceof AxiosError ) { + if ( error instanceof GitbeakerRequestError ) { return req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, {}, `Unknown gitlab error: ${ message }`, gitlabError); + return; } return req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, {}, `Unknown error: ${ message }`, internalError); diff --git a/ExpressAPI/src/managers/AssignmentManager.ts b/ExpressAPI/src/managers/AssignmentManager.ts index ad51e6b..9053b8e 100644 --- a/ExpressAPI/src/managers/AssignmentManager.ts +++ b/ExpressAPI/src/managers/AssignmentManager.ts @@ -45,7 +45,6 @@ class AssignmentManager { } else { return this.getByName(nameOrUrl, include); } - } } diff --git a/ExpressAPI/src/managers/GitlabManager.ts b/ExpressAPI/src/managers/GitlabManager.ts index 7f4ff71..f1d0e7b 100644 --- a/ExpressAPI/src/managers/GitlabManager.ts +++ b/ExpressAPI/src/managers/GitlabManager.ts @@ -15,24 +15,24 @@ class GitlabManager extends SharedGitlabManager { super(Config.gitlab.account.token); } - public async getUserProfile(token: string): Promise<ExpandedUserSchema | undefined> { + getUserProfile(token: string): Promise<ExpandedUserSchema> | undefined { try { const profileApi = new Gitlab({ host : SharedConfig.gitlab.URL, oauthToken: token }); - return await profileApi.Users.showCurrentUser(); + return profileApi.Users.showCurrentUser(); } catch ( e ) { return undefined; } } - async getRepositoryMembers(idOrNamespace: string): Promise<Array<MemberSchema>> { + getRepositoryMembers(idOrNamespace: string): Promise<Array<MemberSchema>> { return this.api.ProjectMembers.all(idOrNamespace, { includeInherited: true }); } - async getRepositoryReleases(repoId: number): Promise<Array<ReleaseSchema>> { + getRepositoryReleases(repoId: number): Promise<Array<ReleaseSchema>> { return this.api.ProjectReleases.all(repoId); } @@ -46,12 +46,12 @@ class GitlabManager extends SharedGitlabManager { return commits.length > 0 ? commits[0] : undefined; } catch ( e ) { - logger.error(e); + logger.error(JSON.stringify(e)); return undefined; } } - async createRepository(name: string, description: string, visibility: 'public' | 'internal' | 'private', initializeWithReadme: boolean, namespace: number, sharedRunnersEnabled: boolean, wikiEnabled: boolean, import_url: string): Promise<ProjectSchema> { + createRepository(name: string, description: string, visibility: 'public' | 'internal' | 'private', initializeWithReadme: boolean, namespace: number, sharedRunnersEnabled: boolean, wikiEnabled: boolean, import_url: string): Promise<ProjectSchema> { return this.api.Projects.create({ name : name, description : description, @@ -64,11 +64,11 @@ class GitlabManager extends SharedGitlabManager { }); } - async deleteRepository(repoId: number): Promise<void> { + deleteRepository(repoId: number): Promise<void> { return this.api.Projects.remove(repoId); } - async forkRepository(forkId: number, name: string, path: string, description: string, visibility: 'public' | 'internal' | 'private', namespace: number): Promise<ProjectSchema> { + forkRepository(forkId: number, name: string, path: string, description: string, visibility: 'public' | 'internal' | 'private', namespace: number): Promise<ProjectSchema> { return this.api.Projects.fork(forkId, { name : name, path : path, @@ -78,19 +78,19 @@ class GitlabManager extends SharedGitlabManager { }); } - async editRepository(repoId: number, newAttributes: EditProjectOptions): Promise<ProjectSchema> { + editRepository(repoId: number, newAttributes: EditProjectOptions): Promise<ProjectSchema> { return this.api.Projects.edit(repoId, newAttributes); } - async changeRepositoryVisibility(repoId: number, visibility: GitlabVisibility): Promise<ProjectSchema> { + changeRepositoryVisibility(repoId: number, visibility: GitlabVisibility): Promise<ProjectSchema> { return this.editRepository(repoId, { visibility: visibility }); } - async addRepositoryMember(repoId: number, userId: number, accessLevel: Exclude<AccessLevel, AccessLevel.ADMIN>): Promise<MemberSchema> { + addRepositoryMember(repoId: number, userId: number, accessLevel: Exclude<AccessLevel, AccessLevel.ADMIN>): Promise<MemberSchema> { return this.api.ProjectMembers.add(repoId, userId, accessLevel); } - async addRepositoryVariable(repoId: number, key: string, value: string, isProtected: boolean, isMasked: boolean): Promise<ProjectVariableSchema> { + addRepositoryVariable(repoId: number, key: string, value: string, isProtected: boolean, isMasked: boolean): Promise<ProjectVariableSchema> { return this.api.ProjectVariables.create(repoId, key, value, { variableType: 'env_var', protected : isProtected, @@ -98,7 +98,7 @@ class GitlabManager extends SharedGitlabManager { }); } - async addRepositoryBadge(repoId: number, linkUrl: string, imageUrl: string, name: string): Promise<ProjectBadgeSchema> { + addRepositoryBadge(repoId: number, linkUrl: string, imageUrl: string, name: string): Promise<ProjectBadgeSchema> { return this.api.ProjectBadges.add(repoId, linkUrl, imageUrl, { name: name }); @@ -143,7 +143,7 @@ class GitlabManager extends SharedGitlabManager { } } - async protectBranch(repoId: number, branchName: string, allowForcePush: boolean, allowedToMerge: ProtectedBranchAccessLevel, allowedToPush: ProtectedBranchAccessLevel, allowedToUnprotect: ProtectedBranchAccessLevel): Promise<ProtectedBranchSchema> { + protectBranch(repoId: number, branchName: string, allowForcePush: boolean, allowedToMerge: ProtectedBranchAccessLevel, allowedToPush: ProtectedBranchAccessLevel, allowedToUnprotect: ProtectedBranchAccessLevel): Promise<ProtectedBranchSchema> { return this.api.ProtectedBranches.protect(repoId, branchName, { allowForcePush : allowForcePush, mergeAccessLevel : allowedToMerge, @@ -152,18 +152,18 @@ class GitlabManager extends SharedGitlabManager { }); } - async getRepositoryTree(repoId: number, recursive: boolean = true, branch: string = 'main'): Promise<Array<RepositoryTreeSchema>> { + getRepositoryTree(repoId: number, recursive: boolean = true, branch: string = 'main'): Promise<Array<RepositoryTreeSchema>> { return this.api.Repositories.allRepositoryTrees(repoId, { recursive: recursive, ref : branch }); } - async getFile(repoId: number, filePath: string, branch: string = 'main'): Promise<RepositoryFileExpandedSchema> { + getFile(repoId: number, filePath: string, branch: string = 'main'): Promise<RepositoryFileExpandedSchema> { return this.api.RepositoryFiles.show(repoId, filePath, branch); } - private async createUpdateFile(create: boolean, repoId: number, filePath: string, fileBase64: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<RepositoryFileSchema> { + private createUpdateFile(create: boolean, repoId: number, filePath: string, fileBase64: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<RepositoryFileSchema> { const gitFunction = create ? this.api.RepositoryFiles.create : this.api.RepositoryFiles.edit; return gitFunction(repoId, filePath, branch, fileBase64, commitMessage, { @@ -173,16 +173,16 @@ class GitlabManager extends SharedGitlabManager { }); } - async createFile(repoId: number, filePath: string, fileBase64: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<RepositoryFileSchema> { + createFile(repoId: number, filePath: string, fileBase64: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<RepositoryFileSchema> { return this.createUpdateFile(true, repoId, filePath, fileBase64, commitMessage, branch, authorName, authorMail); } - async updateFile(repoId: number, filePath: string, fileBase64: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<RepositoryFileSchema> { + updateFile(repoId: number, filePath: string, fileBase64: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<RepositoryFileSchema> { return this.createUpdateFile(false, repoId, filePath, fileBase64, commitMessage, branch, authorName, authorMail); } - async deleteFile(repoId: number, filePath: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<void> { - await this.api.RepositoryFiles.remove(repoId, filePath, branch, commitMessage, { + deleteFile(repoId: number, filePath: string, commitMessage: string, branch: string = 'main', authorName: string = 'Dojo', authorMail: string | undefined = undefined): Promise<void> { + return this.api.RepositoryFiles.remove(repoId, filePath, branch, commitMessage, { authorName : authorName, authorEmail: authorMail }); diff --git a/ExpressAPI/src/middlewares/ClientVersionCheckerMiddleware.ts b/ExpressAPI/src/middlewares/ClientVersionCheckerMiddleware.ts index 952fd95..f58b2b7 100644 --- a/ExpressAPI/src/middlewares/ClientVersionCheckerMiddleware.ts +++ b/ExpressAPI/src/middlewares/ClientVersionCheckerMiddleware.ts @@ -1,9 +1,9 @@ -import express from 'express'; -import Config from '../config/Config'; -import semver from 'semver/preload'; -import Session from '../controllers/Session'; -import { HttpStatusCode } from 'axios'; -import DojoStatusCode from '../shared/types/Dojo/DojoStatusCode'; +import express from 'express'; +import Config from '../config/Config'; +import semver from 'semver/preload'; +import Session from '../controllers/Session'; +import DojoStatusCode from '../shared/types/Dojo/DojoStatusCode'; +import { StatusCodes } from 'http-status-codes'; class ClientVersionCheckerMiddleware { @@ -19,13 +19,13 @@ class ClientVersionCheckerMiddleware { next(); return; } else { - new Session().sendResponse(res, HttpStatusCode.MethodNotAllowed, {}, `Client version ${ requestClientVersion } is not supported. Please update your client.`, DojoStatusCode.CLIENT_VERSION_NOT_SUPPORTED); + new Session().sendResponse(res, StatusCodes.METHOD_NOT_ALLOWED, {}, `Client version ${ requestClientVersion } is not supported. Please update your client.`, DojoStatusCode.CLIENT_VERSION_NOT_SUPPORTED); return; } } } - new Session().sendResponse(res, HttpStatusCode.MethodNotAllowed, {}, `Unsupported client.`, DojoStatusCode.CLIENT_NOT_SUPPORTED); + new Session().sendResponse(res, StatusCodes.METHOD_NOT_ALLOWED, {}, `Unsupported client.`, DojoStatusCode.CLIENT_NOT_SUPPORTED); } else { next(); } diff --git a/ExpressAPI/src/middlewares/SecurityMiddleware.ts b/ExpressAPI/src/middlewares/SecurityMiddleware.ts index b08aed0..a3a5059 100644 --- a/ExpressAPI/src/middlewares/SecurityMiddleware.ts +++ b/ExpressAPI/src/middlewares/SecurityMiddleware.ts @@ -25,7 +25,7 @@ class SecurityMiddleware { return false; } } catch ( e ) { - logger.error('Security check failed !!! => ' + e); + logger.error('Security check failed !!! => ' + JSON.stringify(e)); return false; } } diff --git a/ExpressAPI/src/routes/AssignmentRoutes.ts b/ExpressAPI/src/routes/AssignmentRoutes.ts index a9ab260..796cc02 100644 --- a/ExpressAPI/src/routes/AssignmentRoutes.ts +++ b/ExpressAPI/src/routes/AssignmentRoutes.ts @@ -1,27 +1,27 @@ -import { Express } from 'express-serve-static-core'; -import express from 'express'; -import * as ExpressValidator from 'express-validator'; -import { StatusCodes } from 'http-status-codes'; -import RoutesManager from '../express/RoutesManager'; -import ParamsValidatorMiddleware from '../middlewares/ParamsValidatorMiddleware'; -import SecurityMiddleware from '../middlewares/SecurityMiddleware'; -import SecurityCheckType from '../types/SecurityCheckType'; -import GitlabManager from '../managers/GitlabManager'; -import Config from '../config/Config'; -import { AxiosError, HttpStatusCode } from 'axios'; -import logger from '../shared/logging/WinstonLogger'; -import DojoValidators from '../helpers/DojoValidators'; -import { Prisma } from '@prisma/client'; -import db from '../helpers/DatabaseHelper'; -import { Assignment } from '../types/DatabaseTypes'; -import AssignmentManager from '../managers/AssignmentManager'; -import fs from 'fs'; -import path from 'path'; -import SharedAssignmentHelper from '../shared/helpers/Dojo/SharedAssignmentHelper'; -import GlobalHelper from '../helpers/GlobalHelper'; -import DojoStatusCode from '../shared/types/Dojo/DojoStatusCode'; -import DojoModelsHelper from '../helpers/DojoModelsHelper'; -import * as Gitlab from '@gitbeaker/rest'; +import { Express } from 'express-serve-static-core'; +import express from 'express'; +import * as ExpressValidator from 'express-validator'; +import { StatusCodes } from 'http-status-codes'; +import RoutesManager from '../express/RoutesManager'; +import ParamsValidatorMiddleware from '../middlewares/ParamsValidatorMiddleware'; +import SecurityMiddleware from '../middlewares/SecurityMiddleware'; +import SecurityCheckType from '../types/SecurityCheckType'; +import GitlabManager from '../managers/GitlabManager'; +import Config from '../config/Config'; +import logger from '../shared/logging/WinstonLogger'; +import DojoValidators from '../helpers/DojoValidators'; +import { Prisma } from '@prisma/client'; +import db from '../helpers/DatabaseHelper'; +import { Assignment } from '../types/DatabaseTypes'; +import AssignmentManager from '../managers/AssignmentManager'; +import fs from 'fs'; +import path from 'path'; +import SharedAssignmentHelper from '../shared/helpers/Dojo/SharedAssignmentHelper'; +import GlobalHelper from '../helpers/GlobalHelper'; +import DojoStatusCode from '../shared/types/Dojo/DojoStatusCode'; +import DojoModelsHelper from '../helpers/DojoModelsHelper'; +import * as Gitlab from '@gitbeaker/rest'; +import { GitbeakerRequestError } from '@gitbeaker/requester-utils'; class AssignmentRoutes implements RoutesManager { @@ -91,17 +91,20 @@ class AssignmentRoutes implements RoutesManager { repository = await GitlabManager.createRepository(params.name, Config.assignment.default.description.replace('{{ASSIGNMENT_NAME}}', params.name), Config.assignment.default.visibility, Config.assignment.default.initReadme, Config.gitlab.group.assignments, Config.assignment.default.sharedRunnersEnabled, Config.assignment.default.wikiEnabled, params.template); } catch ( error ) { logger.error('Repo creation error'); - logger.error(error); - - if ( error instanceof AxiosError ) { - if ( error.response?.data.message.name && error.response.data.message.name === 'has already been taken' ) { - return req.session.sendResponse(res, StatusCodes.CONFLICT, {}, `Repository name has already been take`, DojoStatusCode.ASSIGNMENT_NAME_CONFLICT); + logger.error(JSON.stringify(error)); + + if ( error instanceof GitbeakerRequestError ) { + if ( error.cause?.description ) { + const description = error.cause.description as unknown; + if ( description instanceof Object && 'name' in description && description.name instanceof Array && description.name.length > 0 && description.name[0] === 'has already been taken' ) { + return req.session.sendResponse(res, StatusCodes.CONFLICT, {}, `Repository name has already been taken`, DojoStatusCode.ASSIGNMENT_NAME_CONFLICT); + } } - return req.session.sendResponse(res, error.response?.status ?? HttpStatusCode.InternalServerError); + return req.session.sendResponse(res, error.cause?.response.status ?? StatusCodes.INTERNAL_SERVER_ERROR); } - return req.session.sendResponse(res, HttpStatusCode.InternalServerError); + return req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR); } await new Promise(resolve => setTimeout(resolve, Config.gitlab.repository.timeoutAfterCreation)); @@ -130,7 +133,7 @@ class AssignmentRoutes implements RoutesManager { return await GitlabManager.addRepositoryMember(repository.id, memberId, Gitlab.AccessLevel.DEVELOPER); } catch ( error ) { logger.error('Add member error'); - logger.error(error); + logger.error(JSON.stringify(error)); return false; } })); @@ -189,13 +192,14 @@ class AssignmentRoutes implements RoutesManager { req.session.sendResponse(res, StatusCodes.OK); } catch ( error ) { - if ( error instanceof AxiosError ) { - res.status(error.response?.status ?? HttpStatusCode.InternalServerError).send(); + logger.error(JSON.stringify(error)); + + if ( error instanceof GitbeakerRequestError ) { + req.session.sendResponse(res, error.cause?.response.status ?? StatusCodes.INTERNAL_SERVER_ERROR, undefined, 'Error while updating the assignment state'); return; } - logger.error(error); - res.status(StatusCodes.INTERNAL_SERVER_ERROR).send(); + req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, undefined, 'Error while updating the assignment state'); } }; } diff --git a/ExpressAPI/src/routes/ExerciseRoutes.ts b/ExpressAPI/src/routes/ExerciseRoutes.ts index febc46d..665e734 100644 --- a/ExpressAPI/src/routes/ExerciseRoutes.ts +++ b/ExpressAPI/src/routes/ExerciseRoutes.ts @@ -7,7 +7,6 @@ import ParamsValidatorMiddleware from '../middlewares/ParamsValidatorMiddleware' import SecurityMiddleware from '../middlewares/SecurityMiddleware'; import GitlabManager from '../managers/GitlabManager'; import Config from '../config/Config'; -import { AxiosError } from 'axios'; import logger from '../shared/logging/WinstonLogger'; import DojoValidators from '../helpers/DojoValidators'; import { v4 as uuidv4 } from 'uuid'; @@ -26,6 +25,7 @@ import { IFileDirStat } from '../shared/helpers/recursiveFilesStats/Rec import ExerciseManager from '../managers/ExerciseManager'; import * as Gitlab from '@gitbeaker/rest'; import GitlabTreeFileType from '../shared/types/Gitlab/GitlabTreeFileType'; +import { GitbeakerRequestError } from '@gitbeaker/requester-utils'; class ExerciseRoutes implements RoutesManager { @@ -107,14 +107,17 @@ class ExerciseRoutes implements RoutesManager { break; } catch ( error ) { logger.error('Repo creation error'); - logger.error(error); - - if ( error instanceof AxiosError ) { - if ( error.response?.data.message.name && error.response.data.message.name === 'has already been taken' ) { - suffix++; - } else { - req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, {}, 'Unknown gitlab error while forking repository', DojoStatusCode.EXERCISE_CREATION_GITLAB_ERROR); - return undefined; + logger.error(JSON.stringify(error)); + + if ( error instanceof GitbeakerRequestError ) { + if ( error.cause?.description ) { + const description = error.cause.description as unknown; + if ( description instanceof Object && 'name' in description && description.name instanceof Array && description.name.length > 0 && description.name[0] === 'has already been taken' ) { + suffix++; + } else { + req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, {}, 'Unknown gitlab error while forking repository', DojoStatusCode.EXERCISE_CREATION_GITLAB_ERROR); + return undefined; + } } } else { req.session.sendResponse(res, StatusCodes.INTERNAL_SERVER_ERROR, {}, 'Unknown error while forking repository', DojoStatusCode.EXERCISE_CREATION_INTERNAL_ERROR); @@ -181,7 +184,7 @@ class ExerciseRoutes implements RoutesManager { return await GitlabManager.addRepositoryMember(repository.id, memberId, Gitlab.AccessLevel.DEVELOPER); } catch ( error ) { logger.error('Add member error'); - logger.error(error); + logger.error(JSON.stringify(error)); return false; } })); -- GitLab