Skip to content
This repository
Browse code

Issue #1, #2: Complete feature to introduce "error continuing" policy…

…, with the ability for individual error handlers to override that policy. Also, fix bug originally blocking this feature. This bug entailed the individual error handlers not being called for corresponding commands.
  • Loading branch information...
commit 0a8897c8edb1b042f87ecb5f8165c831e82f7a1a 1 parent f5ce47c
Ryan Self authored September 28, 2012
41  main.js
@@ -24,12 +24,27 @@ var finish = function (execPlan) {
24 24
  * @param errorHandler Function
25 25
  *                       @param error Error
26 26
  *                       @param stderr String
  27
+ * @return Boolean - whether the plan should continue to execute
27 28
  */
28 29
 var handleError = function (execPlan, error, stderr, errorHandler) {
29  
-    if (!utils.isFunction(errorHandler) || (errorHandler(error, stderr) !== false)) {
30  
-        // fire event if errorHandler allows us
  30
+    var shouldContinue      = execPlan.continuesOnError(); // default to using configured policy
  31
+    var errorHandlerDefined = utils.isFunction(errorHandler);
  32
+    var errorHandlerReturn;
  33
+    var shouldOverridePolicy; // whether the given error handler is overriding the
  34
+                              // "continue on error" policy already set on the exec plan
  35
+    
  36
+    // determine whether the "continue on error" policy should be overridden by the given error
  37
+    // error handler. this is determined by whether the given error handler returns either true
  38
+    // or false. any other return, e.g., undefined, will signify that the policy should be used.
  39
+    if (errorHandlerDefined) errorHandlerReturn = errorHandler(error, stderr);
  40
+    shouldOverridePolicy = ((errorHandlerReturn === true) || (errorHandlerReturn === false));
  41
+    if (shouldOverridePolicy) {
  42
+        shouldContinue = errorHandlerReturn;
  43
+    } else {  // not overriding the policy has the side-effect of allowing 'execerror' to fire
31 44
         execPlan.emit('execerror', error, stderr);
32 45
     }
  46
+
  47
+    return shouldContinue;
33 48
 };
34 49
 
35 50
 /**
@@ -72,19 +87,26 @@ var makeFinalFn = (function (execPlan, errorHandler) {
72 87
  */
73 88
 var makeStep = function (execPlan, first, preLogic, command, options, errorHandler, nextStep) {
74 89
     return function (error, stdout, stderr) {
  90
+        var shouldContinue; // whether the plan should continue executing beyond this step
  91
+
75 92
         // log stdout/err
76 93
         if (!first) {  // a previous step's stdout/err is available
77 94
             if (execPlan.willAutoPrintOut())           console.log(stdout);
78 95
             if (error && execPlan.willAutoPrintErr())  console.error(stderr);
79 96
         }
80 97
 
81  
-        if (error) {  // error occurred during this step
82  
-            handleError(execPlan, error, stderr, errorHandler);
83  
-            finish(execPlan);
84  
-        } else {  // this step successfully executed
  98
+        // continue execution plan, unless it should be stopped, as determined by, e.g.,
  99
+        // the error handler.
  100
+        shouldContinue = (!error || handleError(execPlan, error, stderr, errorHandler));
  101
+
  102
+        // BUG: the error handler is off by 1... the error handler should be passed in to
  103
+        //      the next step instead. This could be tricky...
  104
+        if (shouldContinue) {
85 105
             preLogic(stdout);
86 106
             if (options === null) exec(command, nextStep);
87 107
             else                  exec(command, options, nextStep);
  108
+        } else {
  109
+            finish(execPlan);
88 110
         }
89 111
     };
90 112
 };
@@ -278,14 +300,13 @@ ExecPlan.prototype.execute = function () {
278 300
         if (idx < lastIdx) nextStep = steps[idx+1]; // use already created step
279 301
         else               nextStep = makeFinalFn(this, errorHandlers[idx]);
280 302
         steps[idx] = makeStep(this, (idx === 0), preLogic, command, execOpts,
281  
-                errorHandlers[idx], nextStep);
  303
+                // an individual step will process the previous steps error messages, but will
  304
+                // point 'exec' to call the 'next step'; therefore, we need to account for that here
  305
+                (idx === 0) ? emptyFn : errorHandlers[idx-1], nextStep);
282 306
     }
283 307
 
284 308
     // start the execution process
285 309
     steps[0]();
286  
-
287  
-    // return plan to empty state
288  
-    this.plan = [];
289 310
 };
290 311
 
291 312
 /* --- DEBUG HELP --- */
4  test/ExecPlanTest.js
@@ -222,7 +222,7 @@ exports.errorHandlers = {
222 222
         var execPlan = new ExecPlan({
223 223
             autoPrintOut: false, 
224 224
             autoPrintErr: false,
225  
-            shouldContinueOnError: false
  225
+            continueOnError: false
226 226
         });
227 227
         var errorHandlerSpy = sinon.spy();
228 228
         var secondCommandSpy = sinon.spy();
@@ -258,7 +258,7 @@ exports.errorHandlers = {
258 258
         var execPlan = new ExecPlan({
259 259
             autoPrintOut: false, 
260 260
             autoPrintErr: false, 
261  
-            shouldContinueOnError: false
  261
+            continueOnError: false
262 262
         });
263 263
         var errorHandlerSpy = sinon.spy();
264 264
         var secondCommandSpy = sinon.spy();

0 notes on commit 0a8897c

Please sign in to comment.
Something went wrong with that request. Please try again.