fix tests, move most errors to shared code

common/script/api-v3/customError.js

@@ -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);
-  }
-}

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.';
+  }
+}

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'),
       });

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,

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;

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

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();