From 6e4ea950e350d31c443bc192832cdf9aadcd36f9 Mon Sep 17 00:00:00 2001
From: Tyler Eastman <tyler.eastman@gmail.com>
Date: Fri, 29 Sep 2017 10:43:35 -0700
Subject: [PATCH] Improved error handling.

---
 README.md         | 12 +++++------
 ftp-download.js   | 54 +++++++++++++++++++++++++++++++++++------------
 package-lock.json |  2 +-
 3 files changed, 47 insertions(+), 21 deletions(-)

diff --git a/README.md b/README.md
index 048f25a..22f3941 100644
--- a/README.md
+++ b/README.md
@@ -14,16 +14,16 @@ Usage
 This node allows you to download one or more files within a single connection.
 
 The file paths may be defined in one of a number of ways.
-- `msg.`: A path to a variable on the msg object that contains an array of Strings.
-- `flow.`: A path to a variable on the flow context object that contains an array of Strings.
-- `global.`: A path to a variable on the global context object that contains an array of Strings.
+- `msg.`: A variable on the msg object that contains an array of Strings.
+- `flow.`: A variable on the flow context object that contains an array of Strings.
+- `global.`: A variable on the global context object that contains an array of Strings.
 - `json`: A JSON array of Strings.
 - `string`: A singular path String.
 
 The destination path may be defined in one of a number of ways.
-- `msg.`: A path to a variable on the msg object that contains a local path String.
-- `flow.`: A path to a variable on the flow context object that contains a local path String
-- `global.`: A path to a variable on the global context object that contains a local path String
+- `msg.`: A variable on the msg object that contains a local path String.
+- `flow.`: A variable on the flow context object that contains a local path String
+- `global.`: A variable on the global context object that contains a local path String
 - `string`: A singular local path String.
 
 As an output an Array of local file path Strings is places in `msg.payload`
diff --git a/ftp-download.js b/ftp-download.js
index d58a3ef..74ec42e 100644
--- a/ftp-download.js
+++ b/ftp-download.js
@@ -73,16 +73,19 @@ module.exports = function (RED) {
                         fileList = global.get(node.files);
                         break;
                     case 'json':
-                        fileList = node.files;
+                        fileList = JSON.parse(node.files);
                         break;
                     default:
                         fileList = [node.files];
                 }
-                if (!(fileList instanceof Array))
-                    node.error("Files field must be an array.");
+                if (!Array.isArray(fileList)) {
+                    node.error("Files field must be an array.", msg);
+                    return;
+                }
             }
             catch (err) {
                 node.error("Could not load files variable, type: " + node.filesType + " location: " + node.files, msg);
+                return;
             }
 
             // Load the destination from the appropriate variable.
@@ -103,6 +106,7 @@ module.exports = function (RED) {
             }
             catch (err) {
                 node.error("Could not load destination variable, type: " + node.destinationType + " location: " + node.destination, msg);
+                return;
             }
 
             // Assert we have access to write to the destination
@@ -111,6 +115,7 @@ module.exports = function (RED) {
             }
             catch (err) {
                 node.error("Lacking permission to write files to " + destination, msg);
+                return;
             }
 
             conn.on('ready', () => {
@@ -123,32 +128,53 @@ module.exports = function (RED) {
 
                 promise
                     .then((filePaths)=> {
-                        conn.end();
                         msg.payload = filePaths;
                         node.send(msg);
                     })
                     .catch((err) => {
-                        conn.end();
-                        msg.payload = err;
+                        msg.payload = {
+                            fileList: fileList,
+                            destination: destination,
+                            error: err
+                        };
                         node.error("FTP download failed", msg);
                     })
             });
 
             function download(file) {
                 return (filePaths) => new Promise((resolve, reject) => {
-                    conn.get(file, (err, stream) => {
-                        if (err)
-                            throw err;
-                        let filePath = path.join(destination, path.basename(file));
-                        filePaths.push(filePath);
-                        stream.once('finish', () => resolve(filePaths));
-                        stream.pipe(fs.createWriteStream(filePath));
-                    });
+                    try {
+                        conn.get(file, (err, stream) => {
+                            try {
+                                if (err)
+                                    throw err;
+                                let filePath = path.join(destination, path.basename(file));
+                                filePaths.push(filePath);
+                                stream.once('finish', () => resolve(filePaths));
+                                stream.pipe(fs.createWriteStream(filePath));
+                            }
+                            catch (error) {
+                                reject(error);
+                            }
+                        });
+                    }
+                    catch (error) {
+                        reject(error);
+                    }
                 });
             }
 
             conn.connect(node.serverConfig.options);
         });
+
+        node.on('close', () => {
+            try {
+                conn.destroy();
+            }
+            catch (err) {
+                // Do nothing as the node is closed anyway.
+            }
+        });
     }
 
     RED.nodes.registerType('ftp-download', FtpDownloadNode);
diff --git a/package-lock.json b/package-lock.json
index 1a6d3a0..45e6f38 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1,6 +1,6 @@
 {
   "name": "node-red-contrib-ftp-download",
-  "version": "1.0.0",
+  "version": "1.0.1",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {