MXS-1300: Use native promises for requests

The request-promise-native package adds support for request with native
promises. This is a convenient way to synchronize multiple HTTP requests.

The functions are changed to use promises to make testing easier. With
promises, the testing code can use the chai-as-promised library.

There are a few cases where doRequest is called with a callback that again
calls doAsyncRequest. With multiple hosts, this causes each command to be
propagated to all servers. This is a design flaw of the current multi-host
mode and needs to be changed.

Changed maxctrl.js to wait on the promises to silence some warnings if the
promise rejections are ignored.

maxctrl/common.js

@@ -11,7 +11,7 @@
  * Public License.
  */
 
-var request = require('request');
+var request = require('request-promise-native');
 var colors = require('colors/safe');
 var Table = require('cli-table');
 
@@ -127,46 +127,63 @@ module.exports = function() {
         return base + argv.user + ':' + argv.password + '@' + host + '/v1/' + endpoint
     }
 
-    // Helper for executing requests and handling their responses
+    // Helper for executing requests and handling their responses, returns a
-    this.doRequest = function(resource, cb, obj) {
+    // promise that is fulfilled when all requests successfully complete. The
+    // promise is rejected if any of the requests fails.
+    this.doAsyncRequest = function(resource, cb, obj) {
+        return new Promise(function (resolve, reject) {
             pingCluster(this.argv.hosts)
                 .then(function() {
-                var argv = this.argv
-                argv.hosts.forEach(function(host) {
-                    args = obj || {}
-                    args.uri = getUri(host, argv.secure, resource)
-                    args.json = true
-                    args.timeout = argv.timeout
 
-                    request(args, function(err, resp, res) {
+                    var promises = []
-                        if (err) {
+
-                            // Failed to request
+                    this.argv.hosts.forEach(function(host) {
-                            console.log(colors.yellow(host) + ':')
+                        args = obj || {}
-                            logError(JSON.stringify(err, null, 4))
+                        args.uri = getUri(host, this.argv.secure, resource)
-                        } else if (resp.statusCode == 200 && cb) {
+                        args.json = true
+                        args.timeout = this.argv.timeout
+
+                        var p = request(args)
+                            .then(function(res) {
+                                if (res && cb) {
                                     // Request OK, returns data
-                            if (!argv.tsv) {
+                                    if (!this.argv.tsv) {
                                         console.log(colors.yellow(host) + ':')
                                     }
-                            cb(res)
+                                    return cb(res)
-                        } else if (resp.statusCode == 204) {
-                            // Request OK, no data
-                            console.log(colors.yellow(host) + ': ' + colors.green('OK'))
                                 } else {
-                            // Unexpected return code, probably an error
+                                    // Request OK, no data or data is ignored
-                            var errstr = resp.statusCode + ' ' + resp.statusMessage
+                                    console.log(colors.yellow(host) + ': ' + colors.green('OK'))
-                            if (res) {
-                                errstr += ' ' + JSON.stringify(res, null, 4)
                                 }
+                                return Promise.resolve()
+                            }, function(err) {
                                 console.log(colors.yellow(host) + ':')
-                            logError(errstr)
+                                if (err.response.body) {
+                                    logError(JSON.stringify(err.response.body, null, 4))
+                                } else {
+                                    logError('Server responded with ' + err.statusCode)
                                 }
+                                return Promise.reject()
+                            })
+
+                        // Push the request on to the stack
+                        promises.push(p)
+                    })
+
+                    // Wait for all promises to resolve or reject
+                    Promise.all(promises)
+                        .then(resolve, reject)
+                }, function(err) {
+                    // One of the HTTP request pings to the cluster failed, log the error
+                    logError(JSON.stringify(err.error, null, 4))
+                    reject()
                 })
         })
-            })
+    }
-            .catch(function(err) {
+
-                logError(JSON.stringify(err, null, 4))
+    this.doRequest = function(resource, cb, obj) {
-            })
+        return doAsyncRequest(resource, cb, obj)
+            .then(this.argv.resolve, this.argv.reject)
     }
 
     this.updateValue = function(resource, key, value) {
@@ -178,6 +195,11 @@ module.exports = function() {
     this.logError = function(err) {
         console.log(colors.red('Error:'), err)
     }
+
+    this.error = function(err) {
+        console.log(colors.red('Error:'), err)
+        this.argv.reject()
+    }
 }
 
 var tsvopts = {
@@ -230,25 +252,11 @@ function getTable(headobj) {
     return new Table(opts)
 }
 
-function pingMaxScale(host) {
-    return new Promise(function(resolve, reject) {
-        request('http://' + host + '/v1', function(err, resp, res) {
-            if (err) {
-                reject(err)
-            } else if (resp.statusCode != 200) {
-                reject(resp.statusCode + ' ' + resp.statusMessage)
-            } else {
-                resolve()
-            }
-        })
-    })
-}
-
 function pingCluster(hosts) {
     var promises = []
 
     hosts.forEach(function(i) {
-        promises.push(pingMaxScale(i))
+        promises.push(request('http://' + i + '/v1'))
     })
 
     return Promise.all(promises)

maxctrl/core.js

@@ -82,12 +82,6 @@ module.exports = function(argv) {
             .command('*', 'the default command', {}, () => {
                 console.log('Unknown command. See output of `help` for a list of commands.')
             })
-            .parse(process.argv, function(err, argv, output) {
+            .parse(argv, {resolve: resolve, reject: reject})
-                if (err) {
-                    reject(output)
-                } else {
-                    resolve(output);
-                }
-            })
     })
 }

maxctrl/lib/call.js

@@ -32,8 +32,8 @@ exports.builder = function(yargs) {
                         }
                     })
 
-                    maxctrl
+                    return maxctrl
-                        .doRequest('maxscale/modules/' + argv.module + '/' + argv.command + '?' + argv.parameters.join('&'),
+                        .doAsyncRequest('maxscale/modules/' + argv.module + '/' + argv.command + '?' + argv.parameters.join('&'),
                                         function(resp) {
                                             console.log(JSON.stringify(resp, null, 4))
                                         }, { method: verb })

maxctrl/lib/disable.js

@@ -30,7 +30,7 @@ exports.builder = function(yargs) {
                     .updateValue('maxscale/logs', 'data.attributes.parameters.log_' + argv.log, false)
             } else {
                 maxctrl(argv)
-                    .logError('Invalid log priority: ' + argv.log);
+                    .error('Invalid log priority: ' + argv.log);
             }
         })
         .command('maxlog', 'Disable MaxScale logging', {}, function(argv) {

maxctrl/lib/enable.js

@@ -30,7 +30,7 @@ exports.builder = function(yargs) {
                     .updateValue('maxscale/logs', 'data.attributes.parameters.log_' + argv.log, true)
             } else {
                 maxctrl(argv)
-                    .logError('Invalid log priority: ' + argv.log);
+                    .error('Invalid log priority: ' + argv.log);
             }
         })
         .command('maxlog', 'Enable MaxScale logging', {}, function(argv) {

maxctrl/lib/link.js

@@ -25,8 +25,8 @@ function addServer(argv, path, targets) {
             _.set(res, 'data.relationships.servers.data', servers)
             delete res.data.attributes
 
-            maxctrl(argv)
+            return maxctrl(argv)
-                .doRequest(path, null, {method: 'PATCH', body: res})
+                .doAsyncRequest(path, null, {method: 'PATCH', body: res})
         })
 }
 

maxctrl/lib/unlink.js

@@ -25,8 +25,8 @@ function removeServer(argv, path, targets) {
             _.set(res, 'data.relationships.servers.data', servers)
             delete res.data.attributes
 
-            maxctrl(argv)
+            return maxctrl(argv)
-                .doRequest(path, null, {method: 'PATCH', body: res})
+                .doAsyncRequest(path, null, {method: 'PATCH', body: res})
         })
 }
 

maxctrl/maxctrl.js

@@ -13,18 +13,14 @@
 
 'use strict';
 
-var argv = process.argv
+var maxctrl = require('./core.js')
 
 // Mangle the arguments if we are being called from the command line
-if (argv[0] == process.execPath) {
+if (process.argv[0] == process.execPath) {
-    argv.shift()
+    process.argv.shift()
     // The first argument is always the script
-    argv.shift()
+    process.argv.shift()
 }
 
-require('./core.js')(argv)
+maxctrl(process.argv)
-    .then(function(output){
+    .then(function(out) {}, function(out) {})
-        if (output.length > 0) {
-            console.log(output)
-        }
-    }, console.log)

maxctrl/package.json

@@ -20,6 +20,7 @@
     "lodash": "^4.17.4",
     "lodash-getpath": "^0.2.4",
     "request": "^2.81.0",
+    "request-promise-native": "^1.0.3",
     "yargs": "^8.0.2"
   }
 }