From c6bc5e6efc3ed335fe84594b218d2dc0c0598008 Mon Sep 17 00:00:00 2001 From: Alberto Casado Torres Date: Thu, 19 Sep 2024 11:12:20 +0200 Subject: [PATCH] Added loglevel to errors beign handled and added FileNotFound case (#7240) --- app/api/preserve/specs/preserve.spec.ts | 2 +- app/api/templates/specs/templates.spec.js | 2 +- app/api/users/specs/users.spec.js | 4 +-- app/api/utils/Error.js | 15 ++++++--- app/api/utils/handleError.js | 32 ++++++++++++++----- .../__snapshots__/handleError.spec.js.snap | 1 + 6 files changed, 40 insertions(+), 16 deletions(-) diff --git a/app/api/preserve/specs/preserve.spec.ts b/app/api/preserve/specs/preserve.spec.ts index 28a1769f82..3dfbd0168a 100644 --- a/app/api/preserve/specs/preserve.spec.ts +++ b/app/api/preserve/specs/preserve.spec.ts @@ -73,7 +73,7 @@ describe('Preserve', () => { ]; await testingEnvironment.setUp({ ...fixtures, settings: newSettings }, 'preserve-index'); - await expect(Preserve.setup('en', { _id: 'someid' })).rejects.toEqual({ + await expect(Preserve.setup('en', { _id: 'someid' })).rejects.toMatchObject({ message: 'Preserve configuration not found', code: 402, }); diff --git a/app/api/templates/specs/templates.spec.js b/app/api/templates/specs/templates.spec.js index bce97c9c96..d18ba12f6c 100644 --- a/app/api/templates/specs/templates.spec.js +++ b/app/api/templates/specs/templates.spec.js @@ -206,7 +206,7 @@ describe('templates', () => { await templates.save(changedTemplate); throw new Error('properties have swaped names, should have failed with an error'); } catch (error) { - expect(error).toEqual({ code: 400, message: "Properties can't swap names: text" }); + expect(error).toMatchObject({ code: 400, message: "Properties can't swap names: text" }); } }); diff --git a/app/api/users/specs/users.spec.js b/app/api/users/specs/users.spec.js index 7daaed279a..b6e81ae9d8 100644 --- a/app/api/users/specs/users.spec.js +++ b/app/api/users/specs/users.spec.js @@ -121,7 +121,7 @@ describe('Users', () => { _id: userId.toString(), username: 'user name', }; - await expect(users.save(userdata, currentUser)).rejects.toEqual({ + await expect(users.save(userdata, currentUser)).rejects.toMatchObject({ code: 400, message: 'Usernames can not contain spaces.', }); @@ -259,7 +259,7 @@ describe('Users', () => { role: 'editor', groups: [], }; - await expect(users.newUser(userdata, domain)).rejects.toEqual({ + await expect(users.newUser(userdata, domain)).rejects.toMatchObject({ code: 400, message: 'Usernames can not contain spaces.', }); diff --git a/app/api/utils/Error.js b/app/api/utils/Error.js index 045d718693..3fb74d8e01 100644 --- a/app/api/utils/Error.js +++ b/app/api/utils/Error.js @@ -1,11 +1,18 @@ -export default function (message, code = 500) { +/** + * @deprecated + */ +export default function (message, code = 500, logLevel = 'debug') { + if (code === 500) { + logLevel = 'error'; + } + if (message.ajv) { - return { message: message.message, ...message, code }; + return { message: message.message, ...message, code, logLevel }; } if (message instanceof Error) { - return { message: message.message, stack: message.stack, code, original: message }; + return { message: message.message, stack: message.stack, code, original: message, logLevel }; } - return { message, code }; + return { message, code, logLevel }; } diff --git a/app/api/utils/handleError.js b/app/api/utils/handleError.js index fa4750f183..38a8259773 100644 --- a/app/api/utils/handleError.js +++ b/app/api/utils/handleError.js @@ -4,6 +4,7 @@ import { createError } from 'api/utils/index'; import { appContext } from 'api/utils/AppContext'; import { UnauthorizedError } from 'api/authorization.v2/errors/UnauthorizedError'; import { ValidationError } from 'api/common.v2/validation/ValidationError'; +import { FileNotFound } from 'api/files/FileNotFound'; const ajvPrettifier = error => { const errorMessage = [error.message]; @@ -58,39 +59,53 @@ const prettifyError = (error, { req = {}, uncaught = false } = {}) => { let result = error; if (error instanceof Error) { - result = { code: 500, message: error.stack }; + result = { code: 500, message: error.stack, logLevel: 'error' }; } if (error instanceof Ajv.ValidationError) { - result = { code: 422, message: error.message, validations: error.errors }; + result = { code: 422, message: error.message, validations: error.errors, logLevel: 'debug' }; } if (error.name === 'ValidationError') { - result = { code: 422, message: error.message, validations: error.properties }; + result = { + code: 422, + message: error.message, + validations: error.properties, + logLevel: 'debug', + }; } if (error instanceof ValidationError) { - result = { code: 422, message: error.message, validations: error.errors }; + result = { code: 422, message: error.message, validations: error.errors, logLevel: 'debug' }; } if (error instanceof UnauthorizedError) { - result = { code: 401, message: error.message }; + result = { code: 401, message: error.message, logLevel: 'debug' }; + } + + if (error instanceof FileNotFound) { + result = { code: 404, message: error.message, logLevel: 'debug' }; } if (error.name === 'MongoError') { result.code = 500; + result.logLevel = 'error'; } if (error.message && error.message.match(/Cast to ObjectId failed for value/)) { result.code = 400; + result.logLevel = 'debug'; } if (error.message && error.message.match(/rison decoder error/)) { result.code = 400; + result.logLevel = 'debug'; } if (uncaught) { result.message = `uncaught exception or unhandled rejection, Node process finished !!\n ${result.message}`; + result.logLevel = 'error'; + result.code = 500; } const obfuscatedRequest = obfuscateCredentials(req); @@ -119,11 +134,12 @@ const getErrorMessage = (data, error) => { const sendLog = (data, error, errorOptions) => { const messageToLog = getErrorMessage(data, error); - if (data.code === 500) { - legacyLogger.error(messageToLog, errorOptions); - } else if (data.code === 400) { + if (data.logLevel === 'debug') { legacyLogger.debug(messageToLog, errorOptions); + return; } + + legacyLogger.error(messageToLog, errorOptions); }; function setRequestId(result) { diff --git a/app/api/utils/specs/__snapshots__/handleError.spec.js.snap b/app/api/utils/specs/__snapshots__/handleError.spec.js.snap index b52d62ba99..6fb41686ef 100644 --- a/app/api/utils/specs/__snapshots__/handleError.spec.js.snap +++ b/app/api/utils/specs/__snapshots__/handleError.spec.js.snap @@ -3,6 +3,7 @@ exports[`handleError errors by type when error is created with createError should return the error 1`] = ` Object { "code": 400, + "logLevel": "debug", "message": "test error", "prettyMessage": " test error",