Switch to fetch for url requests (#1165)

* Change load from url request to use fetch

* Remove phin dependency

* Fix linting for using after in tests

* Fix http server being created for browser tests in core request

.eslintrc.js

@@ -41,6 +41,7 @@ module.exports = {
         it: true,
         describe: true,
         before: true,
+        after: true,
         test: true,
       },
       rules: {

packages/core/package.json

@@ -34,8 +34,8 @@
     "buffer": "^5.2.0",
     "exif-parser": "^0.1.12",
     "file-type": "^16.5.4",
+    "isomorphic-fetch": "^3.0.0",
     "mkdirp": "^0.5.1",
-    "phin": "^2.9.1",
     "pixelmatch": "^4.0.2",
     "tinycolor2": "^1.4.1"
   },

packages/core/src/index.js

@@ -58,29 +58,20 @@ function bufferFromArrayBuffer(arrayBuffer) {
 }
 
 function loadFromURL(options, cb) {
-  request(options, (err, response, data) => {
+  request(options, (err, data) => {
     if (err) {
       return cb(err);
     }
 
-    if ("headers" in response && "location" in response.headers) {
-      options.url = response.headers.location;
-      return loadFromURL(options, cb);
-    }
-
     if (typeof data === "object" && Buffer.isBuffer(data)) {
       return cb(null, data);
     }
 
-    const msg =
-      "Could not load Buffer from <" +
-      options.url +
-      "> " +
-      "(HTTP: " +
-      response.statusCode +
-      ")";
+    if (typeof data === "object" && isArrayBuffer(data)) {
+      return cb(null, bufferFromArrayBuffer(data));
+    }
 
-    return new Error(msg);
+    return new Error(`Could not load Buffer from <${options.url}>`);
   });
 }
 

packages/core/src/request.js

@@ -1,58 +1,18 @@
-/* global XMLHttpRequest */
+import "isomorphic-fetch";
 
-if (
-  process.browser ||
-  process.env.ENVIRONMENT === "BROWSER" ||
-  (typeof process.versions.electron !== "undefined" &&
-    process.type === "renderer" &&
-    typeof XMLHttpRequest === "function")
-) {
-  // If we run into a browser or the electron renderer process,
-  // use XHR method instead of Request node module.
-
-  module.exports = function (options, cb) {
-    const xhr = new XMLHttpRequest();
-    xhr.open("GET", options.url, true);
-    xhr.responseType = "arraybuffer";
-    xhr.addEventListener("load", function () {
-      if (xhr.status < 400) {
-        try {
-          const data = Buffer.from(this.response);
-          cb(null, xhr, data);
-        } catch (error) {
-          return cb(
-            new Error(
-              "Response is not a buffer for url " +
-                options.url +
-                ". Error: " +
-                error.message
-            )
+export default ({ url, ...options }, cb) => {
+  fetch(url, options)
+    .then((response) => {
+      if (response.ok) {
+        return response.arrayBuffer().catch((error) => {
+          throw new Error(
+            `Response is not a buffer for url ${url}. Error: ${error.message}`
           );
-        }
-      } else {
-        cb(new Error("HTTP Status " + xhr.status + " for url " + options.url));
+        });
       }
-    });
-    xhr.addEventListener("error", (e) => {
-      cb(e);
-    });
-    xhr.send();
-  };
-} else {
-  module.exports = function ({ ...options }, cb) {
-    const p = require("phin");
-    const allOptions = { compression: true, ...options };
 
-    try {
-      p(allOptions, (err, res) => {
-        if (err) {
-          cb(err);
-        } else {
-          cb(null, res, res.body);
-        }
-      });
-    } catch (error) {
-      cb(error);
-    }
-  };
-}
+      throw new Error(`HTTP Status ${response.status} for url ${url}`);
+    })
+    .then((data) => cb(null, data))
+    .catch((error) => cb(error));
+};

packages/core/test/redirect.test.js → packages/core/test/request.test.js

@@ -1,4 +1,5 @@
 import { Jimp as jimp, getTestDir } from "@jimp/test-utils";
+import expect from "@storybook/expect";
 
 const fs = require("fs");
 const http = require("http");
@@ -33,26 +34,33 @@ const httpHandler = (req, res) => {
   }
 };
 
-describe("redirect", () => {
+describe("request", () => {
   if (typeof window !== "undefined" && typeof window.document !== "undefined") {
-    xit("Not testing redirects in browser");
+    xit("Not testing requests in browser");
   } else {
     const httpServer = http.createServer(httpHandler);
     before(() => {
       httpServer.listen(5136);
     });
 
-    it("follows 301 redirect", (done) => {
-      jimp
-        .read("http://localhost:5136/redirect.png")
-        .then(() => {
-          httpServer.close();
-          done();
-        })
-        .catch((error) => {
-          httpServer.close();
-          done(error);
-        });
+    after(() => {
+      httpServer.close();
+    });
+
+    it("loads standard response", async () => {
+      await jimp.read("http://localhost:5136/corrected.png");
+    });
+
+    it("follows 301 redirect", async () => {
+      await jimp.read("http://localhost:5136/redirect.png");
+    });
+
+    it("reports 404 correctly", () => {
+      return expect(
+        jimp.read("http://localhost:5136/not-found.png")
+      ).rejects.toThrow(
+        "HTTP Status 404 for url http://localhost:5136/not-found.png"
+      );
     });
   }
 });

yarn.lock

@@ -1401,8 +1401,8 @@
     buffer "^5.2.0"
     exif-parser "^0.1.12"
     file-type "^16.5.4"
+    isomorphic-fetch "^3.0.0"
     mkdirp "^0.5.1"
-    phin "^2.9.1"
     pixelmatch "^4.0.2"
     tinycolor2 "^1.4.1"
 
@@ -7309,6 +7309,14 @@ isobject@^4.0.0:
   resolved "https://registry.yarnpkg.com/isobject/-/isobject-4.0.0.tgz#3f1c9155e73b192022a80819bacd0343711697b0"
   integrity sha512-S/2fF5wH8SJA/kmwr6HYhK/RI/OkhD84k8ntalo0iJjZikgq1XFvR5M8NPT1x5F7fBwCG3qHfnzeP/Vh/ZxCUA==
 
+isomorphic-fetch@^3.0.0:
+  version "3.0.0"
+  resolved "https://registry.yarnpkg.com/isomorphic-fetch/-/isomorphic-fetch-3.0.0.tgz#0267b005049046d2421207215d45d6a262b8b8b4"
+  integrity sha512-qvUtwJ3j6qwsF3jLxkZ72qCgjMysPzDfeV240JHiGZsANBYd+EEuu35v7dfrJ9Up0Ak07D7GGSkGhCHTqg/5wA==
+  dependencies:
+    node-fetch "^2.6.1"
+    whatwg-fetch "^3.4.1"
+
 isstream@~0.1.2:
   version "0.1.2"
   resolved "https://registry.yarnpkg.com/isstream/-/isstream-0.1.2.tgz#47e63f7af55afa6f92e1500e690eb8b8529c099a"
@@ -8584,7 +8592,7 @@ node-fetch@^2.3.0, node-fetch@^2.5.0:
   resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.6.0.tgz#e633456386d4aa55863f676a7ab0daa8fdecb0fd"
   integrity sha512-8dG4H5ujfvFiqDmVu9fQ5bOHUC15JMjMY/Zumv26oOvvVJjM67KF8koCWIabKQ1GJIa9r2mMZscBq/TbdOcmNA==
 
-node-fetch@^2.6.0, node-fetch@^2.6.7:
+node-fetch@^2.6.0, node-fetch@^2.6.1, node-fetch@^2.6.7:
   version "2.6.9"
   resolved "https://registry.npmjs.org/node-fetch/-/node-fetch-2.6.9.tgz#7c7f744b5cc6eb5fd404e0c7a9fec630a55657e6"
   integrity sha512-DJm/CJkZkRjKKj4Zi4BsKVZh3ValV5IR5s7LVZnW+6YMh0W1BfNA8XSs6DLMGYlId5F3KnA70uu2qepcR08Qqg==
@@ -11722,6 +11730,11 @@ webpack@^5.75.0:
     watchpack "^2.4.0"
     webpack-sources "^3.2.3"
 
+whatwg-fetch@^3.4.1:
+  version "3.6.2"
+  resolved "https://registry.yarnpkg.com/whatwg-fetch/-/whatwg-fetch-3.6.2.tgz#dced24f37f2624ed0281725d51d0e2e3fe677f8c"
+  integrity sha512-bJlen0FcuU/0EMLrdbJ7zOnW6ITZLrZMIarMUVmdKtsGvZna8vxKYaexICWPfZ8qwf9fzNq+UEIZrnSaApt6RA==
+
 whatwg-url@^5.0.0:
   version "5.0.0"
   resolved "https://registry.npmjs.org/whatwg-url/-/whatwg-url-5.0.0.tgz#966454e8765462e37644d3626f6742ce8b70965d"