From 8e7fa61466d3e0c010daeb6a2c48e9f109405aac Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 4 Feb 2015 12:46:31 -0300 Subject: [PATCH 1/4] refactor errors --- lib/server.js | 40 +++++++++++++++++++++++++--------------- test/integration.js | 16 +++++++++------- 2 files changed, 34 insertions(+), 22 deletions(-) diff --git a/lib/server.js b/lib/server.js index 511635f..81b732b 100644 --- a/lib/server.js +++ b/lib/server.js @@ -23,6 +23,16 @@ var Address = require('./model/address'); var TxProposal = require('./model/txproposal'); +function CopayError(code, message, inner) { + this.code = code; + this.message = message; + this.inner = inner; +}; + +function BadRequestError(message) { + this.message = message || 'Bad request'; +}; + /** * Creates an instance of the Copay server. * @constructor @@ -57,9 +67,9 @@ CopayServer.prototype.createWallet = function(opts, cb) { pubKey; Utils.checkRequired(opts, ['id', 'name', 'm', 'n', 'pubKey']); - if (!Wallet.verifyCopayerLimits(opts.m, opts.n)) return cb('Invalid m/n combination'); + if (!Wallet.verifyCopayerLimits(opts.m, opts.n)) return cb(new BadRequestError('Invalid combination of required copayers / total copayers')); var network = opts.network || 'livenet'; - if (network != 'livenet' && network != 'testnet') return cb('Invalid network'); + if (network != 'livenet' && network != 'testnet') return cb(new BadRequestError('Invalid network')); try { pubKey = new PublicKey.fromString(opts.pubKey); @@ -69,7 +79,7 @@ CopayServer.prototype.createWallet = function(opts, cb) { self.storage.fetchWallet(opts.id, function(err, wallet) { if (err) return cb(err); - if (wallet) return cb('Wallet already exists'); + if (wallet) return cb(new CopayError('WEXISTS', 'Wallet already exists')); var wallet = new Wallet({ id: opts.id, @@ -95,7 +105,7 @@ CopayServer.prototype.getWallet = function(opts, cb) { self.storage.fetchWallet(opts.id, function(err, wallet) { if (err) return cb(err); - if (!wallet) return cb('Wallet not found'); + if (!wallet) return cb(new BadRequestError('Wallet not found')); return cb(null, wallet); }); }; @@ -132,13 +142,13 @@ CopayServer.prototype.joinWallet = function(opts, cb) { if (err) return cb(err); if (!self._verifySignature(opts.xPubKey, opts.xPubKeySignature, wallet.pubKey)) { - return cb('Bad request'); + return cb(new BadRequestError()); } if (_.find(wallet.copayers, { xPubKey: opts.xPubKey - })) return cb('Copayer already in wallet'); - if (wallet.copayers.length == wallet.n) return cb('Wallet full'); + })) return cb(new CopayError('CINWALLET', 'Copayer already in wallet')); + if (wallet.copayers.length == wallet.n) return cb(new CopayError('WFULL', 'Wallet full')); var copayer = new Copayer({ id: opts.id, @@ -234,7 +244,7 @@ CopayServer.prototype.verifyMessageSignature = function(opts, cb) { if (err) return cb(err); var copayer = wallet.getCopayer(opts.copayerId); - if (!copayer) return cb('Copayer not found'); + if (!copayer) return cb(new BadRequestError('Copayer not found')); var isValid = self._verifySignature(opts.message, opts.signature, copayer.signingPubKey); return cb(null, isValid); @@ -268,7 +278,7 @@ CopayServer.prototype._getUtxos = function(opts, cb) { // Get addresses for this wallet self.storage.fetchAddresses(opts.walletId, function(err, addresses) { if (err) return cb(err); - if (addresses.length == 0) return cb('The wallet has no addresses'); + if (addresses.length == 0) return cb(new BadRequestError('The wallet has no addresses')); var addresses = _.pluck(addresses, 'address'); @@ -453,12 +463,12 @@ CopayServer.prototype.signTx = function(opts, cb) { self.fetchTx(opts.walletId, opts.txProposalId, function(err, txp) { if (err) return cb(err); - if (!txp) return cb('Transaction proposal not found'); + if (!txp) return cb(new BadRequestError('Transaction proposal not found')); var action = _.find(txp.actions, { copayerId: opts.copayerId }); - if (action) return cb('Copayer already voted on this transaction proposal'); - if (txp.status != 'pending') return cb('The transaction proposal is not pending'); + if (action) return cb(new CopayError('CVOTED', 'Copayer already voted on this transaction proposal')); + if (txp.status != 'pending') return cb(new CopayError('TXNOTPENDING', 'The transaction proposal is not pending')); txp.sign(opts.copayerId, opts.signature); @@ -495,12 +505,12 @@ CopayServer.prototype.rejectTx = function(opts, cb) { self.fetchTx(opts.walletId, opts.txProposalId, function(err, txp) { if (err) return cb(err); - if (!txp) return cb('Transaction proposal not found'); + if (!txp) return cb(new BadRequestError('Transaction proposal not found')); var action = _.find(txp.actions, { copayerId: opts.copayerId }); - if (action) return cb('Copayer already voted on this transaction proposal'); - if (txp.status != 'pending') return cb('The transaction proposal is not pending'); + if (action) return cb(new CopayError('CVOTED', 'Copayer already voted on this transaction proposal')); + if (txp.status != 'pending') return cb(new CopayError('TXNOTPENDING', 'The transaction proposal is not pending')); txp.reject(opts.copayerId); diff --git a/test/integration.js b/test/integration.js index 03f1640..27068aa 100644 --- a/test/integration.js +++ b/test/integration.js @@ -218,7 +218,7 @@ describe('Copay server', function() { id: '345' }, function(err, wallet) { should.exist(err); - err.should.equal('Wallet not found'); + err.message.should.equal('Wallet not found'); done(); }); }); @@ -306,7 +306,7 @@ describe('Copay server', function() { opts.n = pair.n; server.createWallet(opts, function(err) { should.exist(err); - err.should.contain('Invalid m/n combination'); + err.message.should.equal('Invalid combination of required copayers / total copayers'); return cb(); }); }, function(err) { @@ -413,7 +413,8 @@ describe('Copay server', function() { wallet.status.should.equal('complete'); server.joinWallet(copayer2Opts, function(err) { should.exist(err); - err.should.equal('Wallet full'); + err.code.should.equal('WFULL'); + err.message.should.equal('Wallet full'); done(); }); }); @@ -442,7 +443,8 @@ describe('Copay server', function() { should.not.exist(err); server.joinWallet(copayerOpts, function(err) { should.exist(err); - err.should.equal('Copayer already in wallet'); + err.code.should.equal('CINWALLET'); + err.message.should.equal('Copayer already in wallet'); done(); }); }); @@ -468,7 +470,7 @@ describe('Copay server', function() { xPubKeySignature: 'bad sign', }; server.joinWallet(copayerOpts, function(err) { - err.should.contain('Bad request'); + err.message.should.equal('Bad request'); done(); }); }); @@ -518,7 +520,7 @@ describe('Copay server', function() { xPubKeySignature: someXPubKeysSignatures[0], }; server.joinWallet(copayerOpts, function(err) { - err.should.contain('Bad request'); + err.message.should.equal('Bad request'); done(); }); }); @@ -571,7 +573,7 @@ describe('Copay server', function() { signature: 'dummy', }; server.verifyMessageSignature(opts, function(err, isValid) { - err.should.equal('Copayer not found'); + err.message.should.equal('Copayer not found'); done(); }); }); From fdd3b831c038c1aeba04c867532e36b5577bf0ca Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 4 Feb 2015 12:50:23 -0300 Subject: [PATCH 2/4] fix insufficient funds error msg & tests --- lib/server.js | 5 +++-- test/integration.js | 6 ++++-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/server.js b/lib/server.js index 81b732b..78fc0ea 100644 --- a/lib/server.js +++ b/lib/server.js @@ -427,8 +427,9 @@ CopayServer.prototype.createTx = function(opts, cb) { }); txp.inputs = self._selectUtxos(txp, utxos); - if (!txp.inputs) - return cb('insufficient funds') + if (!txp.inputs) { + return cb(new CopayError('INSUFFICIENTFUNDS', 'Insufficient funds')); + } // no need to do this now: // TODO remove this comment //self._createRawTx(txp); diff --git a/test/integration.js b/test/integration.js index 27068aa..d631f98 100644 --- a/test/integration.js +++ b/test/integration.js @@ -778,7 +778,8 @@ describe('Copay server', function() { }; server.createTx(txOpts, function(err, tx) { - err.should.contain('insufficient'); + err.code.should.equal('INSUFFICIENTFUNDS'); + err.message.should.equal('Insufficient funds'); server.getPendingTxs({ walletId: '123' }, function(err, txs) { @@ -879,7 +880,8 @@ describe('Copay server', function() { requestSignature: 'dummy', }; server.createTx(txOpts2, function(err, tx) { - err.should.contain('insufficient'); + err.code.should.equal('INSUFFICIENTFUNDS'); + err.message.should.equal('Insufficient funds'); should.not.exist(tx); server.getPendingTxs({ walletId: '123' From 283eca4e12436ea9f24f0034dbce5213c1e66ad4 Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 4 Feb 2015 13:08:25 -0300 Subject: [PATCH 3/4] rename BadRequestError -> RequestError --- lib/server.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/server.js b/lib/server.js index 78fc0ea..1a3639c 100644 --- a/lib/server.js +++ b/lib/server.js @@ -29,7 +29,7 @@ function CopayError(code, message, inner) { this.inner = inner; }; -function BadRequestError(message) { +function RequestError(message) { this.message = message || 'Bad request'; }; @@ -67,9 +67,9 @@ CopayServer.prototype.createWallet = function(opts, cb) { pubKey; Utils.checkRequired(opts, ['id', 'name', 'm', 'n', 'pubKey']); - if (!Wallet.verifyCopayerLimits(opts.m, opts.n)) return cb(new BadRequestError('Invalid combination of required copayers / total copayers')); + if (!Wallet.verifyCopayerLimits(opts.m, opts.n)) return cb(new RequestError('Invalid combination of required copayers / total copayers')); var network = opts.network || 'livenet'; - if (network != 'livenet' && network != 'testnet') return cb(new BadRequestError('Invalid network')); + if (network != 'livenet' && network != 'testnet') return cb(new RequestError('Invalid network')); try { pubKey = new PublicKey.fromString(opts.pubKey); @@ -105,7 +105,7 @@ CopayServer.prototype.getWallet = function(opts, cb) { self.storage.fetchWallet(opts.id, function(err, wallet) { if (err) return cb(err); - if (!wallet) return cb(new BadRequestError('Wallet not found')); + if (!wallet) return cb(new RequestError('Wallet not found')); return cb(null, wallet); }); }; @@ -142,7 +142,7 @@ CopayServer.prototype.joinWallet = function(opts, cb) { if (err) return cb(err); if (!self._verifySignature(opts.xPubKey, opts.xPubKeySignature, wallet.pubKey)) { - return cb(new BadRequestError()); + return cb(new RequestError()); } if (_.find(wallet.copayers, { @@ -244,7 +244,7 @@ CopayServer.prototype.verifyMessageSignature = function(opts, cb) { if (err) return cb(err); var copayer = wallet.getCopayer(opts.copayerId); - if (!copayer) return cb(new BadRequestError('Copayer not found')); + if (!copayer) return cb(new RequestError('Copayer not found')); var isValid = self._verifySignature(opts.message, opts.signature, copayer.signingPubKey); return cb(null, isValid); @@ -278,7 +278,7 @@ CopayServer.prototype._getUtxos = function(opts, cb) { // Get addresses for this wallet self.storage.fetchAddresses(opts.walletId, function(err, addresses) { if (err) return cb(err); - if (addresses.length == 0) return cb(new BadRequestError('The wallet has no addresses')); + if (addresses.length == 0) return cb(new RequestError('The wallet has no addresses')); var addresses = _.pluck(addresses, 'address'); @@ -464,7 +464,7 @@ CopayServer.prototype.signTx = function(opts, cb) { self.fetchTx(opts.walletId, opts.txProposalId, function(err, txp) { if (err) return cb(err); - if (!txp) return cb(new BadRequestError('Transaction proposal not found')); + if (!txp) return cb(new RequestError('Transaction proposal not found')); var action = _.find(txp.actions, { copayerId: opts.copayerId }); @@ -506,7 +506,7 @@ CopayServer.prototype.rejectTx = function(opts, cb) { self.fetchTx(opts.walletId, opts.txProposalId, function(err, txp) { if (err) return cb(err); - if (!txp) return cb(new BadRequestError('Transaction proposal not found')); + if (!txp) return cb(new RequestError('Transaction proposal not found')); var action = _.find(txp.actions, { copayerId: opts.copayerId }); From b5e089bd183e4d1653092f01e6a2ec141a977dab Mon Sep 17 00:00:00 2001 From: Ivan Socolsky Date: Wed, 4 Feb 2015 13:31:02 -0300 Subject: [PATCH 4/4] replace all errors with ClientError --- lib/clienterror.js | 21 +++++++++++++++++++++ lib/server.js | 43 +++++++++++++++++-------------------------- 2 files changed, 38 insertions(+), 26 deletions(-) create mode 100644 lib/clienterror.js diff --git a/lib/clienterror.js b/lib/clienterror.js new file mode 100644 index 0000000..2e6fd84 --- /dev/null +++ b/lib/clienterror.js @@ -0,0 +1,21 @@ +function ClientError() { + var args = Array.prototype.slice.call(arguments); + + switch (args.length) { + case 0: + this.code = 'BADREQUEST'; + this.message = 'Bad request'; + break; + case 1: + this.code = 'BADREQUEST'; + this.message = args[0]; + break; + default: + case 2: + this.code = args[0]; + this.message = args[1]; + break; + } +}; + +module.exports = ClientError; diff --git a/lib/server.js b/lib/server.js index 1a3639c..edf7834 100644 --- a/lib/server.js +++ b/lib/server.js @@ -13,6 +13,7 @@ var PublicKey = Bitcore.PublicKey; var HDPublicKey = Bitcore.HDPublicKey; var Explorers = require('bitcore-explorers'); +var ClientError = require('./clienterror'); var Utils = require('./utils'); var Storage = require('./storage'); var SignUtils = require('./signutils'); @@ -23,16 +24,6 @@ var Address = require('./model/address'); var TxProposal = require('./model/txproposal'); -function CopayError(code, message, inner) { - this.code = code; - this.message = message; - this.inner = inner; -}; - -function RequestError(message) { - this.message = message || 'Bad request'; -}; - /** * Creates an instance of the Copay server. * @constructor @@ -67,9 +58,9 @@ CopayServer.prototype.createWallet = function(opts, cb) { pubKey; Utils.checkRequired(opts, ['id', 'name', 'm', 'n', 'pubKey']); - if (!Wallet.verifyCopayerLimits(opts.m, opts.n)) return cb(new RequestError('Invalid combination of required copayers / total copayers')); + if (!Wallet.verifyCopayerLimits(opts.m, opts.n)) return cb(new ClientError('Invalid combination of required copayers / total copayers')); var network = opts.network || 'livenet'; - if (network != 'livenet' && network != 'testnet') return cb(new RequestError('Invalid network')); + if (network != 'livenet' && network != 'testnet') return cb(new ClientError('Invalid network')); try { pubKey = new PublicKey.fromString(opts.pubKey); @@ -79,7 +70,7 @@ CopayServer.prototype.createWallet = function(opts, cb) { self.storage.fetchWallet(opts.id, function(err, wallet) { if (err) return cb(err); - if (wallet) return cb(new CopayError('WEXISTS', 'Wallet already exists')); + if (wallet) return cb(new ClientError('WEXISTS', 'Wallet already exists')); var wallet = new Wallet({ id: opts.id, @@ -105,7 +96,7 @@ CopayServer.prototype.getWallet = function(opts, cb) { self.storage.fetchWallet(opts.id, function(err, wallet) { if (err) return cb(err); - if (!wallet) return cb(new RequestError('Wallet not found')); + if (!wallet) return cb(new ClientError('Wallet not found')); return cb(null, wallet); }); }; @@ -142,13 +133,13 @@ CopayServer.prototype.joinWallet = function(opts, cb) { if (err) return cb(err); if (!self._verifySignature(opts.xPubKey, opts.xPubKeySignature, wallet.pubKey)) { - return cb(new RequestError()); + return cb(new ClientError()); } if (_.find(wallet.copayers, { xPubKey: opts.xPubKey - })) return cb(new CopayError('CINWALLET', 'Copayer already in wallet')); - if (wallet.copayers.length == wallet.n) return cb(new CopayError('WFULL', 'Wallet full')); + })) return cb(new ClientError('CINWALLET', 'Copayer already in wallet')); + if (wallet.copayers.length == wallet.n) return cb(new ClientError('WFULL', 'Wallet full')); var copayer = new Copayer({ id: opts.id, @@ -244,7 +235,7 @@ CopayServer.prototype.verifyMessageSignature = function(opts, cb) { if (err) return cb(err); var copayer = wallet.getCopayer(opts.copayerId); - if (!copayer) return cb(new RequestError('Copayer not found')); + if (!copayer) return cb(new ClientError('Copayer not found')); var isValid = self._verifySignature(opts.message, opts.signature, copayer.signingPubKey); return cb(null, isValid); @@ -278,7 +269,7 @@ CopayServer.prototype._getUtxos = function(opts, cb) { // Get addresses for this wallet self.storage.fetchAddresses(opts.walletId, function(err, addresses) { if (err) return cb(err); - if (addresses.length == 0) return cb(new RequestError('The wallet has no addresses')); + if (addresses.length == 0) return cb(new ClientError('The wallet has no addresses')); var addresses = _.pluck(addresses, 'address'); @@ -428,7 +419,7 @@ CopayServer.prototype.createTx = function(opts, cb) { txp.inputs = self._selectUtxos(txp, utxos); if (!txp.inputs) { - return cb(new CopayError('INSUFFICIENTFUNDS', 'Insufficient funds')); + return cb(new ClientError('INSUFFICIENTFUNDS', 'Insufficient funds')); } // no need to do this now: // TODO remove this comment @@ -464,12 +455,12 @@ CopayServer.prototype.signTx = function(opts, cb) { self.fetchTx(opts.walletId, opts.txProposalId, function(err, txp) { if (err) return cb(err); - if (!txp) return cb(new RequestError('Transaction proposal not found')); + if (!txp) return cb(new ClientError('Transaction proposal not found')); var action = _.find(txp.actions, { copayerId: opts.copayerId }); - if (action) return cb(new CopayError('CVOTED', 'Copayer already voted on this transaction proposal')); - if (txp.status != 'pending') return cb(new CopayError('TXNOTPENDING', 'The transaction proposal is not pending')); + if (action) return cb(new ClientError('CVOTED', 'Copayer already voted on this transaction proposal')); + if (txp.status != 'pending') return cb(new ClientError('TXNOTPENDING', 'The transaction proposal is not pending')); txp.sign(opts.copayerId, opts.signature); @@ -506,12 +497,12 @@ CopayServer.prototype.rejectTx = function(opts, cb) { self.fetchTx(opts.walletId, opts.txProposalId, function(err, txp) { if (err) return cb(err); - if (!txp) return cb(new RequestError('Transaction proposal not found')); + if (!txp) return cb(new ClientError('Transaction proposal not found')); var action = _.find(txp.actions, { copayerId: opts.copayerId }); - if (action) return cb(new CopayError('CVOTED', 'Copayer already voted on this transaction proposal')); - if (txp.status != 'pending') return cb(new CopayError('TXNOTPENDING', 'The transaction proposal is not pending')); + if (action) return cb(new ClientError('CVOTED', 'Copayer already voted on this transaction proposal')); + if (txp.status != 'pending') return cb(new ClientError('TXNOTPENDING', 'The transaction proposal is not pending')); txp.reject(opts.copayerId);