From 79f3669fac3d2b5b9881624946762bb6f6ded01c Mon Sep 17 00:00:00 2001
From: Nick O'Leary <nick.oleary@gmail.com>
Date: Tue, 5 Feb 2019 14:28:20 +0000
Subject: [PATCH] Add 'catch uncaught only' mode to Catch node Closes #1747

This was inspired by a PR from @mauriciom75 but implemented in a different way
due to some of the internal reworking done to Flow and Subflow in the dev branch
---
 .../@node-red/nodes/core/core/25-catch.html   | 18 ++++++++-
 .../@node-red/nodes/core/core/25-catch.js     |  1 +
 .../nodes/locales/en-US/core/25-catch.html    |  3 +-
 .../nodes/locales/en-US/messages.json         |  4 +-
 .../@node-red/runtime/lib/nodes/flows/Flow.js | 25 ++++++++++++
 .../runtime/lib/nodes/flows/Flow_spec.js      | 38 +++++++++++++++----
 6 files changed, 78 insertions(+), 11 deletions(-)

diff --git a/packages/node_modules/@node-red/nodes/core/core/25-catch.html b/packages/node_modules/@node-red/nodes/core/core/25-catch.html
index f32dfeb9b..b46b03330 100644
--- a/packages/node_modules/@node-red/nodes/core/core/25-catch.html
+++ b/packages/node_modules/@node-red/nodes/core/core/25-catch.html
@@ -7,6 +7,10 @@
             <option value="target" data-i18n="catch.scope.selected"></options>
         </select>
     </div>
+    <div class="form-row node-input-uncaught-row">
+        <input type="checkbox" id="node-input-uncaught" style="display: inline-block; width: auto; vertical-align: top; margin-left: 30px; margin-right: 5px;">
+        <label for="node-input-uncaught" style="width: auto" data-i18n="catch.label.uncaught"></label>
+    </div>
     <div class="form-row node-input-target-row" style="display: none;">
         <div id="node-input-catch-target-container-div" style="min-height: 100px;position: relative;   box-sizing: border-box; border-radius: 2px; height: 180px;  border: 1px solid #ccc;overflow:hidden; ">
             <div style="box-sizing: border-box; line-height: 20px; font-size: 0.8em; border-bottom: 1px solid #ddd; height: 20px;">
@@ -64,13 +68,20 @@
         color:"#e49191",
         defaults: {
             name: {value:""},
-            scope: {value:null}
+            scope: {value:null},
+            uncaught: {value:false}
         },
         inputs:0,
         outputs:1,
         icon: "alert.png",
         label: function() {
-            return this.name||(this.scope?this._("catch.catchNodes",{number:this.scope.length}):this._("catch.catch"));
+            if (this.name) {
+                return this.name;
+            }
+            if (this.scope) {
+                return this._("catch.catchNodes",{number:this.scope.length});
+            }
+            return this.uncaught?this._("catch.catchUncaught"):this._("catch.catch")
         },
         labelStyle: function() {
             return this.name?"node_label_italic":"";
@@ -210,8 +221,10 @@
                 if (scope === "target") {
                     createNodeList();
                     $(".node-input-target-row").show();
+                    $(".node-input-uncaught-row").hide();
                 } else {
                     $(".node-input-target-row").hide();
+                    $(".node-input-uncaught-row").show();
                 }
                 node.resize();
             });
@@ -227,6 +240,7 @@
             if (scope === 'all') {
                 this.scope = null;
             } else {
+                $("#node-input-uncaught").prop("checked",false);
                 var node = this;
                 node.scope = [];
                 $(".node-input-target-node-checkbox").each(function(n) {
diff --git a/packages/node_modules/@node-red/nodes/core/core/25-catch.js b/packages/node_modules/@node-red/nodes/core/core/25-catch.js
index 163ac56e6..86245186c 100644
--- a/packages/node_modules/@node-red/nodes/core/core/25-catch.js
+++ b/packages/node_modules/@node-red/nodes/core/core/25-catch.js
@@ -21,6 +21,7 @@ module.exports = function(RED) {
         RED.nodes.createNode(this,n);
         var node = this;
         this.scope = n.scope;
+        this.uncaught = n.uncaught;
         this.on("input",function(msg) {
             this.send(msg);
         });
diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/core/25-catch.html b/packages/node_modules/@node-red/nodes/locales/en-US/core/25-catch.html
index cfcd53951..49897fe39 100644
--- a/packages/node_modules/@node-red/nodes/locales/en-US/core/25-catch.html
+++ b/packages/node_modules/@node-red/nodes/locales/en-US/core/25-catch.html
@@ -32,7 +32,8 @@
        halt. This node can be used to catch those errors and handle them with a
        dedicated flow.</p>
     <p>By default, the node will catch errors thrown by any node on the same tab. Alternatively
-    it can be targetted at specific nodes.</p>
+    it can be targetted at specific nodes, or configured to only catch errors that
+    have not already been caught by a 'targeted' catch node.</p>
     <p>When an error is thrown, all matching catch nodes will receive the message.</p>
     <p>If an error is thrown within a subflow, the error will get handled by any
        catch nodes within the subflow. If none exists, the error will be propagated
diff --git a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json
index d88469297..f4513519a 100644
--- a/packages/node_modules/@node-red/nodes/locales/en-US/messages.json
+++ b/packages/node_modules/@node-red/nodes/locales/en-US/messages.json
@@ -72,13 +72,15 @@
     "catch": {
         "catch": "catch: all",
         "catchNodes": "catch: __number__",
+        "catchUncaught": "catch: uncaught",
         "label": {
             "source": "Catch errors from",
             "node": "node",
             "type": "type",
             "selectAll": "select all",
             "sortByLabel": "sort by label",
-            "sortByType": "sort by type"
+            "sortByType": "sort by type",
+            "uncaught": "Ignore errors handled by other Catch nodes"
         },
         "scope": {
             "all": "all nodes",
diff --git a/packages/node_modules/@node-red/runtime/lib/nodes/flows/Flow.js b/packages/node_modules/@node-red/runtime/lib/nodes/flows/Flow.js
index 6c0e3ad8a..76b7fddf8 100644
--- a/packages/node_modules/@node-red/runtime/lib/nodes/flows/Flow.js
+++ b/packages/node_modules/@node-red/runtime/lib/nodes/flows/Flow.js
@@ -212,6 +212,21 @@ class Flow {
                 }
             }
         }
+        this.catchNodes.sort(function(A,B) {
+            if (A.scope && !B.scope) {
+                return -1;
+            } else if (!A.scope && B.scope) {
+                return 1;
+            } else if (A.scope && B.scope) {
+                return 0;
+            } else if (A.uncaught && !B.uncaught) {
+                return 1;
+            } else if (!A.uncaught && B.uncaught) {
+                return -1;
+            }
+            return 0;
+        });
+
         if (activeCount > 0) {
             this.trace("------------------|--------------|-----------------");
         }
@@ -419,10 +434,20 @@ class Flow {
             }
             handled = true;
         } else {
+            var handledByUncaught = false;
+
             this.catchNodes.forEach(function(targetCatchNode) {
                 if (targetCatchNode.scope && targetCatchNode.scope.indexOf(reportingNode.id) === -1) {
                     return;
                 }
+                if (!targetCatchNode.scope && targetCatchNode.uncaught && !handledByUncaught) {
+                    if (handled) {
+                        // This has been handled by a !uncaught catch node
+                        return;
+                    }
+                    // This is an uncaught error
+                    handledByUncaught = true;
+                }
                 var errorMessage;
                 if (msg) {
                     errorMessage = redUtil.cloneMessage(msg);
diff --git a/test/unit/@node-red/runtime/lib/nodes/flows/Flow_spec.js b/test/unit/@node-red/runtime/lib/nodes/flows/Flow_spec.js
index feac88e22..739836b9b 100644
--- a/test/unit/@node-red/runtime/lib/nodes/flows/Flow_spec.js
+++ b/test/unit/@node-red/runtime/lib/nodes/flows/Flow_spec.js
@@ -113,6 +113,7 @@ describe('Flow', function() {
         Node.call(this,n);
         var node = this;
         this.scope = n.scope;
+        this.uncaught = n.uncaught;
         this.foo = n.foo;
         this.handled = 0;
         this.messages = [];
@@ -159,6 +160,7 @@ describe('Flow', function() {
             Object.keys(flow.getActiveNodes()).length.should.equal(0);
         });
     });
+
     describe('#start',function() {
         it("instantiates an initial configuration and stops it",function(done) {
             var config = flowUtils.parseConfig([
@@ -413,6 +415,7 @@ describe('Flow', function() {
         });
 
     });
+
     describe('#getNode',function() {
         it("gets a node known to the flow",function(done) {
             var config = flowUtils.parseConfig([
@@ -560,7 +563,6 @@ describe('Flow', function() {
 
     });
 
-
     describe("#handleError",function() {
         it("passes an error event to the adjacent catch node",function(done) {
             var config = flowUtils.parseConfig([
@@ -569,14 +571,15 @@ describe('Flow', function() {
                 {id:"2",x:10,y:10,z:"t1",type:"test",wires:["3"]},
                 {id:"3",x:10,y:10,z:"t1",type:"test",foo:"a",wires:[]},
                 {id:"sn",x:10,y:10,z:"t1",type:"catch",foo:"a",wires:[]},
-                {id:"sn2",x:10,y:10,z:"t1",type:"catch",foo:"a",wires:[]}
+                {id:"sn2",x:10,y:10,z:"t1",type:"catch",foo:"a",wires:[]},
+                {id:"sn3",x:10,y:10,z:"t1",type:"catch",uncaught:true,wires:[]}
             ]);
             var flow = Flow.create({},config,config.flows["t1"]);
 
             flow.start();
 
             var activeNodes = flow.getActiveNodes();
-            Object.keys(activeNodes).should.have.length(5);
+            Object.keys(activeNodes).should.have.length(6);
 
 
             flow.handleError(config.flows["t1"].nodes["1"],"my-error",{a:"foo"});
@@ -601,6 +604,9 @@ describe('Flow', function() {
             statusMessage.error.source.should.have.a.property("type","test");
             statusMessage.error.source.should.have.a.property("name","a");
 
+            // Node sn3 has uncaught:true - so should not get called
+            currentNodes["sn3"].should.have.a.property("handled",0);
+
 
             flow.stop().then(function() {
                 done();
@@ -613,14 +619,16 @@ describe('Flow', function() {
                 {id:"2",x:10,y:10,z:"t1",type:"test",wires:["3"]},
                 {id:"3",x:10,y:10,z:"t1",type:"test",foo:"a",wires:[]},
                 {id:"sn",x:10,y:10,z:"t1",type:"catch",scope:["2"],foo:"a",wires:[]},
-                {id:"sn2",x:10,y:10,z:"t1",type:"catch",scope:["1"],foo:"a",wires:[]}
+                {id:"sn2",x:10,y:10,z:"t1",type:"catch",scope:["1"],foo:"a",wires:[]},
+                {id:"sn3",x:10,y:10,z:"t1",type:"catch",uncaught:true,wires:[]},
+                {id:"sn4",x:10,y:10,z:"t1",type:"catch",uncaught:true,wires:[]}
             ]);
             var flow = Flow.create({},config,config.flows["t1"]);
 
             flow.start();
 
             var activeNodes = flow.getActiveNodes();
-            Object.keys(activeNodes).should.have.length(5);
+            Object.keys(activeNodes).should.have.length(7);
 
             flow.handleError(config.flows["t1"].nodes["1"],"my-error",{a:"foo"});
 
@@ -635,13 +643,29 @@ describe('Flow', function() {
             statusMessage.error.source.should.have.a.property("type","test");
             statusMessage.error.source.should.have.a.property("name","a");
 
+            // Node sn3/4 have uncaught:true - so should not get called
+            currentNodes["sn3"].should.have.a.property("handled",0);
+            currentNodes["sn4"].should.have.a.property("handled",0);
+
+            // Inject error that sn1/2 will ignore - so should get picked up by sn3
+            flow.handleError(config.flows["t1"].nodes["3"],"my-error-2",{a:"foo-2"});
+
+            currentNodes["sn"].should.have.a.property("handled",0);
+            currentNodes["sn2"].should.have.a.property("handled",1);
+            currentNodes["sn3"].should.have.a.property("handled",1);
+            currentNodes["sn4"].should.have.a.property("handled",1);
+
+            statusMessage = currentNodes["sn3"].messages[0];
+            statusMessage.should.have.a.property("error");
+            statusMessage.error.should.have.a.property("message","my-error-2");
+            statusMessage.error.should.have.a.property("source");
+            statusMessage.error.source.should.have.a.property("id","3");
+            statusMessage.error.source.should.have.a.property("type","test");
 
             flow.stop().then(function() {
                 done();
             });
         });
-
-
         it("moves any existing error object sideways",function(done){
             var config = flowUtils.parseConfig([
                 {id:"t1",type:"tab"},