fix: FORMS-1139 handle express errors (bcgov#1380)

nimya-aot · Jun 5, 2024 · 8560143 · 8560143
1 parent 88a1390
commit 8560143
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 13 deletions.
diff --git a/app/src/forms/common/middleware/dataErrors.js b/app/src/forms/common/middleware/dataErrors.js
@@ -2,7 +2,8 @@ const Problem = require('api-problem');
 const Objection = require('objection');
 
 module.exports.dataErrors = async (err, _req, res, next) => {
-  let error = err;
+  let error;
+
   if (err instanceof Objection.DataError) {
     error = new Problem(422, {
       detail: 'Sorry... the database does not like the data you provided :(',
@@ -20,6 +21,14 @@ module.exports.dataErrors = async (err, _req, res, next) => {
       detail: 'Validation Error',
       errors: err.data,
     });
+  } else if (!(err instanceof Problem) && (err.status || err.statusCode)) {
+    // Express throws Errors that are not Problems, but do have an HTTP status
+    // code. For example, 400 is thrown when POST bodies are malformed JSON.
+    error = new Problem(err.status || err.statusCode, {
+      detail: err.message,
+    });
+  } else {
+    error = err;
   }
 
   if (error instanceof Problem && error.status !== 500) {

diff --git a/app/tests/unit/forms/common/middleware/dataErrors.spec.js b/app/tests/unit/forms/common/middleware/dataErrors.spec.js
@@ -9,8 +9,7 @@ describe('test data errors middleware', () => {
     const error = new Objection.DataError({
       nativeError: { message: 'This is a DataError' },
     });
-    const { res } = getMockRes();
-    const next = jest.fn();
+    const { res, next } = getMockRes();
 
     middleware.dataErrors(error, {}, res, next);
 
@@ -22,8 +21,7 @@ describe('test data errors middleware', () => {
     const error = new Objection.NotFoundError({
       nativeError: { message: 'This is a NotFoundError' },
     });
-    const { res } = getMockRes();
-    const next = jest.fn();
+    const { res, next } = getMockRes();
 
     middleware.dataErrors(error, {}, res, next);
 
@@ -35,8 +33,7 @@ describe('test data errors middleware', () => {
     const error = new Objection.UniqueViolationError({
       nativeError: { message: 'This is a UniqueViolationError' },
     });
-    const { res } = getMockRes();
-    const next = jest.fn();
+    const { res, next } = getMockRes();
 
     middleware.dataErrors(error, {}, res, next);
 
@@ -48,19 +45,41 @@ describe('test data errors middleware', () => {
     const error = new Objection.ValidationError({
       nativeError: { message: 'This is a ValidationError' },
     });
-    const { res } = getMockRes();
-    const next = jest.fn();
+    const { res, next } = getMockRes();
 
     middleware.dataErrors(error, {}, res, next);
 
     expect(next).not.toBeCalled();
     expect(res.end).toBeCalledWith(expect.stringContaining('422'));
   });
 
+  it('should handle non-problem errors with a status', () => {
+    const error = new Error('This is a 400 status.');
+    error.status = 400;
+    const { res, next } = getMockRes();
+
+    middleware.dataErrors(error, {}, res, next);
+
+    expect(next).not.toBeCalled();
+    expect(res.end).toBeCalledWith(expect.stringContaining('400'));
+    expect(res.end).toBeCalledWith(expect.stringContaining('This is a 400 status.'));
+  });
+
+  it('should handle non-problem errors with a status code', () => {
+    const error = new Error('This is a 400 status code.');
+    error.statusCode = 400;
+    const { res, next } = getMockRes();
+
+    middleware.dataErrors(error, {}, res, next);
+
+    expect(next).not.toBeCalled();
+    expect(res.end).toBeCalledWith(expect.stringContaining('400'));
+    expect(res.end).toBeCalledWith(expect.stringContaining('This is a 400 status code.'));
+  });
+
   it('should handle any non-500 Problems', () => {
     const error = new Problem(429);
-    const { res } = getMockRes();
-    const next = jest.fn();
+    const { res, next } = getMockRes();
 
     middleware.dataErrors(error, {}, res, next);
 
@@ -70,7 +89,7 @@ describe('test data errors middleware', () => {
 
   it('should pass through any 500s', () => {
     const error = new Problem(500);
-    const next = jest.fn();
+    const { next } = getMockRes();
 
     middleware.dataErrors(error, {}, {}, next);
 
@@ -79,7 +98,7 @@ describe('test data errors middleware', () => {
 
   it('should pass through any Errors', () => {
     const error = new Error();
-    const next = jest.fn();
+    const { next } = getMockRes();
 
     middleware.dataErrors(error, {}, {}, next);