From 8a7bb1be9f07dbec64830d9e570fd663263ad6c2 Mon Sep 17 00:00:00 2001
From: Nick O'Leary <nick.oleary@gmail.com>
Date: Mon, 15 May 2017 13:54:05 +0100
Subject: [PATCH] Log error when non-msg-object is returned from a Function

---
 nodes/core/core/80-function.js           | 23 ++++++-----
 nodes/core/locales/en-US/messages.json   |  3 +-
 test/nodes/core/core/80-function_spec.js | 51 +++++++++++++++++++++++-
 3 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/nodes/core/core/80-function.js b/nodes/core/core/80-function.js
index 2e775bd690..3a4f81d55b 100644
--- a/nodes/core/core/80-function.js
+++ b/nodes/core/core/80-function.js
@@ -28,18 +28,23 @@ module.exports = function(RED) {
         var msgCount = 0;
         for (var m=0;m<msgs.length;m++) {
             if (msgs[m]) {
-                if (util.isArray(msgs[m])) {
-                    for (var n=0; n < msgs[m].length; n++) {
-                        if (msgs[m][n] !== null && msgs[m][n] !== undefined) {
-                            msgs[m][n]._msgid = _msgid;
+                if (!util.isArray(msgs[m])) {
+                    msgs[m] = [msgs[m]];
+                }
+                for (var n=0; n < msgs[m].length; n++) {
+                    var msg = msgs[m][n];
+                    if (msg !== null && msg !== undefined) {
+                        if (typeof msg === 'object' && !Buffer.isBuffer(msg) && !util.isArray(msg)) {
+                            msg._msgid = _msgid;
                             msgCount++;
+                        } else {
+                            var type = typeof msg;
+                            if (type === 'object') {
+                                type = Buffer.isBuffer(msg)?'Buffer':(util.isArray(msg)?'Array':'Date');
+                            }
+                            node.error(RED._("function.error.non-message-returned",{ type: type }))
                         }
                     }
-                } else {
-                    if (msgs[m] !== null && msgs[m] !== undefined) {
-                        msgs[m]._msgid = _msgid;
-                        msgCount++;
-                    }
                 }
             }
         }
diff --git a/nodes/core/locales/en-US/messages.json b/nodes/core/locales/en-US/messages.json
index e514edbe26..cf8cf1a719 100644
--- a/nodes/core/locales/en-US/messages.json
+++ b/nodes/core/locales/en-US/messages.json
@@ -169,7 +169,8 @@
             "outputs": "Outputs"
         },
         "error": {
-            "inputListener":"Cannot add listener to 'input' event within Function"
+            "inputListener":"Cannot add listener to 'input' event within Function",
+            "non-message-returned":"Function tried to send a message of type __type__"
         },
         "tip": "See the Info tab for help writing functions."
     },
diff --git a/test/nodes/core/core/80-function_spec.js b/test/nodes/core/core/80-function_spec.js
index 756de1656c..793aef07b1 100644
--- a/test/nodes/core/core/80-function_spec.js
+++ b/test/nodes/core/core/80-function_spec.js
@@ -127,7 +127,7 @@ describe('function node', function() {
             var n2 = helper.getNode("n2");
             setTimeout(function() {
                 done();
-            }, 200);
+            }, 20);
             n2.on("input", function(msg) {
                 should.fail(null,null,"unexpected message");
             });
@@ -156,9 +156,56 @@ describe('function node', function() {
                 n2MsgCount.should.equal(2);
                 n3MsgCount.should.equal(0);
                 done();
-            },100);
+            },20);
         });
     });
+
+    function testNonObjectMessage(functionText,done) {
+        var flow = [{id:"n1",type:"function",wires:[["n2"]],func:functionText},
+                {id:"n2", type:"helper"}];
+        helper.load(functionNode, flow, function() {
+            var n1 = helper.getNode("n1");
+            var n2 = helper.getNode("n2");
+            var n2MsgCount = 0;
+            n2.on("input", function(msg) {
+                n2MsgCount++;
+            });
+            n1.receive({});
+            setTimeout(function() {
+                try {
+                    n2MsgCount.should.equal(0);
+                    var logEvents = helper.log().args.filter(function(evt) {
+                        return evt[0].type == "function";
+                    });
+                    logEvents.should.have.length(1);
+                    var msg = logEvents[0][0];
+                    msg.should.have.property('level', helper.log().ERROR);
+                    msg.should.have.property('id', 'n1');
+                    msg.should.have.property('type', 'function');
+                    msg.should.have.property('msg', 'function.error.non-message-returned');
+                    done();
+                } catch(err) {
+                    done(err);
+                }
+            },20);
+        });
+    }
+    it('should drop and log non-object message types - string', function(done) {
+        testNonObjectMessage('return "foo"', done)
+    });
+    it('should drop and log non-object message types - buffer', function(done) {
+        testNonObjectMessage('return new Buffer("hello")', done)
+    });
+    it('should drop and log non-object message types - array', function(done) {
+        testNonObjectMessage('return [[[1,2,3]]]', done)
+    });
+    it('should drop and log non-object message types - boolean', function(done) {
+        testNonObjectMessage('return true', done)
+    });
+    it('should drop and log non-object message types - number', function(done) {
+        testNonObjectMessage('return 123', done)
+    });
+
     it('should handle and log script error', function(done) {
         var flow = [{id:"n1",type:"function",wires:[["n2"]],func:"retunr"}];
         helper.load(functionNode, flow, function() {