BugFix: Updated API Error Handling In Response (#1323)

Errors now bubbling up to the frontend, includes validation errors

api/src/database/db.test.ts

@@ -141,10 +141,7 @@ describe('db', () => {
 
             expect(expectedError.errors?.length).to.be.greaterThan(0);
             expectedError.errors?.forEach((item) => {
-              expect(item).to.be.instanceOf(Error);
-              if (item instanceof Error) {
-                expect(item.message).to.be.eql('DBPool is not initialized');
-              }
+              expect(item).to.be.eql({ name: 'Error', message: 'DBPool is not initialized' });
             });
             expect(getDBPoolStub).to.have.been.calledOnce;
 
@@ -227,10 +224,7 @@ describe('db', () => {
 
             expect(expectedError.errors?.length).to.be.greaterThan(0);
             expectedError.errors?.forEach((item) => {
-              expect(item).to.be.instanceOf(Error);
-              if (item instanceof Error) {
-                expect(item.message).to.be.eql('DBConnection is not open');
-              }
+              expect(item).to.be.eql({ name: 'Error', message: 'DBConnection is not open' });
             });
           });
         });
@@ -266,10 +260,7 @@ describe('db', () => {
 
             expect(expectedError.errors?.length).to.be.greaterThan(0);
             expectedError.errors?.forEach((item) => {
-              expect(item).to.be.instanceOf(Error);
-              if (item instanceof Error) {
-                expect(item.message).to.be.eql('DBConnection is not open');
-              }
+              expect(item).to.be.eql({ name: 'Error', message: 'DBConnection is not open' });
             });
           });
         });
@@ -315,10 +306,7 @@ describe('db', () => {
 
             expect(expectedError.errors?.length).to.be.greaterThan(0);
             expectedError.errors?.forEach((item) => {
-              expect(item).to.be.instanceOf(Error);
-              if (item instanceof Error) {
-                expect(item.message).to.be.eql('DBConnection is not open');
-              }
+              expect(item).to.be.eql({ name: 'Error', message: 'DBConnection is not open' });
             });
           });
         });
@@ -357,10 +345,7 @@ describe('db', () => {
 
             expect(expectedError.errors?.length).to.be.greaterThan(0);
             expectedError.errors?.forEach((item) => {
-              expect(item).to.be.instanceOf(Error);
-              if (item instanceof Error) {
-                expect(item.message).to.be.eql('DBConnection is not open');
-              }
+              expect(item).to.be.eql({ name: 'Error', message: 'DBConnection is not open' });
             });
           });
         });

api/src/errors/api-error.ts

@@ -1,27 +1,15 @@
+import { BaseError } from './base-error';
+
 export enum ApiErrorType {
   BUILD_SQL = 'Error constructing SQL query',
   EXECUTE_SQL = 'Error executing SQL query',
   GENERAL = 'Error',
   UNKNOWN = 'Unknown Error'
 }
 
-export class ApiError extends Error {
-  errors?: (string | object)[];
-
+export class ApiError extends BaseError {
   constructor(name: ApiErrorType, message: string, errors?: (string | object)[], stack?: string) {
-    super(message);
-
-    this.name = name;
-    this.errors = errors || [];
-    this.stack = stack;
-
-    if (stack) {
-      this.stack = stack;
-    }
-
-    if (!this.stack) {
-      Error.captureStackTrace(this);
-    }
+    super(name, message, errors, stack);
   }
 }
 

api/src/errors/base-error.ts

@@ -0,0 +1,25 @@
+export class BaseError extends Error {
+  errors: (string | object)[] = [];
+
+  constructor(name: string, message: string, errors?: (string | object)[], stack?: string) {
+    super(message);
+
+    this.name = name;
+
+    for (const error of errors ?? []) {
+      if (error instanceof Error) {
+        // By default, properties of Error objects are not enumerable, so we need to manually extract them.
+        this.errors.push({ name: error.name, message: error.message });
+      } else {
+        this.errors.push(error);
+      }
+    }
+
+    // If a stack is provided, use it. Otherwise, generate a new stack trace.
+    if (stack) {
+      this.stack = stack;
+    } else {
+      Error.captureStackTrace(this);
+    }
+  }
+}

api/src/errors/http-error.test.ts

@@ -2,7 +2,7 @@ import { expect } from 'chai';
 import { describe } from 'mocha';
 import { DatabaseError } from 'pg';
 import { ApiError, ApiErrorType } from './api-error';
-import { ensureHTTPError, HTTP400, HTTP401, HTTP403, HTTP409, HTTP500, HTTPError } from './http-error';
+import { ensureHTTPError, HTTP400, HTTP401, HTTP403, HTTP409, HTTP500, HTTPError, isAjvError } from './http-error';
 
 describe('HTTPError', () => {
   describe('No error value provided', () => {
@@ -74,6 +74,22 @@ describe('ensureHTTPError', () => {
     ]);
   });
 
+  it('returns a HTTPError when a AJVError object provided', function () {
+    const ajvError = { status: 400, errors: [{ instancePath: '/path' }] };
+
+    const ensuredError = ensureHTTPError(ajvError);
+
+    expect(ensuredError).to.be.instanceof(HTTPError);
+
+    expect(ensuredError.status).to.equal(400);
+    expect(ensuredError.message).to.equal('Request Validation Error');
+    expect(ensuredError.errors).to.eql([
+      {
+        instancePath: '/path'
+      }
+    ]);
+  });
+
   it('returns a HTTPError when a non Http Error provided', function () {
     const nonHttpError = new Error('a non http error');
 
@@ -83,7 +99,7 @@ describe('ensureHTTPError', () => {
 
     expect(ensuredError.status).to.equal(500);
     expect(ensuredError.message).to.equal('Unexpected Error');
-    expect(ensuredError.errors).to.eql(['Error', 'a non http error']);
+    expect(ensuredError.errors).to.eql([{ message: 'a non http error', name: 'Error' }]);
   });
 
   it('returns a generic HTTPError when a non Error provided', function () {
@@ -95,6 +111,25 @@ describe('ensureHTTPError', () => {
 
     expect(ensuredError.status).to.equal(500);
     expect(ensuredError.message).to.equal('Unexpected Error');
-    expect(ensuredError.errors).to.eql([]);
+    expect(ensuredError.errors).to.eql(['not an Error']);
+  });
+});
+
+describe('isAjvError', () => {
+  it('should return false when not an object', () => {
+    expect(isAjvError('not ajv')).to.be.false;
+    expect(isAjvError([])).to.be.false;
+    expect(isAjvError(1)).to.be.false;
+  });
+
+  it('should return false when missing status or errors', () => {
+    expect(isAjvError({ status: 1 })).to.be.false;
+    expect(isAjvError({ errors: 1 })).to.be.false;
+    expect(isAjvError({ status: 1, oops: 'oops' })).to.be.false;
+  });
+
+  it('should return true when errors and status both defined', () => {
+    expect(isAjvError({ status: 1, errors: [] })).to.be.true;
+    expect(isAjvError({ status: 1, errors: ['hello'] })).to.be.true;
   });
 });

api/src/errors/http-error.ts

@@ -1,5 +1,6 @@
 import { DatabaseError } from 'pg';
 import { ApiError } from './api-error';
+import { BaseError } from './base-error';
 
 export enum HTTPErrorType {
   BAD_REQUEST = 'Bad Request',
@@ -9,24 +10,13 @@ export enum HTTPErrorType {
   INTERNAL_SERVER_ERROR = 'Internal Server Error'
 }
 
-export class HTTPError extends Error {
+export class HTTPError extends BaseError {
   status: number;
-  errors?: (string | object)[];
 
   constructor(name: HTTPErrorType, status: number, message: string, errors?: (string | object)[], stack?: string) {
-    super(message);
+    super(name, message, errors, stack);
 
-    this.name = name;
     this.status = status;
-    this.errors = errors || [];
-
-    if (stack) {
-      this.stack = stack;
-    }
-
-    if (!this.stack) {
-      Error.captureStackTrace(this);
-    }
   }
 }
 
@@ -125,9 +115,24 @@ export const ensureHTTPError = (error: HTTPError | ApiError | Error | any): HTTP
     );
   }
 
+  if (isAjvError(error)) {
+    return new HTTPError(HTTPErrorType.BAD_REQUEST, error.status, 'Request Validation Error', error.errors);
+  }
+
   if (error instanceof Error) {
-    return new HTTP500('Unexpected Error', [error.name, error.message]);
+    return new HTTP500('Unexpected Error', [{ name: error.name, message: error.message }]);
   }
 
-  return new HTTP500('Unexpected Error');
+  return new HTTP500('Unexpected Error', [error]);
+};
+
+/**
+ * Checks if an error object is a AJV validation error.
+ *
+ * @see https://github.com/kogosoftwarellc/open-api/blob/main/packages/openapi-request-validator/index.ts
+ * @param {any} error - Error object
+ * @returns {boolean}
+ */
+export const isAjvError = (error: any): error is { status: number; errors: any[] } => {
+  return typeof error === 'object' && 'status' in error && 'errors' in error;
 };