Ensure node-red-admin has non-0 return code when commands fail

knolleary · knolleary · commit 1c7f7aefa5ec · 2020-06-04T10:54:55.000+01:00
diff --git a/lib/commands/disable.js b/lib/commands/disable.js
@@ -26,7 +26,7 @@ function command(argv,result) {
         data: {
             enabled: false
         }
-    }).then(result.logList).catch(result.warn);
+    }).then(result.logList);
 }
 command.alias = "disable";
 command.usage = command.alias+" {module|id}";
diff --git a/lib/commands/enable.js b/lib/commands/enable.js
@@ -26,7 +26,7 @@ function command(argv,result) {
         data: {
             enabled: true
         }
-    }).then(result.logList).catch(result.warn);
+    }).then(result.logList);
 }
 command.alias = "enable";
 command.usage = command.alias+" {module|id}";
diff --git a/lib/commands/info.js b/lib/commands/info.js
@@ -21,7 +21,7 @@ function command(argv,result) {
     if (!node) {
         return result.help(command);
     }
-    return request.request('/nodes/' + node, {}).then(result.logDetails).catch(result.warn);
+    return request.request('/nodes/' + node, {}).then(result.logDetails);
 }
 command.alias = "info";
 command.usage = command.alias+" {module|node}";
diff --git a/lib/commands/install.js b/lib/commands/install.js
@@ -37,7 +37,7 @@ function command(argv,result) {
     return request.request('/nodes', {
         method: "POST",
         data: data
-    }).then(result.logDetails).catch(result.warn);
+    }).then(result.logDetails);
 }
 command.alias = "install";
 command.usage = command.alias+" <module> [<url>]";
diff --git a/lib/commands/list.js b/lib/commands/list.js
@@ -17,7 +17,7 @@
 var request = require("../request");
 
 function command(argv,result) {
-    return request.request('/nodes', {}).then(result.logNodeList).catch(result.warn);
+    return request.request('/nodes', {}).then(result.logNodeList);
 }
 command.alias = "list";
 command.usage = command.alias;
diff --git a/lib/commands/login.js b/lib/commands/login.js
@@ -20,42 +20,42 @@ var prompt = require("../prompt");
 
 function command(argv,result) {
     config.tokens(null);
-    if (argv.json) {
-        console.warn("login command does not support json format output");
-    }
     return request.request('/auth/login',{}).then(function(resp) {
-        if (resp.type) {
-            if (resp.type == "credentials") {
-                prompt.read({prompt:"Username:"},function(err, username) {
-                    prompt.read({prompt:"Password:",silent: true},function(err, password) {
-                        request.request('/auth/token', {
-                            method: "POST",
-                            data: {
-                                client_id: 'node-red-admin',
-                                grant_type: 'password',
-                                scope: '*',
-                                username: username,
-                                password: password
-                            }
-                        }).then(function(resp) {
-                            config.tokens(resp);
-                            result.log("Logged in");
-                        }).catch(function(resp) {
-                            result.warn("Login failed");
+        return new Promise((resolve,reject) => {
+            if (resp.type) {
+                if (resp.type == "credentials") {
+                    prompt.read({prompt:"Username:"},function(err, username) {
+                        prompt.read({prompt:"Password:",silent: true},function(err, password) {
+                            request.request('/auth/token', {
+                                method: "POST",
+                                data: {
+                                    client_id: 'node-red-admin',
+                                    grant_type: 'password',
+                                    scope: '*',
+                                    username: username,
+                                    password: password
+                                }
+                            }).then(function(resp) {
+                                config.tokens(resp);
+                                result.log("Logged in");
+                                resolve();
+                            }).catch(function(resp) {
+                                reject("Login failed");
+                            });
                         });
                     });
-                });
+                } else {
+                    reject("Unsupported login type");
+                }
             } else {
-                result.warn("Unsupported login type");
+                resolve();
             }
-        }
-    }).catch(function(resp) {
-        result.warn("Login failed");
+        });
     });
 }
 
 command.alias = "login";
 command.usage = command.alias+"";
-command.description = "Log user in to the targetted Node-RED admin api";
+command.description = "Log in to the targetted Node-RED admin api";
 
 module.exports = command;
diff --git a/lib/commands/projects.js b/lib/commands/projects.js
@@ -17,7 +17,7 @@
 var request = require("../request");
 
 function command(argv,result) {
-    return request.request('/projects', {}).then(result.logProjectList).catch(result.warn);
+    return request.request('/projects', {}).then(result.logProjectList);
 }
 command.alias = "projects";
 command.usage = command.alias;
diff --git a/lib/commands/remove.js b/lib/commands/remove.js
@@ -29,7 +29,7 @@ function command(argv,result) {
         } else {
             result.log("Uninstalled " + module);
         }
-    }).catch(result.warn);
+    });
 }
 command.alias = "remove";
 command.usage = command.alias+" <module>";
diff --git a/lib/commands/search.js b/lib/commands/search.js
@@ -65,10 +65,8 @@ function command(argv,result) {
                 }
             }
         } else {
-            result.warn(response.status + ": " + response.data);
+            throw new Error(response.status + ": " + response.data);
         }
-    }).catch(err => {
-        result.warn(err.toString());
     });
 
 }
diff --git a/lib/commands/target.js b/lib/commands/target.js
@@ -17,23 +17,25 @@
 var config = require("../config");
 
 function command(argv,result) {
-    var target = argv._[1];
-    if (target) {
-        if (!/^https?:\/\/.+/.test(target)) {
-            result.warn("Invalid target URL: " + target);
-            return;
+    return new Promise((resolve,reject) => {
+        var target = argv._[1];
+        if (target) {
+            if (!/^https?:\/\/.+/.test(target)) {
+                reject("Invalid target URL: " + target);
+                return;
+            }
+            if (target.slice(-1) == "/") {
+                target = target.slice(0, target.length - 1);
+            }
+            config.target(target);
         }
-        if (target.slice(-1) == "/") {
-            target = target.slice(0, target.length - 1);
+        if (argv.json) {
+            result.log(JSON.stringify({target: config.target()}," ",4));
+        } else {
+            result.log("Target: " + config.target());
         }
-        config.target(target);
-
-    }
-    if (argv.json) {
-        result.log(JSON.stringify({target: config.target()}," ",4));
-    } else {
-        result.log("Target: " + config.target());
-    }
+        resolve();
+    });
 }
 
 command.alias = "target";
diff --git a/lib/index.js b/lib/index.js
@@ -65,7 +65,10 @@ module.exports = function(args) {
         if (argv.h || argv.help || argv['?']) {
             result.help(commands[command]);
         } else {
-            commands[command](argv,result);
+            return commands[command](argv,result).catch(err => {
+                result.warn(err);
+                throw err;
+            });
         }
     } else {
         help();
diff --git a/lib/result.js b/lib/result.js
@@ -16,6 +16,7 @@
 
 var Table = require('cli-table');
 var util = require("util");
+const config = require("./config");
 
 var outputFormat = "text";
 
@@ -155,40 +156,44 @@ module.exports = {
         if (msg.response) {
             if (msg.response.status === 401) {
                 if (outputFormat === "json") {
-                    console.log({error:"Not logged in. Use 'login' to log in.", status: 401});
+                    console.log(JSON.stringify({error:"Not logged in. Use 'login' to log in.", status: 401}," ",4));
                 } else {
                     console.warn("Not logged in. Use 'login' to log in.");
                 }
             } else if (msg.response.data) {
                 if (msg.response.status === 404 && !msg.response.data.message) {
                     if (outputFormat === "json") {
-                        console.log({error:"Node-RED Admin API not found. Use 'target' to set API location", status: 404});
+                        console.log(JSON.stringify({error:"Node-RED Admin API not found. Use 'target' to set API location", status: 404}," ",4));
                     } else {
                         console.warn("Node-RED Admin API not found. Use 'target' to set API location");
                     }
                 } else {
                     if (outputFormat === "json") {
-                        console.log({error:msg.response.data.message, status: msg.response.status});
+                        console.log(JSON.stringify({error:msg.response.data.message, status: msg.response.status}," ",4));
                     } else {
                         console.warn(msg.response.status+": "+msg.response.data.message);
                     }
                 }
             } else {
                 if (outputFormat === "json") {
-                    console.log({error:msg.toString(), status: msg.response.status});
+                    console.log(JSON.stringify({error:msg.toString(), status: msg.response.status}," ",4));
                 } else {
                     console.warn(msg.response.status+": "+msg.toString());
                 }
             }
         } else {
+            var text = msg.toString();
+            if (/ECONNREFUSED/.test(text)) {
+                text = "Failed to connect to "+config.target();
+            }
             if (outputFormat === "json") {
-                console.log({error:msg.toString()});
+                console.log(JSON.stringify({error:text}));
             } else {
-                console.warn(msg.toString());
+                console.warn(text);
             }
         }
     },
-    help:function(command) {
+    help: function(command) {
         var helpText = "Usage:" + "\n" +
             "   node-red-admin " + command.usage + "\n\n" +
             "Description:" + "\n" +
@@ -197,6 +202,7 @@ module.exports = {
             (command.options ? "   " + command.options + "\n" : "") +
             "   -h|?  --help      display this help text and exit";
         console.log(helpText);
+        return Promise.resolve();
     },
     logList:logList,
     logNodeList:logNodeList,
diff --git a/node-red-admin.js b/node-red-admin.js
@@ -15,4 +15,6 @@
  * limitations under the License.
  **/
 
-require("./lib/index.js")(process.argv.slice(2));
+require("./lib/index.js")(process.argv.slice(2)).catch(err => {
+    process.exit(1);
+});
diff --git a/test/lib/commands/info_spec.js b/test/lib/commands/info_spec.js
@@ -62,14 +62,8 @@ describe("commands/info", function() {
             return Promise.reject("error");
         });
         command({_:[null,"testnode"]},result).then(function() {
-            if (error) {
-                throw error;
-            }
-            result.logDetails.called.should.be.false();
-            result.warn.called.should.be.true();
-            result.warn.args[0][0].should.eql("error");
-            done();
-        }).catch(done);
+            done("Should have returned the error");
+        }).catch(err => { done() });
     });
 
     it('displays command help if node not specified', function(done) {
diff --git a/test/lib/commands/login_spec.js b/test/lib/commands/login_spec.js
@@ -51,7 +51,7 @@ describe("commands/list", function() {
         requestStub.onCall(1).returns(Promise.resolve({access_token:"12345"}));
 
 
-        command({},result).then(function() {
+        command({},result).then(() => {
             requestStub.calledTwice.should.be.true();
             requestStub.args[0][0].should.eql("/auth/login");
             requestStub.args[1][0].should.eql("/auth/token");
@@ -75,17 +75,17 @@ describe("commands/list", function() {
         var requestStub = sinon.stub(request,"request");
         requestStub.onCall(0).returns(Promise.resolve({type:"unknown"}));
         requestStub.onCall(1).returns(Promise.resolve({access_token:"12345"}));
-        command({},result).then(function() {
-            requestStub.calledOnce.should.be.true();
-            requestStub.args[0][0].should.eql("/auth/login");
-            /Unsupported login type/.test(result.warn.args[0][0]).should.be.true();
+        command({},result).then(() => {
+            done("Should not have resolved with unsupported login type")
+        }).catch(err => {
+            /Unsupported login type/.test(err).should.be.true();
             done();
         }).catch(done);
     });
     it('handles no authentication', function(done) {
         var requestStub = sinon.stub(request,"request");
         requestStub.onCall(0).returns(Promise.resolve({}));
-        command({},result).then(function() {
+        command({},result).then(() => {
             requestStub.calledOnce.should.be.true();
             requestStub.args[0][0].should.eql("/auth/login");
             result.log.called.should.be.false();
@@ -97,26 +97,25 @@ describe("commands/list", function() {
         var requestStub = sinon.stub(request,"request");
         requestStub.onCall(0).returns(Promise.resolve({type:"credentials"}));
         requestStub.onCall(1).returns(Promise.reject());
-        command({},result).then(function() {
+        command({},result).then(() => {
+            done("Should not have resolved with login failure");
+        }).catch(err => {
             config.tokens.calledOnce.should.be.true();
             should.not.exist(config.tokens.args[0][0]);
-
-            result.log.called.should.be.false();
-            result.warn.called.should.be.true();
-            /Login failed/.test(result.warn.args[0][0]).should.be.true();
+            /Login failed/.test(err).should.be.true();
             done();
         }).catch(done);
     });
 
     it('handles unexpected error', function(done) {
         var requestStub = sinon.stub(request,"request");
-        requestStub.onCall(0).returns(Promise.reject());
-        command({},result).then(function() {
+        requestStub.onCall(0).returns(Promise.reject("fail"));
+        command({},result).then(() => {
+            done("Should not have resolved with login failure");
+        }).catch(err => {
             config.tokens.calledOnce.should.be.true();
             should.not.exist(config.tokens.args[0][0]);
-            result.log.called.should.be.false();
-            result.warn.called.should.be.true();
-            /Login failed/.test(result.warn.args[0][0]).should.be.true();
+            /fail/.test(err).should.be.true();
             done();
         }).catch(done);
     });
diff --git a/test/lib/commands/search_spec.js b/test/lib/commands/search_spec.js
@@ -63,9 +63,10 @@ describe("commands/install", function() {
         sinon.stub(httpRequest,"get").returns(Promise.resolve({status:101,data:"testError"}));
 
         command({_:[null,"testnode"]},result).then(function() {
+            done("Should not have resolved")
+        }).catch(err => {
             result.log.called.should.be.false();
-            result.warn.called.should.be.true();
-            result.warn.args[0][0].should.eql("101: testError");
+            /101: testError/.test(err).should.be.true();
             done();
         }).catch(done);
     });
diff --git a/test/lib/commands/target_spec.js b/test/lib/commands/target_spec.js

Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ function command(argv,result) {`
`26`	`26`	`data: {`
`27`	`27`	`enabled: false`
`28`	`28`	`}`
`29`		`- }).then(result.logList).catch(result.warn);`
	`29`	`+ }).then(result.logList);`
`30`	`30`	`}`
`31`	`31`	`command.alias = "disable";`
`32`	`32`	`command.usage = command.alias+" {module\|id}";`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,7 @@ function command(argv,result) {`
`21`	`21`	`if (!node) {`
`22`	`22`	`return result.help(command);`
`23`	`23`	`}`
`24`		`- return request.request('/nodes/' + node, {}).then(result.logDetails).catch(result.warn);`
	`24`	`+ return request.request('/nodes/' + node, {}).then(result.logDetails);`
`25`	`25`	`}`
`26`	`26`	`command.alias = "info";`
`27`	`27`	`command.usage = command.alias+" {module\|node}";`
Original file line number	Diff line number	Diff line change
`@@ -37,7 +37,7 @@ function command(argv,result) {`
`37`	`37`	`return request.request('/nodes', {`
`38`	`38`	`method: "POST",`
`39`	`39`	`data: data`
`40`		`- }).then(result.logDetails).catch(result.warn);`
	`40`	`+ }).then(result.logDetails);`
`41`	`41`	`}`
`42`	`42`	`command.alias = "install";`
`43`	`43`	`command.usage = command.alias+" <module> [<url>]";`
Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@`
`17`	`17`	`var request = require("../request");`
`18`	`18`
`19`	`19`	`function command(argv,result) {`
`20`		`- return request.request('/nodes', {}).then(result.logNodeList).catch(result.warn);`
	`20`	`+ return request.request('/nodes', {}).then(result.logNodeList);`
`21`	`21`	`}`
`22`	`22`	`command.alias = "list";`
`23`	`23`	`command.usage = command.alias;`
Original file line number	Diff line number	Diff line change
`@@ -29,7 +29,7 @@ function command(argv,result) {`
`29`	`29`	`} else {`
`30`	`30`	`result.log("Uninstalled " + module);`
`31`	`31`	`}`
`32`		`- }).catch(result.warn);`
	`32`	`+ });`
`33`	`33`	`}`
`34`	`34`	`command.alias = "remove";`
`35`	`35`	`command.usage = command.alias+" <module>";`
Original file line number	Diff line number	Diff line change
`@@ -65,10 +65,8 @@ function command(argv,result) {`
`65`	`65`	`}`
`66`	`66`	`}`
`67`	`67`	`} else {`
`68`		`- result.warn(response.status + ": " + response.data);`
	`68`	`+ throw new Error(response.status + ": " + response.data);`
`69`	`69`	`}`
`70`		`- }).catch(err => {`
`71`		`- result.warn(err.toString());`
`72`	`70`	`});`
`73`	`71`
`74`	`72`	`}`