From 9fad77715b1cf7afde292970bd7cacd1be95d249 Mon Sep 17 00:00:00 2001 From: emizzle Date: Fri, 1 Mar 2019 15:50:58 +1100 Subject: [PATCH] fix(@embark/core): Allow errors in event actions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add the ability to pass an error in the callback of an event action. Currently, event actions could either pass back a an error OR a result, but not both, and with no way to distinguish between the two parameters. This PR adds the ability to pass an error as the first parameter of the action callback. An example of a use case that this fixed: errors on `deployIf` were silently swallowed, which has now been fixed. All instances of `events.request(“deploy:contracts”)` were given an `error` parameter in their callback, and errors printed if it is not undefined. All instances of `registerActionForEvent` were combed to ensure any callbacks were passing the expected error and result. --- packages/embark/src/cmd/cmd_controller.js | 9 ++++++--- packages/embark/src/lib/core/engine.js | 6 +++++- packages/embark/src/lib/core/plugins.js | 8 ++++---- packages/embark/src/lib/modules/deploytracker/index.js | 9 ++++++--- packages/embark/src/lib/modules/specialconfigs/index.js | 6 +++--- packages/embark/src/lib/modules/tests/test.js | 4 ++-- 6 files changed, 26 insertions(+), 16 deletions(-) diff --git a/packages/embark/src/cmd/cmd_controller.js b/packages/embark/src/cmd/cmd_controller.js index 19ddc7d25..b97fa42de 100644 --- a/packages/embark/src/cmd/cmd_controller.js +++ b/packages/embark/src/cmd/cmd_controller.js @@ -136,7 +136,7 @@ class EmbarkController { engine.config.reloadConfig(); engine.events.request('deploy:contracts', function (err) { if (err) { - return; + return engine.logger.error(err.message || err); } engine.logger.info(__('Deployment Done')); }); @@ -251,7 +251,10 @@ class EmbarkController { callback(err, true); }); } - ], function (_err, canExit) { + ], function (err, canExit) { + if(err) { + engine.logger.error(err.message || err); + } // TODO: this should be moved out and determined somewhere else if (canExit || !engine.config.contractsConfig.afterDeploy || !engine.config.contractsConfig.afterDeploy.length) { process.exit(); @@ -562,7 +565,7 @@ class EmbarkController { engine.config.reloadConfig(); engine.events.request('deploy:contracts', function (err) { if (err) { - return; + return engine.logger.error(err.message || err); } engine.logger.info(__('Deployment Done')); }); diff --git a/packages/embark/src/lib/core/engine.js b/packages/embark/src/lib/core/engine.js index 4685e76b9..fe48546ee 100644 --- a/packages/embark/src/lib/core/engine.js +++ b/packages/embark/src/lib/core/engine.js @@ -246,7 +246,11 @@ class Engine { // TODO: for now need to deploy on asset changes as well // because the contractsManager config is corrupted after a deploy if (fileType === 'contract' || fileType === 'config') { - self.events.request('deploy:contracts', () => {}); + self.events.request('deploy:contracts', (err) => { + if (err) { + self.logger.error(err.message || err); + } + }); } }, 50); }); diff --git a/packages/embark/src/lib/core/plugins.js b/packages/embark/src/lib/core/plugins.js index c872cbd71..799190a5b 100644 --- a/packages/embark/src/lib/core/plugins.js +++ b/packages/embark/src/lib/core/plugins.js @@ -163,12 +163,12 @@ Plugins.prototype.runActionsForEvent = function(eventName, args, cb) { async.reduce(actionPlugins, args, function(current_args, plugin, nextEach) { if (typeof (args) === 'function') { - plugin.call(plugin, (params) => { - nextEach(null, (params || current_args)); + plugin.call(plugin, (...params) => { + nextEach(...params || current_args); }); } else { - plugin.call(plugin, args, (params) => { - nextEach(null, (params || current_args)); + plugin.call(plugin, args, (...params) => { + nextEach(...params || current_args); }); } }, cb); diff --git a/packages/embark/src/lib/modules/deploytracker/index.js b/packages/embark/src/lib/modules/deploytracker/index.js index ff25f3a00..f4b7fb146 100644 --- a/packages/embark/src/lib/modules/deploytracker/index.js +++ b/packages/embark/src/lib/modules/deploytracker/index.js @@ -42,7 +42,7 @@ class DeployTracker { self.embark.registerActionForEvent("deploy:contract:shouldDeploy", (params, cb) => { if (!self.trackContracts) { - return cb(params); + return cb(null, params); } let contract = params.contract; @@ -53,7 +53,7 @@ class DeployTracker { if (params.shouldDeploy && trackedContract) { params.shouldDeploy = true; } - cb(params); + cb(null, params); }); } @@ -63,7 +63,10 @@ class DeployTracker { this.currentChain = {contracts: []}; return cb(); } - this.events.request("blockchain:block:byNumber", 0, function(_err, block) { + this.events.request("blockchain:block:byNumber", 0, function(err, block) { + if (err) { + return cb(err); + } let chainId = block.hash; if (self.chainConfig[chainId] === undefined) { diff --git a/packages/embark/src/lib/modules/specialconfigs/index.js b/packages/embark/src/lib/modules/specialconfigs/index.js index 1d5ed30e2..5ca10c45f 100644 --- a/packages/embark/src/lib/modules/specialconfigs/index.js +++ b/packages/embark/src/lib/modules/specialconfigs/index.js @@ -189,14 +189,14 @@ class SpecialConfigs { let cmd = params.contract.deployIf; const contract = params.contract; if (!cmd) { - return cb(params); + return cb(null, params); } if (typeof cmd === 'function') { try { const dependencies = await this.getOnDeployLifecycleHookDependencies(contract); params.shouldDeploy = await contract.deployIf(dependencies); - cb(params); + cb(null, params); } catch (err) { return cb(new Error(`Error when registering deployIf hook for ${contract.name}: ${err.message}`)); } @@ -212,7 +212,7 @@ class SpecialConfigs { params.shouldDeploy = false; } - cb(params); + cb(null, params); }); } }); diff --git a/packages/embark/src/lib/modules/tests/test.js b/packages/embark/src/lib/modules/tests/test.js index f01a68e3c..0392cdadf 100644 --- a/packages/embark/src/lib/modules/tests/test.js +++ b/packages/embark/src/lib/modules/tests/test.js @@ -295,8 +295,8 @@ class Test { }); }, function deploy(accounts, web3, next) { - self.events.request('deploy:contracts:test', () => { - next(null, accounts, web3); + self.events.request('deploy:contracts:test', (err) => { + next(err, accounts, web3); }); }, function waitForProvidersReady(accounts, web3, next) {