From bfc10b1ddfebe34b8a751f977eed4f03c1fc4b41 Mon Sep 17 00:00:00 2001
From: David Luecke <daff@neyeon.com>
Date: Thu, 8 Feb 2018 15:36:38 -0800
Subject: [PATCH] Turn argument validation into the first hook to make
 consistent responses

---
 lib/hooks.js             | 17 ++++++++---------
 test/hooks/hooks.test.js | 15 +++++++++++++++
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/lib/hooks.js b/lib/hooks.js
index 1d8d9f9703..9596360dfb 100644
--- a/lib/hooks.js
+++ b/lib/hooks.js
@@ -35,14 +35,6 @@ const hookMixin = exports.hookMixin = function hookMixin (service) {
       const returnHook = args[args.length - 1] === true
         ? args.pop() : false;
 
-      // We have to try/catch this so that argument validation
-      // returns a rejected promise
-      try {
-        validateArguments(method, args);
-      } catch (e) {
-        return Promise.reject(e);
-      }
-
       // A reference to the original method
       const _super = service._super.bind(service);
       // Create the hook object that gets passed through
@@ -51,7 +43,14 @@ const hookMixin = exports.hookMixin = function hookMixin (service) {
         service,
         app
       });
-      const beforeHooks = getHooks(app, service, 'before', method);
+      // A hook that validates the arguments and will always be the very first
+      const validateHook = context => {
+        validateArguments(method, args);
+
+        return context;
+      };
+      // The `before` hook chain (including the validation hook)
+      const beforeHooks = [ validateHook, ...getHooks(app, service, 'before', method) ];
 
       // Process all before hooks
       return processHooks.call(service, beforeHooks, hookObject)
diff --git a/test/hooks/hooks.test.js b/test/hooks/hooks.test.js
index 3e483b0be6..d361d6ac32 100644
--- a/test/hooks/hooks.test.js
+++ b/test/hooks/hooks.test.js
@@ -182,6 +182,21 @@ describe('hooks basics', () => {
       });
     });
 
+    it('on argument validation error', () => {
+      const app = feathers().use('/dummy', {
+        get (id) {
+          return Promise.resolve({ id });
+        }
+      });
+
+      return app.service('dummy').get(undefined, {}, true).catch(context => {
+        assert.equal(context.service, app.service('dummy'));
+        assert.equal(context.type, 'error');
+        assert.equal(context.path, 'dummy');
+        assert.equal(context.error.message, 'An id must be provided to the \'get\' method');
+      });
+    });
+
     it('still swallows error if context.result is set', () => {
       const result = { message: 'this is a test' };
       const app = feathers().use('/dummy', {