Skip to content

Commit

Permalink
fix: remove form.msgSrvcName validation and add human error recovery …
Browse files Browse the repository at this point in the history
…message (#1235)
  • Loading branch information
karrui authored Mar 1, 2021
1 parent b37186a commit 5d16aeb
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 26 deletions.
4 changes: 0 additions & 4 deletions src/app/models/form.server.model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,10 +315,6 @@ const compileFormModel = (db: Mongoose): IFormModel => {
// Name of credentials for messaging service, stored in secrets manager
type: String,
required: false,
validate: [
/^([a-zA-Z0-9-/])+$/i,
'msgSrvcName must be alphanumeric, dashes and slashes are allowed',
],
},

submissionLimit: { type: Number, default: null },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from 'src/app/modules/core/core.errors'
import { MissingUserError } from 'src/app/modules/user/user.errors'
import * as UserService from 'src/app/modules/user/user.service'
import { formatErrorRecoveryMessage } from 'src/app/utils/handle-mongo-error'
import { aws } from 'src/config/config'
import { EditFieldActions, VALID_UPLOAD_FILE_TYPES } from 'src/shared/constants'
import {
Expand Down Expand Up @@ -361,7 +362,7 @@ describe('admin-form.service', () => {
// Assert
expect(actual.isErr()).toEqual(true)
expect(actual._unsafeUnwrapErr()).toEqual(
new DatabaseError(mockErrorString),
new DatabaseError(formatErrorRecoveryMessage(mockErrorString)),
)
})
})
Expand Down Expand Up @@ -518,7 +519,7 @@ describe('admin-form.service', () => {
// Assert
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toEqual(
new DatabaseError(mockErrorString),
new DatabaseError(formatErrorRecoveryMessage(mockErrorString)),
)
expect(createSpy).toHaveBeenCalledWith(expectedParams)
expect(mockForm.getDuplicateParams).toHaveBeenCalledWith({
Expand Down Expand Up @@ -763,7 +764,7 @@ describe('admin-form.service', () => {
// Assert
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toEqual(
new DatabaseError(mockPopulateErrorStr),
new DatabaseError(formatErrorRecoveryMessage(mockPopulateErrorStr)),
)
expect(mockValidForm.transferOwner).toHaveBeenCalledWith(
MOCK_CURRENT_OWNER,
Expand Down Expand Up @@ -867,7 +868,9 @@ describe('admin-form.service', () => {

// Assert
expect(actualResult._unsafeUnwrapErr()).toEqual(
new DatabasePayloadSizeError(mockErrorString),
new DatabasePayloadSizeError(
formatErrorRecoveryMessage(mockErrorString),
),
)
expect(createSpy).toHaveBeenCalledWith(formParams)
})
Expand All @@ -890,7 +893,7 @@ describe('admin-form.service', () => {

// Assert
expect(actualResult._unsafeUnwrapErr()).toEqual(
new DatabaseError(mockErrorString),
new DatabaseError(formatErrorRecoveryMessage(mockErrorString)),
)
expect(createSpy).toHaveBeenCalledWith(formParams)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
MalformedParametersError,
} from 'src/app/modules/core/core.errors'
import { CreatePresignedUrlError } from 'src/app/modules/form/admin-form/admin-form.errors'
import { formatErrorRecoveryMessage } from 'src/app/utils/handle-mongo-error'
import { aws } from 'src/config/config'
import {
SubmissionCursorData,
Expand Down Expand Up @@ -434,7 +435,7 @@ describe('encrypt-submission.service', () => {
// Should be error.
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toEqual(
new DatabaseError(mockErrorString),
new DatabaseError(formatErrorRecoveryMessage(mockErrorString)),
)
expect(getSubmissionSpy).toHaveBeenCalledWith(
mockFormId,
Expand Down Expand Up @@ -611,7 +612,7 @@ describe('encrypt-submission.service', () => {
// Arrange
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toEqual(
new DatabaseError(mockErrorString),
new DatabaseError(formatErrorRecoveryMessage(mockErrorString)),
)
expect(getMetaSpy).toHaveBeenCalledWith(MOCK_FORM_ID, mockSubmissionId)
})
Expand Down Expand Up @@ -689,7 +690,7 @@ describe('encrypt-submission.service', () => {
// Arrange
expect(actualResult.isErr()).toEqual(true)
expect(actualResult._unsafeUnwrapErr()).toEqual(
new DatabaseError(mockErrorString),
new DatabaseError(formatErrorRecoveryMessage(mockErrorString)),
)
expect(getMetaSpy).toHaveBeenCalledWith(MOCK_FORM_ID, { page: undefined })
})
Expand Down
25 changes: 19 additions & 6 deletions src/app/utils/handle-mongo-error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ import {
DatabaseValidationError,
} from '../modules/core/core.errors'

/**
* Exported for testing.
* Format error recovery message to be returned to client.
* @param errMsg the error message
*/
export const formatErrorRecoveryMessage = (errMsg: string): string => {
return `Error: [${errMsg}]. Please refresh and try again. If you still need help, email us at [email protected].`
}

export const getMongoErrorMessage = (
err?: unknown,
// Default error message if no more specific error
Expand All @@ -21,9 +30,11 @@ export const getMongoErrorMessage = (
if (err instanceof MongoError) {
switch (err.code) {
case 10334: // BSONObj size invalid error
return 'Your form is too large to be supported by the system.'
return formatErrorRecoveryMessage(
'Your form is too large to be supported by the system.',
)
default:
return defaultErrorMessage
return formatErrorRecoveryMessage(defaultErrorMessage)
}
}

Expand All @@ -34,18 +45,20 @@ export const getMongoErrorMessage = (
.map((err) => err.message)
.join(', ')

return joinedMessage ?? err.message ?? defaultErrorMessage
return formatErrorRecoveryMessage(
joinedMessage ?? err.message ?? defaultErrorMessage,
)
}

if (err instanceof MongooseError || err instanceof Error) {
return err.message ?? defaultErrorMessage
return formatErrorRecoveryMessage(err.message ?? defaultErrorMessage)
}

if (typeof err === 'string') {
return err ?? defaultErrorMessage
return formatErrorRecoveryMessage(err ?? defaultErrorMessage)
}

return defaultErrorMessage
return formatErrorRecoveryMessage(defaultErrorMessage)
}

/**
Expand Down
46 changes: 38 additions & 8 deletions tests/unit/backend/utils/handle-mongo-error.spec.ts
Original file line number Diff line number Diff line change
@@ -1,51 +1,81 @@
/* eslint-disable @typescript-eslint/ban-ts-comment */
import { MongoError } from 'mongodb'
import { Error as MongooseError } from 'mongoose'

import { getMongoErrorMessage } from 'src/app/utils/handle-mongo-error'
import {
formatErrorRecoveryMessage,
getMongoErrorMessage,
} from 'src/app/utils/handle-mongo-error'

describe('handleMongoError', () => {
describe('getMongoErrorMessage', () => {
it('should return blank string if no error', () => {
expect(getMongoErrorMessage()).toEqual('')
})

it('should return string if error is string', () => {
it('should return formatted string if error is string', () => {
const err = 'something failed'
expect(getMongoErrorMessage(err)).toEqual(err)
expect(getMongoErrorMessage(err)).toEqual(formatErrorRecoveryMessage(err))
})

it('should return form too large error message for error code 10334', () => {
const err = new MongoError('MongoError')
err.code = 10334

expect(getMongoErrorMessage(err)).toEqual(
'Your form is too large to be supported by the system.',
formatErrorRecoveryMessage(
'Your form is too large to be supported by the system.',
),
)
})

it('should return default error message for other MongoError error code', () => {
const err = new MongoError('MongoError')
expect(getMongoErrorMessage(err)).toEqual(
'An unexpected error happened. Please try again.',
formatErrorRecoveryMessage(
'An unexpected error happened. Please try again.',
),
) // Preset default error message
expect(getMongoErrorMessage(err, 'new error message')).toEqual(
'new error message',
formatErrorRecoveryMessage('new error message'),
) // Changed default error message
})

it('should join all error messages into a single message if available.', () => {
// @ts-ignore
const err = new MongooseError.ValidationError()
// @ts-ignore
const err1 = new MongooseError.ValidatorError({})
err1.message = 'abc'
// @ts-ignore
const err2 = new MongooseError.ValidatorError({})
err2.message = 'def'
err.errors = { err1, err2 }
expect(getMongoErrorMessage(err)).toEqual('abc, def')
expect(getMongoErrorMessage(err)).toEqual(
formatErrorRecoveryMessage('abc, def'),
)
})

it('should return error message for MongooseError', () => {
const err = new MongooseError('mongooseError')
expect(getMongoErrorMessage(err)).toEqual('mongooseError')
expect(getMongoErrorMessage(err)).toEqual(
formatErrorRecoveryMessage('mongooseError'),
)
})
})

describe('formatErrorRecoveryMessage', () => {
it('should format given string', () => {
// Arrange
const errorString = 'some test string'

// Act
const actual = formatErrorRecoveryMessage(errorString)

// Assert
expect(actual).toEqual(
`Error: [${errorString}]. Please refresh and try again. If you still need help, email us at [email protected].`,
)
})
})
})

0 comments on commit 5d16aeb

Please sign in to comment.