Replace "request" by "got" or "axios"

[guimard]

Hi,

"request" has been deprecated. You should replace it by got or axios

[guimard]

Hi,

here is a patch which succeeds except for "correctly caches remote files" test:

diff --git a/lib/millstone.js b/lib/millstone.js
index 01a59cf..3f09539 100644
--- a/lib/millstone.js
+++ b/lib/millstone.js
@@ -14,7 +14,7 @@ var env = process.env.NODE_ENV || 'development';
 // Third party modules
 var _ = require('underscore'),
     srs = require('srs'),
-    request = require('request'),
+    axios = require('axios'),
     zipfile = require('zipfile'),
     Step = require('step'),
     utils = require('./util.js');
@@ -137,25 +137,17 @@ function download(url, options, callback) {
                 return return_on_error(err);
             } else {
                 if (env == 'development') console.error("[millstone] downloading: '" + url + "'");
-                var req;
-                try {
-                req = request({
-                    url: url,
-                    proxy: process.env.HTTP_PROXY
-                });
-                } catch (err) {
-                    // catch Invalid URI error
-                    return return_on_error(err);
-                }
-                req.on('error', function(err) {
-                    return return_on_error(err);
-                });
-                req.pipe(fs.createWriteStream(dl)).on('error', function(err) {
+                // TODO
+                axios({
+                  method: 'get',
+                  url: url,
+                  responseType: 'stream'
+                })
+                .then( (resp) => {
+                  resp.data.pipe(fs.createWriteStream(dl))
+                  .on('error', function(err) {
                     return return_on_error(err);
-                }).on('close', function() {
-                    if (!req.response || (req.response && req.response.statusCode >= 400)) {
-                        return return_on_error(new Error('server returned ' + req.response.statusCode));
-                    } else {
+                  }).on('close', function() {
                         pool.release(obj);
                         fs.rename(dl, options.filepath, function(err) {
                             if (err) {
@@ -169,9 +161,9 @@ function download(url, options, callback) {
                                 // only use the `content-disposition` header to determine
                                 // what kind of file we downloaded if it doesn't have an
                                 // extension.
-                                var req_meta = _(req.req.res.headers).clone();
-                                if (req.req.res.request && req.req.res.request.path) {
-                                    req_meta.path = req.req.res.request.path;
+                                var req_meta = _(resp.headers).clone();
+                                if (resp.request && resp.request.path) {
+                                    req_meta.path = resp.request.path;
                                 }
                                 fs.writeFile(metapath(options.filepath), JSON.stringify(req_meta), 'utf-8', function(err) {
                                     downloads[dkey].emit('done', err, options.filepath);
@@ -180,8 +172,10 @@ function download(url, options, callback) {
                                 });
                             }
                         });
-                    }
-                });
+                  })
+                }).catch( (err) => {
+                  return return_on_error(err);
+                })
             }
         });
     }
@@ -614,17 +608,15 @@ function resolve(options, callback) {
 
             // URL, download.
             if (uri.protocol && (uri.protocol == 'http:' || uri.protocol == 'https:')) {
-                return request({
-                    url: s,
-                    proxy: process.env.HTTP_PROXY
-                }, function(err, response, data) {
-                    if (err) return next(err);
-
+                axios.get(s)
+                .then( (response) => {
                     resolved.Stylesheet[index] = {
                         id: path.basename(uri.pathname),
-                        data: data.toString()
+                        data: response.data.toString()
                     };
                     localizeCartoURIs(resolved.Stylesheet[index],next);
+                }).catch( (err) => {
+                  return next(err);
                 });
             }
 
diff --git a/package.json b/package.json
index e2a0d3c..cee5afb 100644
--- a/package.json
+++ b/package.json
@@ -22,7 +22,7 @@
         "underscore": "~1.6.0",
         "step": "~0.0.5",
         "generic-pool": "~2.4.1",
-        "request": "2.x",
+        "axios": "^0.20.0",
         "srs": "1.x",
         "zipfile": "~0.5.5",
         "sqlite3": "4.1.0",

[guimard]

Error is:

  28 passing (4s)
  1 failing

  1) correctly caches remote files:
     Uncaught Error: ENOENT: no such file or directory, open '/millstone/test/cache-url.mss'
  



(node:3820402) UnhandledPromiseRejectionWarning: Error: ENOENT: no such file or directory, open '/millstone/test/cache-url.mss'
(node:3820402) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:3820402) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
npm ERR! code 1
npm ERR! path /tmp/tt/millstone
npm ERR! command failed
npm ERR! command sh -c mocha -R spec --timeout 10000

[guimard]

The error commes from next here: https://github.com/tilemill-project/millstone/blob/master/lib/millstone.js#L605

[guimard]

@csytsma: hi, could you help me to finish this ? This is required for next Debian release since request won't be part of it due to deprecation and security risks. Cheers, Xavier

[csytsma]

@guimard What do you need me to do? I can give you collaboration permission on Millstone, so you can make the necessary changes?

[guimard]

@csytsma: I'd like to push a PR, but I'm unable to fix error mentioned above

[guimard]

@csytsma: could you take a look at #145? I'm unable to fix test error