Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to fetch for url requests #1165

Merged
merged 4 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ module.exports = {
it: true,
describe: true,
before: true,
after: true,
test: true,
},
rules: {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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"
},
Expand Down
19 changes: 5 additions & 14 deletions packages/core/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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}>`);
});
}

Expand Down
68 changes: 14 additions & 54 deletions packages/core/src/request.js
Original file line number Diff line number Diff line change
@@ -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));
};
Original file line number Diff line number Diff line change
@@ -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");
Expand Down Expand Up @@ -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"
);
});
}
});
17 changes: 15 additions & 2 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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==
Expand Down Expand Up @@ -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"
Expand Down