Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: use celebrate error handler #458

Merged
merged 4 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 38 additions & 27 deletions src/app/modules/auth/__tests__/auth.routes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as OtpUtils from 'src/app/utils/otp'
import { IAgencySchema } from 'src/types'

import { setupApp } from 'tests/integration/helpers/express-setup'
import { buildCelebrateError } from 'tests/unit/backend/helpers/celebrate'
import dbHandler from 'tests/unit/backend/helpers/jest-db'

import { MailSendError } from '../../../services/mail/mail.errors'
Expand Down Expand Up @@ -38,9 +39,9 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'email' } }),
)
})

it('should return 400 when body.email is invalid', async () => {
Expand All @@ -54,9 +55,11 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({
body: { key: 'email', message: 'Please enter a valid email' },
}),
)
})

it('should return 401 when domain of body.email does not exist in Agency collection', async () => {
Expand Down Expand Up @@ -131,9 +134,9 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'email' } }),
)
})

it('should return 400 when body.email is invalid', async () => {
Expand All @@ -147,9 +150,11 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({
body: { key: 'email', message: 'Please enter a valid email' },
}),
)
})

it('should return 401 when domain of body.email does not exist in Agency collection', async () => {
Expand Down Expand Up @@ -267,9 +272,9 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'email' } }),
)
})

it('should return 400 when body.otp is not provided as a param', async () => {
Expand All @@ -280,9 +285,9 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'otp' } }),
)
})

it('should return 400 when body.email is invalid', async () => {
Expand All @@ -296,9 +301,11 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({
body: { key: 'email', message: 'Please enter a valid email' },
}),
)
})

it('should return 400 when body.otp is less than 6 digits', async () => {
Expand All @@ -310,9 +317,11 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({
body: { key: 'otp', message: 'Please enter a valid otp' },
}),
)
})

it('should return 400 when body.otp is 6 characters but does not consist purely of digits', async () => {
Expand All @@ -324,9 +333,11 @@ describe('auth.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({
body: { key: 'otp', message: 'Please enter a valid otp' },
}),
)
})

it('should return 401 when domain of body.email does not exist in Agency collection', async () => {
Expand Down
31 changes: 16 additions & 15 deletions src/app/modules/user/__tests__/user.routes.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { IAgencySchema, IUserSchema } from 'src/types'

import { createAuthedSession } from 'tests/integration/helpers/express-auth'
import { setupApp } from 'tests/integration/helpers/express-setup'
import { buildCelebrateError } from 'tests/unit/backend/helpers/celebrate'
import dbHandler from 'tests/unit/backend/helpers/jest-db'

import { DatabaseError } from '../../core/core.errors'
Expand Down Expand Up @@ -135,9 +136,9 @@ describe('user.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'contact' } }),
)
})

it('should return 400 when body.userId is not provided as a param', async () => {
Expand All @@ -148,9 +149,9 @@ describe('user.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'userId' } }),
)
})

it('should return 401 when body.userId does not match current session userId', async () => {
Expand Down Expand Up @@ -284,9 +285,9 @@ describe('user.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'contact' } }),
)
})

it('should return 400 when body.userId is not provided as a param', async () => {
Expand All @@ -298,9 +299,9 @@ describe('user.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'userId' } }),
)
})

it('should return 400 when body.otp is not provided as a param', async () => {
Expand All @@ -312,9 +313,9 @@ describe('user.routes', () => {

// Assert
expect(response.status).toEqual(400)
expect(response.body).toEqual({
message: 'Some required parameters are missing',
})
expect(response.body).toEqual(
buildCelebrateError({ body: { key: 'otp' } }),
)
})

it('should return 401 when body.userId does not match current session userId', async () => {
Expand Down
8 changes: 4 additions & 4 deletions src/loaders/express/error-handler.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { isCelebrateError } from 'celebrate'
import { errors, isCelebrateError } from 'celebrate'
import { ErrorRequestHandler, RequestHandler } from 'express'
import { StatusCodes } from 'http-status-codes'
import get from 'lodash/get'

import { createLoggerWithLabel } from '../../config/logger'

const logger = createLoggerWithLabel(module)
const celebrateErrorHandler = errors()

const errorHandlerMiddlewares = (): (
| ErrorRequestHandler
Expand Down Expand Up @@ -37,12 +38,11 @@ const errorHandlerMiddlewares = (): (
action: 'genericErrorHandlerMiddleware',
// formId is only present for Joi validated routes that require it
formId: get(req, 'form._id', null),
details: Object.fromEntries(err.details),
},
error: err,
})
return res
.status(StatusCodes.BAD_REQUEST)
.json({ message: 'Some required parameters are missing' })
return celebrateErrorHandler(err, req, res, next)
}

logger.error({
Expand Down
42 changes: 42 additions & 0 deletions tests/unit/backend/helpers/celebrate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { Segments } from 'celebrate'
import { getReasonPhrase } from 'http-status-codes'
import { mapValues } from 'lodash'
/**
* Maps the key type (e.g. param, body) to the required keys
*/
type ICelebrateSpec = {
[s in Segments]?: {
key: string
message?: string
}
}

/**
* Options for customising celebrate error
*/
interface ICelebrateOpts {
message?: string
statusCode?: number
}

export const buildCelebrateError = (
spec: ICelebrateSpec,
opts: ICelebrateOpts = {},
) => {
opts.message ??= 'celebrate request validation failed'
opts.statusCode ??= 400
return {
statusCode: opts.statusCode,
message: opts.message,
error: getReasonPhrase(opts.statusCode),
validation: mapValues(spec, (requirements, keyType) => {
return (
requirements && {
source: keyType,
keys: [requirements.key],
message: requirements.message ?? `"${requirements.key}" is required`,
}
)
}),
}
}