Skip to content

Commit

Permalink
Rename AppError to ApiError
Browse files Browse the repository at this point in the history
  • Loading branch information
hagopj13 committed May 12, 2020
1 parent ffb47ae commit ea226b0
Show file tree
Hide file tree
Showing 9 changed files with 47 additions and 47 deletions.
6 changes: 3 additions & 3 deletions src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const { jwtStrategy } = require('./config/passport');
const { authLimiter } = require('./middlewares/rateLimiter');
const routes = require('./routes/v1');
const { errorConverter, errorHandler } = require('./middlewares/error');
const AppError = require('./utils/AppError');
const ApiError = require('./utils/ApiError');

const app = express();

Expand Down Expand Up @@ -55,10 +55,10 @@ app.use('/v1', routes);

// send back a 404 error for any unknown api request
app.use((req, res, next) => {
next(new AppError(httpStatus.NOT_FOUND, 'Not found'));
next(new ApiError(httpStatus.NOT_FOUND, 'Not found'));
});

// convert error to AppError, if needed
// convert error to ApiError, if needed
app.use(errorConverter);

// handle error
Expand Down
12 changes: 6 additions & 6 deletions src/controllers/auth.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ const httpStatus = require('http-status');
const catchAsync = require('../utils/catchAsync');
const { User, Token } = require('../models');
const { tokenService, emailService } = require('../services');
const AppError = require('../utils/AppError');
const ApiError = require('../utils/ApiError');

const register = catchAsync(async (req, res) => {
if (await User.isEmailTaken(req.body.email)) {
throw new AppError(httpStatus.BAD_REQUEST, 'Email already taken');
throw new ApiError(httpStatus.BAD_REQUEST, 'Email already taken');
}
const user = await User.create(req.body);
const tokens = await tokenService.generateAuthTokens(user.id);
Expand All @@ -18,7 +18,7 @@ const login = catchAsync(async (req, res) => {
const { email, password } = req.body;
const user = await User.findOne({ email });
if (!user || !(await user.isPasswordMatch(password))) {
throw new AppError(httpStatus.UNAUTHORIZED, 'Incorrect email or password');
throw new ApiError(httpStatus.UNAUTHORIZED, 'Incorrect email or password');
}
const tokens = await tokenService.generateAuthTokens(user.id);
const response = { user: user.transform(), tokens };
Expand All @@ -37,14 +37,14 @@ const refreshTokens = catchAsync(async (req, res) => {
const response = { ...tokens };
res.send(response);
} catch (error) {
throw new AppError(httpStatus.UNAUTHORIZED, 'Please authenticate');
throw new ApiError(httpStatus.UNAUTHORIZED, 'Please authenticate');
}
});

const forgotPassword = catchAsync(async (req, res) => {
const user = await User.findOne({ email: req.body.email });
if (!user) {
throw new AppError(httpStatus.NOT_FOUND, 'No users found with this email');
throw new ApiError(httpStatus.NOT_FOUND, 'No users found with this email');
}
const resetPasswordToken = await tokenService.generateResetPasswordToken(user.id);
await emailService.sendResetPasswordEmail(req.body.email, resetPasswordToken);
Expand All @@ -63,7 +63,7 @@ const resetPassword = catchAsync(async (req, res) => {
await Token.deleteMany({ user: user.id, type: 'resetPassword' });
res.status(httpStatus.NO_CONTENT).send();
} catch (error) {
throw new AppError(httpStatus.UNAUTHORIZED, 'Password reset failed');
throw new ApiError(httpStatus.UNAUTHORIZED, 'Password reset failed');
}
});

Expand Down
12 changes: 6 additions & 6 deletions src/controllers/user.controller.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
const httpStatus = require('http-status');
const { pick } = require('lodash');
const { User } = require('../models');
const AppError = require('../utils/AppError');
const ApiError = require('../utils/ApiError');
const catchAsync = require('../utils/catchAsync');
const { getQueryOptions } = require('../utils/query.utils');

const createUser = catchAsync(async (req, res) => {
if (await User.isEmailTaken(req.body.email)) {
throw new AppError(httpStatus.BAD_REQUEST, 'Email already taken');
throw new ApiError(httpStatus.BAD_REQUEST, 'Email already taken');
}
const user = await User.create(req.body);
res.status(httpStatus.CREATED).send(user.transform());
Expand All @@ -24,18 +24,18 @@ const getUsers = catchAsync(async (req, res) => {
const getUser = catchAsync(async (req, res) => {
const user = await User.findById(req.params.userId);
if (!user) {
throw new AppError(httpStatus.NOT_FOUND, 'User not found');
throw new ApiError(httpStatus.NOT_FOUND, 'User not found');
}
res.send(user.transform());
});

const updateUser = catchAsync(async (req, res) => {
const user = await User.findById(req.params.userId);
if (!user) {
throw new AppError(httpStatus.NOT_FOUND, 'User not found');
throw new ApiError(httpStatus.NOT_FOUND, 'User not found');
}
if (req.body.email && (await User.isEmailTaken(req.body.email, user.id))) {
throw new AppError(httpStatus.BAD_REQUEST, 'Email already taken');
throw new ApiError(httpStatus.BAD_REQUEST, 'Email already taken');
}
Object.assign(user, req.body);
await user.save();
Expand All @@ -45,7 +45,7 @@ const updateUser = catchAsync(async (req, res) => {
const deleteUser = catchAsync(async (req, res) => {
const user = await User.findById(req.params.userId);
if (!user) {
throw new AppError(httpStatus.NOT_FOUND, 'User not found');
throw new ApiError(httpStatus.NOT_FOUND, 'User not found');
}
await user.remove();
res.status(httpStatus.NO_CONTENT).send();
Expand Down
6 changes: 3 additions & 3 deletions src/middlewares/auth.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
const passport = require('passport');
const httpStatus = require('http-status');
const AppError = require('../utils/AppError');
const ApiError = require('../utils/ApiError');
const { roleRights } = require('../config/roles');

const verifyCallback = (req, resolve, reject, requiredRights) => async (err, user, info) => {
if (err || info || !user) {
return reject(new AppError(httpStatus.UNAUTHORIZED, 'Please authenticate'));
return reject(new ApiError(httpStatus.UNAUTHORIZED, 'Please authenticate'));
}
req.user = user;

if (requiredRights.length) {
const userRights = roleRights.get(user.role);
const hasRequiredRights = requiredRights.every(requiredRight => userRights.includes(requiredRight));
if (!hasRequiredRights && req.params.userId !== user.id) {
return reject(new AppError(httpStatus.FORBIDDEN, 'Forbidden'));
return reject(new ApiError(httpStatus.FORBIDDEN, 'Forbidden'));
}
}

Expand Down
6 changes: 3 additions & 3 deletions src/middlewares/error.js
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
const httpStatus = require('http-status');
const config = require('../config/config');
const logger = require('../config/logger');
const AppError = require('../utils/AppError');
const ApiError = require('../utils/ApiError');

const errorConverter = (err, req, res, next) => {
let error = err;
if (!(error instanceof AppError)) {
if (!(error instanceof ApiError)) {
const statusCode = error.statusCode || httpStatus.INTERNAL_SERVER_ERROR;
const message = error.message || httpStatus[statusCode];
error = new AppError(statusCode, message, false, err.stack);
error = new ApiError(statusCode, message, false, err.stack);
}
next(error);
};
Expand Down
4 changes: 2 additions & 2 deletions src/middlewares/validate.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const Joi = require('@hapi/joi');
const httpStatus = require('http-status');
const { pick } = require('lodash');
const AppError = require('../utils/AppError');
const ApiError = require('../utils/ApiError');

const validate = schema => (req, res, next) => {
const validSchema = pick(schema, ['params', 'query', 'body']);
Expand All @@ -12,7 +12,7 @@ const validate = schema => (req, res, next) => {

if (error) {
const errorMessage = error.details.map(details => details.message).join(', ');
return next(new AppError(httpStatus.BAD_REQUEST, errorMessage));
return next(new ApiError(httpStatus.BAD_REQUEST, errorMessage));
}
Object.assign(req, value);
return next();
Expand Down
4 changes: 2 additions & 2 deletions src/utils/AppError.js → src/utils/ApiError.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class AppError extends Error {
class ApiError extends Error {
constructor(statusCode, message, isOperational = true, stack = '') {
super(message);
this.statusCode = statusCode;
Expand All @@ -11,4 +11,4 @@ class AppError extends Error {
}
}

module.exports = AppError;
module.exports = ApiError;
14 changes: 7 additions & 7 deletions tests/integration/auth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const app = require('../../src/app');
const config = require('../../src/config/config');
const auth = require('../../src/middlewares/auth');
const { tokenService, emailService } = require('../../src/services');
const AppError = require('../../src/utils/AppError');
const ApiError = require('../../src/utils/ApiError');
const setupTestDB = require('../utils/setupTestDB');
const { User, Token } = require('../../src/models');
const { roleRights } = require('../../src/config/roles');
Expand Down Expand Up @@ -395,7 +395,7 @@ describe('Auth middleware', () => {

await auth()(req, httpMocks.createResponse(), next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({ statusCode: httpStatus.UNAUTHORIZED, message: 'Please authenticate' })
);
Expand All @@ -408,7 +408,7 @@ describe('Auth middleware', () => {

await auth()(req, httpMocks.createResponse(), next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({ statusCode: httpStatus.UNAUTHORIZED, message: 'Please authenticate' })
);
Expand All @@ -423,7 +423,7 @@ describe('Auth middleware', () => {

await auth()(req, httpMocks.createResponse(), next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({ statusCode: httpStatus.UNAUTHORIZED, message: 'Please authenticate' })
);
Expand All @@ -438,7 +438,7 @@ describe('Auth middleware', () => {

await auth()(req, httpMocks.createResponse(), next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({ statusCode: httpStatus.UNAUTHORIZED, message: 'Please authenticate' })
);
Expand All @@ -450,7 +450,7 @@ describe('Auth middleware', () => {

await auth()(req, httpMocks.createResponse(), next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({ statusCode: httpStatus.UNAUTHORIZED, message: 'Please authenticate' })
);
Expand All @@ -463,7 +463,7 @@ describe('Auth middleware', () => {

await auth('anyRight')(req, httpMocks.createResponse(), next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(expect.objectContaining({ statusCode: httpStatus.FORBIDDEN, message: 'Forbidden' }));
});

Expand Down
30 changes: 15 additions & 15 deletions tests/unit/middlewares/error.test.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,29 @@
const httpStatus = require('http-status');
const httpMocks = require('node-mocks-http');
const { errorConverter, errorHandler } = require('../../../src/middlewares/error');
const AppError = require('../../../src/utils/AppError');
const ApiError = require('../../../src/utils/ApiError');
const config = require('../../../src/config/config');
const logger = require('../../../src/config/logger');

describe('Error middlewares', () => {
describe('Error converter', () => {
test('should return the same AppError object it was called with', () => {
const error = new AppError(httpStatus.BAD_REQUEST, 'Any error');
test('should return the same ApiError object it was called with', () => {
const error = new ApiError(httpStatus.BAD_REQUEST, 'Any error');
const next = jest.fn();

errorConverter(error, httpMocks.createRequest(), httpMocks.createResponse, next);

expect(next).toHaveBeenCalledWith(error);
});

test('should convert an Error to AppError and preserve its status and message', () => {
test('should convert an Error to ApiError and preserve its status and message', () => {
const error = new Error('Any error');
error.statusCode = httpStatus.BAD_REQUEST;
const next = jest.fn();

errorConverter(error, httpMocks.createRequest(), httpMocks.createResponse, next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({
statusCode: error.statusCode,
Expand All @@ -33,13 +33,13 @@ describe('Error middlewares', () => {
);
});

test('should convert an Error without status to AppError with status 500', () => {
test('should convert an Error without status to ApiError with status 500', () => {
const error = new Error('Any error');
const next = jest.fn();

errorConverter(error, httpMocks.createRequest(), httpMocks.createResponse, next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({
statusCode: httpStatus.INTERNAL_SERVER_ERROR,
Expand All @@ -49,14 +49,14 @@ describe('Error middlewares', () => {
);
});

test('should convert an Error without message to AppError with default message of that http status', () => {
test('should convert an Error without message to ApiError with default message of that http status', () => {
const error = new Error();
error.statusCode = httpStatus.BAD_REQUEST;
const next = jest.fn();

errorConverter(error, httpMocks.createRequest(), httpMocks.createResponse, next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({
statusCode: error.statusCode,
Expand All @@ -66,13 +66,13 @@ describe('Error middlewares', () => {
);
});

test('should convert any other object to AppError with status 500 and its message', () => {
test('should convert any other object to ApiError with status 500 and its message', () => {
const error = {};
const next = jest.fn();

errorConverter(error, httpMocks.createRequest(), httpMocks.createResponse, next);

expect(next).toHaveBeenCalledWith(expect.any(AppError));
expect(next).toHaveBeenCalledWith(expect.any(ApiError));
expect(next).toHaveBeenCalledWith(
expect.objectContaining({
statusCode: httpStatus.INTERNAL_SERVER_ERROR,
Expand All @@ -89,7 +89,7 @@ describe('Error middlewares', () => {
});

test('should send proper error response and put the error message in res.locals', () => {
const error = new AppError(httpStatus.BAD_REQUEST, 'Any error');
const error = new ApiError(httpStatus.BAD_REQUEST, 'Any error');
const res = httpMocks.createResponse();
const sendSpy = jest.spyOn(res, 'send');

Expand All @@ -101,7 +101,7 @@ describe('Error middlewares', () => {

test('should put the error stack in the response if in development mode', () => {
config.env = 'development';
const error = new AppError(httpStatus.BAD_REQUEST, 'Any error');
const error = new ApiError(httpStatus.BAD_REQUEST, 'Any error');
const res = httpMocks.createResponse();
const sendSpy = jest.spyOn(res, 'send');

Expand All @@ -115,7 +115,7 @@ describe('Error middlewares', () => {

test('should send internal server error status and message if in production mode and error is not operational', () => {
config.env = 'production';
const error = new AppError(httpStatus.BAD_REQUEST, 'Any error', false);
const error = new ApiError(httpStatus.BAD_REQUEST, 'Any error', false);
const res = httpMocks.createResponse();
const sendSpy = jest.spyOn(res, 'send');

Expand All @@ -133,7 +133,7 @@ describe('Error middlewares', () => {

test('should preserve original error status and message if in production mode and error is operational', () => {
config.env = 'production';
const error = new AppError(httpStatus.BAD_REQUEST, 'Any error');
const error = new ApiError(httpStatus.BAD_REQUEST, 'Any error');
const res = httpMocks.createResponse();
const sendSpy = jest.spyOn(res, 'send');

Expand Down

0 comments on commit ea226b0

Please sign in to comment.