From c99b35428b26c0c426b8e236910edaf0c18e057f Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Fri, 25 Jan 2019 13:35:02 +0000 Subject: [PATCH] Ensure status/error events are propagated to parent properly --- .../@node-red/runtime/lib/nodes/flows/Flow.js | 56 +++++++------ .../runtime/lib/nodes/flows/Subflow.js | 44 ++++++++++ .../runtime/lib/nodes/flows/Subflow_spec.js | 80 ++++++++++++++++++- 3 files changed, 153 insertions(+), 27 deletions(-) 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 c78b91a86..08c3998c5 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 @@ -321,21 +321,30 @@ class Flow { } /** - * Handle a status event from a node within this flow. If there are no Status - * nodes within this flow, pass the request to the parent flow. - * @param {[type]} node [description] - * @param {[type]} statusMessage [description] + * Handle a status event from a node within this flow. + * @param {Node} node The original node that triggered the event + * @param {Object} statusMessage The status object + * @param {Node} reportingNode The node emitting the status event. + * This could be a subflow instance node when the status + * is being delegated up. + * @param {boolean} muteStatus Whether to emit the status event * @return {[type]} [description] */ - handleStatus(node,statusMessage) { - events.emit("node-status",{ - id: node.id, - status:statusMessage - }); + handleStatus(node,statusMessage,reportingNode,muteStatus) { + if (!reportingNode) { + reportingNode = node; + } + if (!muteStatus) { + events.emit("node-status",{ + id: node.id, + status:statusMessage + }); + } + + let handled = false; - var handled = false; this.statusNodes.forEach(function(targetStatusNode) { - if (targetStatusNode.scope && targetStatusNode.scope.indexOf(node.id) === -1) { + if (targetStatusNode.scope && targetStatusNode.scope.indexOf(reportingNode.id) === -1) { return; } var message = { @@ -354,21 +363,22 @@ class Flow { targetStatusNode.receive(message); handled = true; }); - if (!handled) { - // // Nothing in this flow handled the status - pass it to the parent - this.parent.handleStatus(node,statusMessage); - } + return handled; } /** * Handle an error event from a node within this flow. If there are no Catch * nodes within this flow, pass the event to the parent flow. - * @param {[type]} node [description] - * @param {[type]} logMessage [description] - * @param {[type]} msg [description] - * @return {[type]} [description] + * @param {[type]} node [description] + * @param {[type]} logMessage [description] + * @param {[type]} msg [description] + * @param {[type]} reportingNode [description] + * @return {[type]} [description] */ - handleError(node,logMessage,msg) { + handleError(node,logMessage,msg,reportingNode) { + if (!reportingNode) { + reportingNode = node; + } // console.log("HE",logMessage); var count = 1; if (msg && msg.hasOwnProperty("error") && msg.error !== null) { @@ -384,7 +394,7 @@ class Flow { } var handled = false; this.catchNodes.forEach(function(targetCatchNode) { - if (targetCatchNode.scope && targetCatchNode.scope.indexOf(node.id) === -1) { + if (targetCatchNode.scope && targetCatchNode.scope.indexOf(reportingNode.id) === -1) { return; } var errorMessage; @@ -411,10 +421,6 @@ class Flow { targetCatchNode.receive(errorMessage); handled = true; }); - if (!handled) { - // Nothing in this flow handled the error - pass it to the parent - handled = this.parent.handleError(node,logMessage); - } return handled; } diff --git a/packages/node_modules/@node-red/runtime/lib/nodes/flows/Subflow.js b/packages/node_modules/@node-red/runtime/lib/nodes/flows/Subflow.js index 84c4d9dfe..cf393868e 100644 --- a/packages/node_modules/@node-red/runtime/lib/nodes/flows/Subflow.js +++ b/packages/node_modules/@node-red/runtime/lib/nodes/flows/Subflow.js @@ -191,6 +191,50 @@ class Subflow extends Flow { } super.start(diff); } + + /** + * Handle a status event from a node within this flow. + * @param {Node} node The original node that triggered the event + * @param {Object} statusMessage The status object + * @param {Node} reportingNode The node emitting the status event. + * This could be a subflow instance node when the status + * is being delegated up. + * @param {boolean} muteStatus Whether to emit the status event + * @return {[type]} [description] + */ + handleStatus(node,statusMessage,reportingNode,muteStatus) { + let handled = super.handleStatus(node,statusMessage,reportingNode,muteStatus); + if (!handled) { + // No status node on this subflow caught the status message. + // Pass up to the parent with this subflow's instance as the + // reporting node + handled = this.parent.handleStatus(node,statusMessage,this.node,true); + } + return handled; + + } + + /** + * Handle an error event from a node within this flow. If there are no Catch + * nodes within this flow, pass the event to the parent flow. + * @param {[type]} node [description] + * @param {[type]} logMessage [description] + * @param {[type]} msg [description] + * @param {[type]} reportingNode [description] + * @return {[type]} [description] + */ + handleError(node,logMessage,msg,reportingNode) { + let handled = super.handleError(node,logMessage,msg,reportingNode); + if (!handled) { + // No catch node on this subflow caught the error message. + // Pass up to the parent with the subflow's instance as the + // reporting node. + handled = this.parent.handleError(node,logMessage,msg,this.node); + } + return handled; + } + + } diff --git a/test/unit/@node-red/runtime/lib/nodes/flows/Subflow_spec.js b/test/unit/@node-red/runtime/lib/nodes/flows/Subflow_spec.js index 4b9d8a928..732c406f7 100644 --- a/test/unit/@node-red/runtime/lib/nodes/flows/Subflow_spec.js +++ b/test/unit/@node-red/runtime/lib/nodes/flows/Subflow_spec.js @@ -386,7 +386,7 @@ describe('Subflow', function() { }); }); describe("#handleStatus",function() { - it("passes a status event to the subflow's parent tab status node",function(done) { + it("passes a status event to the subflow's parent tab status node - all scope",function(done) { var config = flowUtils.parseConfig([ {id:"t1",type:"tab"}, {id:"1",x:10,y:10,z:"t1",type:"test",name:"a",wires:["2"]}, @@ -419,10 +419,49 @@ describe('Subflow', function() { done(); }); }); + + it("passes a status event to the subflow's parent tab status node - targetted scope",function(done) { + var config = flowUtils.parseConfig([ + {id:"t1",type:"tab"}, + {id:"1",x:10,y:10,z:"t1",type:"test",name:"a",wires:["2"]}, + {id:"2",x:10,y:10,z:"t1",type:"subflow:sf1",wires:["3"]}, + {id:"3",x:10,y:10,z:"t1",type:"test",foo:"a",wires:[]}, + {id:"sf1",type:"subflow","name":"Subflow 2","info":"", + "in":[{"wires":[{"id":"sf1-1"}]}],"out":[{"wires":[{"id":"sf1-1","port":0}]}]}, + {id:"sf1-1",type:"testStatus",name:"test-status-node","z":"sf1",x:166,y:99,"wires":[[]]}, + {id:"sn",x:10,y:10,z:"t1",type:"status",scope:["2"],wires:[]} + ]); + var parentFlowStatusCalled = false; + + var flow = Flow.create({handleStatus:() => { parentFlowStatusCalled = true} },config,config.flows["t1"]); + + flow.start(); + + var activeNodes = flow.getActiveNodes(); + + activeNodes["1"].receive({payload:"test"}); + + parentFlowStatusCalled.should.be.false(); + + currentNodes["sn"].should.have.a.property("handled",1); + var statusMessage = currentNodes["sn"].messages[0]; + + statusMessage.should.have.a.property("status"); + statusMessage.status.should.have.a.property("text","test status"); + statusMessage.status.should.have.a.property("source"); + statusMessage.status.source.should.have.a.property("type","testStatus"); + statusMessage.status.source.should.have.a.property("name","test-status-node"); + + flow.stop().then(function() { + + done(); + }); + }); + }); describe("#handleError",function() { - it("passes an error event to the subflow's parent tab catch node",function(done) { + it("passes an error event to the subflow's parent tab catch node - all scope",function(done) { var config = flowUtils.parseConfig([ {id:"t1",type:"tab"}, {id:"1",x:10,y:10,z:"t1",type:"test",name:"a",wires:["2"]}, @@ -450,11 +489,48 @@ describe('Subflow', function() { statusMessage.error.source.should.have.a.property("type","testError"); statusMessage.error.source.should.have.a.property("name","test-error-node"); + flow.stop().then(function() { + done(); + }); + }); + + it("passes an error event to the subflow's parent tab catch node - targetted scope",function(done) { + var config = flowUtils.parseConfig([ + {id:"t1",type:"tab"}, + {id:"1",x:10,y:10,z:"t1",type:"test",name:"a",wires:["2"]}, + {id:"2",x:10,y:10,z:"t1",type:"subflow:sf1",wires:["3"]}, + {id:"3",x:10,y:10,z:"t1",type:"test",foo:"a",wires:[]}, + {id:"sf1",type:"subflow","name":"Subflow 2","info":"", + "in":[{"wires":[{"id":"sf1-1"}]}],"out":[{"wires":[{"id":"sf1-1","port":0}]}]}, + {id:"sf1-1",name:"test-error-node",type:"testError","z":"sf1",x:166,y:99,"wires":[[]]}, + {id:"sn",x:10,y:10,z:"t1",type:"catch",scope:["2"],wires:[]} + ]); + var parentFlowErrorCalled = false; + var flow = Flow.create({handleError:() => { parentFlowErrorCalled = true} },config,config.flows["t1"]); + + flow.start(); + + var activeNodes = flow.getActiveNodes(); + + activeNodes["1"].receive({payload:"test"}); + + parentFlowErrorCalled.should.be.false(); + + currentNodes["sn"].should.have.a.property("handled",1); + var statusMessage = currentNodes["sn"].messages[0]; + + statusMessage.should.have.a.property("error"); + statusMessage.error.should.have.a.property("message","test error"); + statusMessage.error.should.have.a.property("source"); + statusMessage.error.source.should.have.a.property("type","testError"); + statusMessage.error.source.should.have.a.property("name","test-error-node"); + flow.stop().then(function() { done(); }); }); + }); });