- Changed error-handler to return only error message

- Original errors are now available via a secure Symbol - Bumped version to 4.8.0
feathersjs-ecosystem · Feb 1, 2020 · d63a427 · d63a427
1 parent 31bb004
commit d63a427
Show file tree

Hide file tree

Showing 6 changed files with 77 additions and 53 deletions.
diff --git a/README.md b/README.md
@@ -786,7 +786,30 @@ Run the example with `node app` and go to
 You should see an empty array. That's because you don't have any Todos yet, but
 you now have full CRUD for your new todos service!
 
-## Migrating
+## DB migrations
+
+[Knex Migration CLI](http://knexjs.org/#Migrations) can be used to manage DB migrations 
+and to [seed](http://knexjs.org/#Seeds) a table with mock data.
+  
+## Error handling
+
+As of version 4.8.0, `feathers-objection` only throws [Feathers Errors](https://docs.feathersjs.com/api/errors.html) 
+with the message.  
+On the server, the original error can be retrieved through a secure symbol via  `error[require('feathers-objection').ERROR]`.
+
+```js
+const { ERROR } = require('feathers-objection');
+
+try {
+  await objectionService.doSomething();
+} catch (error) {
+  // error is a FeathersError with just the message
+  // Safely retrieve the original error
+  const originalError = error[ERROR];
+}
+```
+
+## Migrating to `feathers-objection` v2
 
 `feathers-objection` 2.0.0 comes with important security and usability updates.
 

diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -1,7 +1,7 @@
 {
   "name": "feathers-objection",
   "description": "A service plugin for ObjectionJS an ORM based on KnexJS",
-  "version": "4.7.0",
+  "version": "4.8.0",
   "homepage": "https://github.com/feathersjs-ecosystem/feathers-objection",
   "keywords": [
     "feathers",

diff --git a/src/error-handler.js b/src/error-handler.js
@@ -1,8 +1,9 @@
 import errors from '@feathersjs/errors';
 
-export default function errorHandler (error) {
-  error = error.nativeError || error;
+const ERROR = Symbol('feathers-knex/error');
 
+export default function errorHandler (error) {
+  const { message } = error.nativeError || error;
   let feathersError = error;
 
   if (error.code === 'SQLITE_ERROR') {
@@ -12,114 +13,102 @@ export default function errorHandler (error) {
       case 18:
       case 19:
       case 20:
-        feathersError = new errors.BadRequest(error);
+        feathersError = new errors.BadRequest(message);
         break;
       case 2:
-        feathersError = new errors.Unavailable(error);
+        feathersError = new errors.Unavailable(message);
         break;
       case 3:
       case 23:
-        feathersError = new errors.Forbidden(error);
+        feathersError = new errors.Forbidden(message);
         break;
       case 12:
-        feathersError = new errors.NotFound(error);
+        feathersError = new errors.NotFound(message);
         break;
       default:
-        feathersError = new errors.GeneralError(error);
+        feathersError = new errors.GeneralError(message);
         break;
     }
-
-    throw feathersError;
-  }
-
-  // Objection validation error
-  if (error.statusCode) {
+  } else if (error.statusCode) { // Objection validation error
     switch (error.statusCode) {
       case 400:
-        feathersError = new errors.BadRequest(error);
+        feathersError = new errors.BadRequest(message);
         break;
 
       case 401:
-        feathersError = new errors.NotAuthenticated(error);
+        feathersError = new errors.NotAuthenticated(message);
         break;
 
       case 402:
-        feathersError = new errors.PaymentError(error);
+        feathersError = new errors.PaymentError(message);
         break;
 
       case 403:
-        feathersError = new errors.Forbidden(error);
+        feathersError = new errors.Forbidden(message);
         break;
 
       case 404:
-        feathersError = new errors.NotFound(error);
+        feathersError = new errors.NotFound(message);
         break;
 
       case 405:
-        feathersError = new errors.MethodNotAllowed(error);
+        feathersError = new errors.MethodNotAllowed(message);
         break;
 
       case 406:
-        feathersError = new errors.NotAcceptable(error);
+        feathersError = new errors.NotAcceptable(message);
         break;
 
       case 408:
-        feathersError = new errors.Timeout(error);
+        feathersError = new errors.Timeout(message);
         break;
 
       case 409:
-        feathersError = new errors.Conflict(error);
+        feathersError = new errors.Conflict(message);
         break;
 
       case 422:
-        feathersError = new errors.Unprocessable(error);
+        feathersError = new errors.Unprocessable(message);
         break;
 
       case 501:
-        feathersError = new errors.NotImplemented(error);
+        feathersError = new errors.NotImplemented(message);
         break;
 
       case 503:
-        feathersError = new errors.Unavailable(error);
+        feathersError = new errors.Unavailable(message);
         break;
 
       case 500:
       default:
-        feathersError = new errors.GeneralError(error);
-    }
-
-    if (error.data) {
-      feathersError.data = error.data;
+        feathersError = new errors.GeneralError(message);
     }
-
-    throw feathersError;
-  }
-
-  // Postgres error code
-  // TODO
-  // Properly detect postgres error
-  if (typeof error.code === 'string') {
+  } else if (typeof error.code === 'string') { // Postgres error code - TODO: Properly detect postgres error
     const pgerror = error.code.substring(0, 2);
 
     switch (pgerror) {
       case '28':
       case '42':
-        feathersError = new errors.Forbidden(error);
+        feathersError = new errors.Forbidden(message);
         break;
 
       case '20':
       case '21':
       case '22':
       case '23':
-        feathersError = new errors.BadRequest(error);
+        feathersError = new errors.BadRequest(message);
         break;
 
       default:
-        feathersError = new errors.GeneralError(error);
+        feathersError = new errors.GeneralError(message);
     }
-
-    throw feathersError;
+  } else if (!(error instanceof errors.FeathersError)) {
+    feathersError = new errors.GeneralError(message);
   }
 
+  feathersError[ERROR] = error;
+
   throw feathersError;
 }
+
+errorHandler.ERROR = ERROR;
diff --git a/src/index.js b/src/index.js
@@ -667,3 +667,4 @@ export default function init (options) {
 }
 
 init.Service = Service;
+init.ERROR = errorHandler.ERROR;
diff --git a/test/index.test.js b/test/index.test.js
@@ -6,7 +6,7 @@ import feathers from '@feathersjs/feathers';
 import knex from 'knex';
 import adapterTests from '@feathersjs/adapter-tests';
 import errors from '@feathersjs/errors';
-import service from '../src';
+import service, { ERROR } from '../src';
 import errorHandler from '../src/error-handler';
 import People from './people';
 import PeopleCustomid from './people-customid';
@@ -316,7 +316,19 @@ describe('Feathers Objection Service', () => {
     it('no error code', () => {
       const error = new Error('Unknown Error');
       expect(errorHandler.bind(null, error)).to.throw('Unknown Error');
-      expect(errorHandler.bind(null, error)).to.not.throw(errors.GeneralError);
+      expect(errorHandler.bind(null, error)).to.throw(errors.GeneralError);
+    });
+
+    it('Get original error', () => {
+      const error = new Error();
+      error.code = 'SQLITE_ERROR';
+      error.errno = 999;
+
+      try {
+        errorHandler(error);
+      } catch (err) {
+        expect(err[ERROR]).to.deep.equal(error);
+      }
     });
 
     describe('SQLite', () => {
@@ -816,7 +828,6 @@ describe('Feathers Objection Service', () => {
         })
         .catch(error => {
           expect(error.code).to.equal(400);
-          expect(error.data).to.deep.equal({});
         });
     });
   });
@@ -1201,7 +1212,7 @@ describe('Feathers Objection Service', () => {
         throw new Error('Should never get here');
       }).catch(function (error) {
         expect(error).to.be.ok;
-        expect(error instanceof errors.BadRequest).to.be.ok;
+        expect(error instanceof errors.GeneralError).to.be.ok;
         expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{numberField}\' AS text) = 1.5 - SQLITE_ERROR: unrecognized token: "#"');
       });
     });
@@ -1211,7 +1222,7 @@ describe('Feathers Objection Service', () => {
         throw new Error('Should never get here');
       }).catch(function (error) {
         expect(error).to.be.ok;
-        expect(error instanceof errors.BadRequest).to.be.ok;
+        expect(error instanceof errors.GeneralError).to.be.ok;
         expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{numberField}\' AS text) > 1.5 - SQLITE_ERROR: unrecognized token: "#"');
       });
     });
@@ -1221,7 +1232,7 @@ describe('Feathers Objection Service', () => {
         throw new Error('Should never get here');
       }).catch(function (error) {
         expect(error).to.be.ok;
-        expect(error instanceof errors.BadRequest).to.be.ok;
+        expect(error instanceof errors.GeneralError).to.be.ok;
         expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonObject`#>>\'{objectField,object}\' AS text) = \'string in jsonObject.objectField.object\' - SQLITE_ERROR: unrecognized token: "#"');
       });
     });
@@ -1237,7 +1248,7 @@ describe('Feathers Objection Service', () => {
         throw new Error('Should never get here');
       }).catch(function (error) {
         expect(error).to.be.ok;
-        expect(error instanceof errors.BadRequest).to.be.ok;
+        expect(error instanceof errors.GeneralError).to.be.ok;
         expect(error.message).to.equal('select `companies`.* from `companies` where CAST(`companies`.`jsonArray`#>>\'{0,objectField,object}\' AS text) = \'I\'\'m string in jsonArray[0].objectField.object\' - SQLITE_ERROR: unrecognized token: "#"');
       });
     });