return more info on missing parameters

lib/common/utils.js

@@ -7,13 +7,13 @@ var encoding = Bitcore.encoding;
 
 var Utils = {};
 
-Utils.checkRequired = function(obj, args) {
+Utils.getMissingFields = function(obj, args) {
   args = [].concat(args);
-  if (!_.isObject(obj)) return false;
+  if (!_.isObject(obj)) return args;
-  for (var i = 0; i < args.length; i++) {
+  var missing = _.filter(args, function(arg) {
-    if (!obj.hasOwnProperty(args[i])) return false;
+    return !obj.hasOwnProperty(arg);
-  }
+  });
-  return true;
+  return missing;
 };
 
 /**

lib/server.js

@@ -59,6 +59,12 @@ function WalletService() {
   this.notifyTicker = 0;
 };
 
+function checkRequired(obj, args, cb) {
+  var missing = Utils.getMissingFields(obj, args);
+  if (_.isEmpty(missing)) return true;
+  cb(new ClientError('Required argument ' + _.first(missing) + ' missing.'));
+  return false;
+};
 
 /**
  * Gets the current version of BWS
@@ -177,8 +183,7 @@ WalletService.getInstance = function(opts) {
  * @param {string} opts.clientVersion - A string that identifies the client issuing the request
  */
 WalletService.getInstanceWithAuth = function(opts, cb) {
-  if (!Utils.checkRequired(opts, ['copayerId', 'message', 'signature']))
+  if (!checkRequired(opts, ['copayerId', 'message', 'signature'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   var server = new WalletService();
   server.storage.fetchCopayerLookup(opts.copayerId, function(err, copayer) {
@@ -217,8 +222,7 @@ WalletService.prototype.createWallet = function(opts, cb) {
   var self = this,
     pubKey;
 
-  if (!Utils.checkRequired(opts, ['name', 'm', 'n', 'pubKey']))
+  if (!checkRequired(opts, ['name', 'm', 'n', 'pubKey'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   if (_.isEmpty(opts.name)) return cb(new ClientError('Invalid wallet name'));
   if (!Wallet.verifyCopayerLimits(opts.m, opts.n))
@@ -512,8 +516,7 @@ WalletService.prototype._addKeyToCopayer = function(wallet, copayer, opts, cb) {
 WalletService.prototype.addAccess = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['copayerId', 'requestPubKey', 'signature']))
+  if (!checkRequired(opts, ['copayerId', 'requestPubKey', 'signature'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self.storage.fetchCopayerLookup(opts.copayerId, function(err, copayer) {
     if (err) return cb(err);
@@ -612,8 +615,7 @@ WalletService._getCopayerHash = function(name, xPubKey, requestPubKey) {
 WalletService.prototype.joinWallet = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['walletId', 'name', 'xPubKey', 'requestPubKey', 'copayerSignature']))
+  if (!checkRequired(opts, ['walletId', 'name', 'xPubKey', 'requestPubKey', 'copayerSignature'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   if (_.isEmpty(opts.name))
     return cb(new ClientError('Invalid copayer name'));
@@ -841,8 +843,7 @@ WalletService.prototype.getMainAddresses = function(opts, cb) {
 WalletService.prototype.verifyMessageSignature = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['message', 'signature']))
+  if (!checkRequired(opts, ['message', 'signature'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self.getWallet({}, function(err, wallet) {
     if (err) return cb(err);
@@ -1164,8 +1165,7 @@ WalletService.prototype.getSendMaxInfo = function(opts, cb) {
 
   opts = opts || {};
 
-  if (!Utils.checkRequired(opts, ['feePerKb']))
+  if (!checkRequired(opts, ['feePerKb'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self.getWallet({}, function(err, wallet) {
     if (err) return cb(err);
@@ -1633,14 +1633,12 @@ WalletService.prototype._canCreateTx = function(cb) {
   });
 };
 
-WalletService.prototype._validateOutputs = function(opts, wallet) {
+WalletService.prototype._validateOutputs = function(opts, wallet, cb) {
   for (var i = 0; i < opts.outputs.length; i++) {
     var output = opts.outputs[i];
     output.valid = false;
 
-    if (!Utils.checkRequired(output, ['toAddress', 'amount'])) {
+    if (!checkRequired(output, ['toAddress', 'amount'], cb)) return;
-      return new ClientError('Required outputs argument missing');
-    }
 
     var toAddress = {};
     try {
@@ -1700,8 +1698,7 @@ WalletService.prototype.createTxLegacy = function(opts, cb) {
   }
   opts.outputs = [].concat(opts.outputs);
 
-  if (!Utils.checkRequired(opts, ['outputs', 'proposalSignature']))
+  if (!checkRequired(opts, ['outputs', 'proposalSignature'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   var type = opts.type || Model.TxProposalLegacy.Types.SIMPLE;
   if (!Model.TxProposalLegacy.isTypeSupported(type))
@@ -1747,7 +1744,7 @@ WalletService.prototype.createTxLegacy = function(opts, cb) {
         if (!canCreate) return cb(Errors.TX_CANNOT_CREATE);
 
         if (type != Model.TxProposalLegacy.Types.EXTERNAL) {
-          var validationError = self._validateOutputs(opts, wallet);
+          var validationError = self._validateOutputs(opts, wallet, cb);
           if (validationError) {
             return cb(validationError);
           }
@@ -1816,8 +1813,7 @@ WalletService.prototype._validateAndSanitizeTxOpts = function(wallet, opts, cb)
   async.series([
 
     function(next) {
-      if (!Utils.checkRequired(opts, ['outputs']))
+      if (!checkRequired(opts, ['outputs'], next)) return;
-        return next(new ClientError('Required argument missing'));
       next();
     },
     function(next) {
@@ -1854,7 +1850,7 @@ WalletService.prototype._validateAndSanitizeTxOpts = function(wallet, opts, cb)
     },
     function(next) {
       if (opts.validateOutputs === false) return next();
-      var validationError = self._validateOutputs(opts, wallet);
+      var validationError = self._validateOutputs(opts, wallet, next);
       if (validationError) {
         return next(validationError);
       }
@@ -1970,8 +1966,7 @@ WalletService.prototype.publishTx = function(opts, cb) {
     return utxo.txid + '|' + utxo.vout
   };
 
-  if (!Utils.checkRequired(opts, ['txProposalId', 'proposalSignature']))
+  if (!checkRequired(opts, ['txProposalId', 'proposalSignature'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self._runLocked(cb, function(cb) {
     self.getWallet({}, function(err, wallet) {
@@ -2087,8 +2082,7 @@ WalletService.prototype.getRemainingDeleteLockTime = function(txp) {
 WalletService.prototype.removePendingTx = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['txProposalId']))
+  if (!checkRequired(opts, ['txProposalId'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self._runLocked(cb, function(cb) {
 
@@ -2126,8 +2120,7 @@ WalletService.prototype._broadcastRawTx = function(network, raw, cb) {
 WalletService.prototype.broadcastRawTx = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['network', 'rawTx']))
+  if (!checkRequired(opts, ['network', 'rawTx'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   var network = opts.network || 'livenet';
   if (network != 'livenet' && network != 'testnet')
@@ -2155,8 +2148,7 @@ WalletService.prototype._checkTxInBlockchain = function(txp, cb) {
 WalletService.prototype.signTx = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['txProposalId', 'signatures']))
+  if (!checkRequired(opts, ['txProposalId', 'signatures'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self.getWallet({}, function(err, wallet) {
     if (err) return cb(err);
@@ -2254,8 +2246,7 @@ WalletService.prototype._processBroadcast = function(txp, opts, cb) {
 WalletService.prototype.broadcastTx = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['txProposalId']))
+  if (!checkRequired(opts, ['txProposalId'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self.getWallet({}, function(err, wallet) {
     if (err) return cb(err);
@@ -2308,8 +2299,7 @@ WalletService.prototype.broadcastTx = function(opts, cb) {
 WalletService.prototype.rejectTx = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['txProposalId']))
+  if (!checkRequired(opts, ['txProposalId'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self.getTx({
     txProposalId: opts.txProposalId
@@ -2790,8 +2780,7 @@ WalletService.prototype.startScan = function(opts, cb) {
 WalletService.prototype.getFiatRate = function(opts, cb) {
   var self = this;
 
-  if (!Utils.checkRequired(opts, ['code']))
+  if (!checkRequired(opts, ['code'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   self.fiatRateService.getRate(opts, function(err, rate) {
     if (err) return cb(err);
@@ -2800,8 +2789,7 @@ WalletService.prototype.getFiatRate = function(opts, cb) {
 };
 
 WalletService.prototype.pushNotificationsSubscribe = function(opts, cb) {
-  if (!Utils.checkRequired(opts, ['token']))
+  if (!checkRequired(opts, ['token'], cb)) return;
-    return cb(new ClientError('Required argument missing'));
 
   var self = this;
 

test/integration/server.js

@@ -509,7 +509,7 @@ describe('Wallet service', function() {
         };
         server.joinWallet(copayerOpts, function(err) {
           should.exist(err);
-          err.message.should.contain('argument missing');
+          err.message.should.contain('argument copayerSignature missing');
           done();
         });
       });

test/utils.js

@@ -7,7 +7,7 @@ var should = chai.should();
 var Utils = require('../lib/common/utils');
 
 describe('Utils', function() {
-  describe('#checkRequired', function() {
+  describe('#getMissingFields', function() {
     it('should check required fields', function() {
       var obj = {
         id: 'id',
@@ -16,36 +16,36 @@ describe('Utils', function() {
       };
       var fixtures = [{
         args: 'id',
-        check: true
+        check: [],
       }, {
         args: ['id'],
-        check: true
+        check: []
       }, {
         args: ['id, name'],
-        check: false
+        check: ['id, name'],
       }, {
         args: ['id', 'name'],
-        check: true
+        check: []
       }, {
         args: 'array',
-        check: true
+        check: []
       }, {
         args: 'dummy',
-        check: false
+        check: ['dummy']
       }, {
         args: ['dummy1', 'dummy2'],
-        check: false
+        check: ['dummy1', 'dummy2']
       }, {
         args: ['id', 'dummy'],
-        check: false
+        check: ['dummy']
       }, ];
       _.each(fixtures, function(f) {
-        Utils.checkRequired(obj, f.args).should.equal(f.check);
+        Utils.getMissingFields(obj, f.args).should.deep.equal(f.check);
       });
     });
     it('should fail to check required fields on non-object', function() {
       var obj = 'dummy';
-      Utils.checkRequired(obj, 'name').should.be.false;
+      Utils.getMissingFields(obj, 'name').should.deep.equal(['name']);
     });
   });