diff --git a/common/script/api-v3/customError.js b/common/script/api-v3/customError.js deleted file mode 100644 index 3ebf5a316f..0000000000 --- a/common/script/api-v3/customError.js +++ /dev/null @@ -1,8 +0,0 @@ -// Base class for custom application errors -// It extends Error and capture the stack trace -export default class CustomError extends Error { - constructor () { - super(); - Error.captureStackTrace(this, this.constructor); - } -} diff --git a/common/script/api-v3/errors.js b/common/script/api-v3/errors.js index d4db1183da..5751a28c64 100644 --- a/common/script/api-v3/errors.js +++ b/common/script/api-v3/errors.js @@ -1,9 +1,37 @@ -import CustomError from './customError'; +// Base class for custom application errors +// It extends Error and capture the stack trace +export class CustomError extends Error { + constructor () { + super(); + Error.captureStackTrace(this, this.constructor); + } +} + +// We specify an httpCode for all errors so that they can be used in the API too export class NotAuthorized extends CustomError { constructor (customMessage) { super(); this.name = this.constructor.name; + this.httpCode = 401; this.message = customMessage || 'Not authorized.'; } } + +export class BadRequest extends CustomError { + constructor (customMessage) { + super(); + this.name = this.constructor.name; + this.httpCode = 400; + this.message = customMessage || 'Bad request.'; + } +} + +export class NotFound extends CustomError { + constructor (customMessage) { + super(); + this.name = this.constructor.name; + this.httpCode = 404; + this.message = customMessage || 'Not found.'; + } +} diff --git a/test/api/v3/integration/user/POST-user_class_cast_spellId.test.js b/test/api/v3/integration/user/POST-user_class_cast_spellId.test.js index 169ad16dd2..0074b38b59 100644 --- a/test/api/v3/integration/user/POST-user_class_cast_spellId.test.js +++ b/test/api/v3/integration/user/POST-user_class_cast_spellId.test.js @@ -68,7 +68,7 @@ describe('POST /user/class/cast/:spellId', () => { it('returns an error if user doesn\'t own the spell', async () => { await expect(user.post(`/user/class/cast/snowball`)) .to.eventually.be.rejected.and.eql({ - code: 400, + code: 401, error: 'NotAuthorized', message: t('spellNotOwned'), }); diff --git a/test/api/v3/unit/libs/errors.test.js b/test/api/v3/unit/libs/errors.test.js index 15110a06ee..2a96f05443 100644 --- a/test/api/v3/unit/libs/errors.test.js +++ b/test/api/v3/unit/libs/errors.test.js @@ -1,5 +1,6 @@ +// TODO move to shared tests +import { CustomError } from '../../../../../common/script/api-v3/errors'; import { - CustomError, NotAuthorized, BadRequest, InternalServerError, diff --git a/test/api/v3/unit/middlewares/errorHandler.test.js b/test/api/v3/unit/middlewares/errorHandler.test.js index 694a02a8fe..269390b35b 100644 --- a/test/api/v3/unit/middlewares/errorHandler.test.js +++ b/test/api/v3/unit/middlewares/errorHandler.test.js @@ -9,7 +9,6 @@ import responseMiddleware from '../../../../../website/src/middlewares/api-v3/re import getUserLanguage from '../../../../../website/src/middlewares/api-v3/getUserLanguage'; import { BadRequest } from '../../../../../website/src/libs/api-v3/errors'; -import { NotAuthorized as NotAuthorizedShared } from '../../../../../common/script/api-v3/errors'; import logger from '../../../../../website/src/libs/api-v3/logger'; describe('errorHandler', () => { @@ -87,21 +86,6 @@ describe('errorHandler', () => { }); }); - it('handle CustomError(s) from shared code', () => { - let error = new NotAuthorizedShared(); - - errorHandler(error, req, res, next); - - expect(res.status).to.be.calledOnce; - expect(res.json).to.be.calledOnce; - - expect(res.status).to.be.calledWith(400); - expect(res.json).to.be.calledWith({ - error: 'NotAuthorized', - message: 'Not authorized.', - }); - }); - it('handle http-errors errors', () => { let error = new Error('custom message'); error.statusCode = 422; diff --git a/website/src/libs/api-v3/errors.js b/website/src/libs/api-v3/errors.js index e1218c805a..884b65f102 100644 --- a/website/src/libs/api-v3/errors.js +++ b/website/src/libs/api-v3/errors.js @@ -1,4 +1,4 @@ -import CustomError from '../../../../common/script/api-v3/customError'; +import { CustomError } from '../../../../common/script/api-v3/errors'; /** * @apiDefine NotAuthorized @@ -11,14 +11,7 @@ import CustomError from '../../../../common/script/api-v3/customError'; * "message": "Not authorized." * } */ -export class NotAuthorized extends CustomError { - constructor (customMessage) { - super(); - this.name = this.constructor.name; - this.httpCode = 401; - this.message = customMessage || 'Not authorized.'; - } -} +export { NotAuthorized } from '../../../../common/script/api-v3/errors'; /** * @apiDefine BadRequest @@ -31,14 +24,7 @@ export class NotAuthorized extends CustomError { * "message": "Bad request." * } */ -export class BadRequest extends CustomError { - constructor (customMessage) { - super(); - this.name = this.constructor.name; - this.httpCode = 400; - this.message = customMessage || 'Bad request.'; - } -} +export { BadRequest } from '../../../../common/script/api-v3/errors'; /** * @apiDefine NotFound @@ -51,14 +37,7 @@ export class BadRequest extends CustomError { * "message": "Not found." * } */ -export class NotFound extends CustomError { - constructor (customMessage) { - super(); - this.name = this.constructor.name; - this.httpCode = 404; - this.message = customMessage || 'Not found.'; - } -} +export { NotFound } from '../../../../common/script/api-v3/errors'; /** * @apiDefine InternalServerError diff --git a/website/src/middlewares/api-v3/errorHandler.js b/website/src/middlewares/api-v3/errorHandler.js index 0518962dff..2c49252384 100644 --- a/website/src/middlewares/api-v3/errorHandler.js +++ b/website/src/middlewares/api-v3/errorHandler.js @@ -1,7 +1,7 @@ // The error handler middleware that handles all errors // and respond to the client import logger from '../../libs/api-v3/logger'; -import CustomError from '../../../../common/script/api-v3/customError'; +import { CustomError } from '../../../../common/script/api-v3/errors'; import { BadRequest, InternalServerError, @@ -24,12 +24,6 @@ export default function errorHandler (err, req, res, next) { // eslint-disable-l // If we can't identify it, respond with a generic 500 error let responseErr = err instanceof CustomError ? err : null; - // TODO don't return always 400 for errors in common code - // If CustomError but without httpCode then they come from shared code, treat as 400s - if (err instanceof CustomError && !err.httpCode) { - err.httpCode = 400; - } - // Handle errors created with 'http-errors' or similar that have a status/statusCode property if (err.statusCode && typeof err.statusCode === 'number') { responseErr = new CustomError();