Use make-fetch-happen instead of request (#3193)

Fixes #3206


Co-authored-by: Camille Drapier <[email protected]>
Co-authored-by: Michael Mifsud <[email protected]>

Author: 3 people

package.json

@@ -60,11 +60,10 @@
     "get-stdin": "^4.0.1",
     "glob": "^7.0.3",
     "lodash": "^4.17.15",
+    "make-fetch-happen": "^9.1.0",
     "meow": "^9.0.0",
     "nan": "^2.13.2",
     "node-gyp": "^8.4.1",
-    "npmlog": "^5.0.0",
-    "request": "^2.88.0",
     "sass-graph": "4.0.0",
     "stdout-stream": "^1.4.0",
     "true-case-path": "^2.2.1"

scripts/install.js

@@ -5,8 +5,7 @@
 var fs = require('fs'),
   eol = require('os').EOL,
   path = require('path'),
-  request = require('request'),
-  log = require('npmlog'),
+  fetch = require('make-fetch-happen'),
   sass = require('../lib/extensions'),
   downloadOptions = require('./util/downloadoptions');
 
@@ -21,21 +20,8 @@ var fs = require('fs'),
 
 function download(url, dest, cb) {
   var reportError = function(err) {
-    var timeoutMessge;
-
-    if (err.code === 'ETIMEDOUT') {
-      if (err.connect === true) {
-        // timeout is hit while your client is attempting to establish a connection to a remote machine
-        timeoutMessge = 'Timed out attemping to establish a remote connection';
-      } else {
-        timeoutMessge = 'Timed out whilst downloading the prebuilt binary';
-        // occurs any time the server is too slow to send back a part of the response
-      }
-
-    }
     cb(['Cannot download "', url, '": ', eol, eol,
       typeof err.message === 'string' ? err.message : err, eol, eol,
-      timeoutMessge ? timeoutMessge + eol + eol : timeoutMessge,
       'Hint: If github.com is not accessible in your location', eol,
       '      try setting a proxy via HTTP_PROXY, e.g. ', eol, eol,
       '      export HTTP_PROXY=http://example.com:1234',eol, eol,
@@ -44,45 +30,22 @@ function download(url, dest, cb) {
   };
 
   var successful = function(response) {
-    return response.statusCode >= 200 && response.statusCode < 300;
+    return response.status >= 200 && response.status < 300;
   };
 
   console.log('Downloading binary from', url);
 
   try {
-    request(url, downloadOptions(), function(err, response, buffer) {
-      if (err) {
-        reportError(err);
-      } else if (!successful(response)) {
-        reportError(['HTTP error', response.statusCode, response.statusMessage].join(' '));
+    fetch(url, downloadOptions()).then(function (response) {
+      fs.createWriteStream(dest).on('error', cb).end(response.data, cb);
+      console.log('Download complete');
+    }).catch(function(err) {
+      if(!successful(err)) {
+        reportError(['HTTP error', err.code, err.message].join(' '));
       } else {
-        console.log('Download complete');
-
-        if (successful(response)) {
-          fs.createWriteStream(dest)
-            .on('error', cb)
-            .end(buffer, cb);
-        } else {
-          cb();
-        }
+        reportError(err);
       }
-    })
-      .on('response', function(response) {
-        var length = parseInt(response.headers['content-length'], 10);
-        var progress = log.newItem('', length);
-
-        // The `progress` is true by default. However if it has not
-        // been explicitly set it's `undefined` which is considered
-        // as far as npm is concerned.
-        if (process.env.npm_config_progress === 'true') {
-          log.enableProgress();
-
-          response.on('data', function(chunk) {
-            progress.completeWork(chunk.length);
-          })
-            .on('end', progress.finish);
-        }
-      });
+    });
   } catch (err) {
     cb(err);
   }

scripts/util/downloadoptions.js

@@ -3,24 +3,18 @@ var proxy = require('./proxy'),
   rejectUnauthorized = require('./rejectUnauthorized');
 
 /**
- * The options passed to request when downloading the bibary
+ * The options passed to make-fetch-happen when downloading the binary
  *
- * There some nuance to how request handles options. Specifically
- * we've been caught by their usage of `hasOwnProperty` rather than
- * falsey checks. By moving the options generation into a util helper
- * we can test for regressions.
- *
- * @return {Object} an options object for request
+ * @return {Object} an options object for make-fetch-happen
  * @api private
  */
 module.exports = function() {
   var options = {
-    rejectUnauthorized: rejectUnauthorized(),
+    strictSSL: rejectUnauthorized(),
     timeout: 60000,
     headers: {
       'User-Agent': userAgent(),
     },
-    encoding: null,
   };
 
   var proxyConfig = proxy();

test/downloadoptions.js

@@ -8,12 +8,11 @@ describe('util', function() {
     describe('without a proxy', function() {
       it('should look as we expect', function() {
         var expected = {
-          rejectUnauthorized: true,
+          strictSSL: true,
           timeout: 60000,
           headers: {
             'User-Agent': ua(),
           },
-          encoding: null,
         };
 
         assert.deepStrictEqual(opts(), expected);
@@ -33,13 +32,12 @@ describe('util', function() {
 
       it('should look as we expect', function() {
         var expected = {
-          rejectUnauthorized: true,
+          strictSSL: true,
           proxy: proxy,
           timeout: 60000,
           headers: {
             'User-Agent': ua(),
           },
-          encoding: null,
         };
 
         assert.deepStrictEqual(opts(), expected);
@@ -59,12 +57,11 @@ describe('util', function() {
 
       it('should look as we expect', function() {
         var expected = {
-          rejectUnauthorized: true,
+          strictSSL: true,
           timeout: 60000,
           headers: {
             'User-Agent': ua(),
           },
-          encoding: null,
         };
 
         assert.deepStrictEqual(opts(), expected);
@@ -78,12 +75,11 @@ describe('util', function() {
 
       it('should look as we expect', function() {
         var expected = {
-          rejectUnauthorized: false,
+          strictSSL: false,
           timeout: 60000,
           headers: {
             'User-Agent': ua(),
           },
-          encoding: null,
         };
 
         assert.deepStrictEqual(opts(), expected);
@@ -97,12 +93,11 @@ describe('util', function() {
 
       it('should look as we expect', function() {
         var expected = {
-          rejectUnauthorized: true,
+          strictSSL: true,
           timeout: 60000,
           headers: {
             'User-Agent': ua(),
           },
-          encoding: null,
         };
 
         assert.deepStrictEqual(opts(), expected);
@@ -116,12 +111,11 @@ describe('util', function() {
 
       it('should look as we expect', function() {
         var expected = {
-          rejectUnauthorized: true,
+          strictSSL: true,
           timeout: 60000,
           headers: {
             'User-Agent': ua(),
           },
-          encoding: null,
         };
 
         assert.deepStrictEqual(opts(), expected);