use internal fetch as replacement for node-fetch (#3184)

related to #2649

I was able to move to internal fetch and all tests seems fine so far.

But we have one problem with the calendar module. In the docs we have
several authentication methods and one of them is `digest`. For this we
used `digest-fetch` which needs `node-fetch` (this is not so clear from
code but I was not able to get it working).

So we have 3 options:
- remove `digest` as authentication method for calendar module (this is
what this PR does at the moment)
- find an alternative npm package or implement the digest stuff
ourselves
- use `digest-fetch` and `node-fetch` for calendar module (so they would
remain as dependencies in `package.json`)

Opinions? @KristjanESPERANTO @rejas @sdetweil @MichMich
This commit is contained in:
Karsten Hassel 2023-09-09 21:12:31 +02:00 committed by GitHub
parent ffdf321e23
commit f2957f90df
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 24 additions and 115 deletions

View file

@ -9,6 +9,8 @@ This project adheres to [Semantic Versioning](https://semver.org/).
_This release is scheduled to be released on 2023-10-01._ _This release is scheduled to be released on 2023-10-01._
> ⚠️ This release needs nodejs version > `v18`, older release have reached end of life and will not work!
### Added ### Added
- Added UV Index support to OpenWeatherMap - Added UV Index support to OpenWeatherMap
@ -20,6 +22,8 @@ _This release is scheduled to be released on 2023-10-01._
### Removed ### Removed
- **Breaking Change**: Removed `digest` authentication method from calendar module (which was already broken since release `2.15.0`)
### Updated ### Updated
- Update roboto fonts to version v5 - Update roboto fonts to version v5
@ -29,6 +33,7 @@ _This release is scheduled to be released on 2023-10-01._
- Update engine node >=18. v16 reached it's end of life. (#3170) - Update engine node >=18. v16 reached it's end of life. (#3170)
- Update typescript definition for modules - Update typescript definition for modules
- Cleaned up nunjuck templates - Cleaned up nunjuck templates
- Replace `node-fetch` with internal fetch (#2649) and remove `digest-fetch`
### Fixed ### Fixed

View file

@ -1,20 +0,0 @@
/**
* Helper class to provide either third party fetch library or (if node >= 18)
* return internal node fetch implementation.
*
* Attention: After some discussion we always return the third party
* implementation until the node implementation is stable and more tested
* @see https://github.com/MichMich/MagicMirror/pull/2952
* @see https://github.com/MichMich/MagicMirror/issues/2649
* @param {string} url to be fetched
* @param {object} options object e.g. for headers
* @class
*/
async function fetch(url, options = {}) {
// return global.fetch(url, options);
const nodefetch = require("node-fetch");
return nodefetch(url, options);
}
module.exports = fetch;

View file

@ -1,7 +1,6 @@
const fs = require("fs"); const fs = require("fs");
const path = require("path"); const path = require("path");
const Log = require("logger"); const Log = require("logger");
const fetch = require("./fetch");
/** /**
* Gets the config. * Gets the config.

View file

@ -6,9 +6,7 @@
*/ */
const https = require("https"); const https = require("https");
const digest = require("digest-fetch");
const ical = require("node-ical"); const ical = require("node-ical");
const fetch = require("fetch");
const Log = require("logger"); const Log = require("logger");
const NodeHelper = require("node_helper"); const NodeHelper = require("node_helper");
const CalendarFetcherUtils = require("./calendarfetcherutils"); const CalendarFetcherUtils = require("./calendarfetcherutils");
@ -39,7 +37,6 @@ const CalendarFetcher = function (url, reloadInterval, excludedEvents, maximumEn
clearTimeout(reloadTimer); clearTimeout(reloadTimer);
reloadTimer = null; reloadTimer = null;
const nodeVersion = Number(process.version.match(/^v(\d+\.\d+)/)[1]); const nodeVersion = Number(process.version.match(/^v(\d+\.\d+)/)[1]);
let fetcher = null;
let httpsAgent = null; let httpsAgent = null;
let headers = { let headers = {
"User-Agent": `Mozilla/5.0 (Node.js ${nodeVersion}) MagicMirror/${global.version}` "User-Agent": `Mozilla/5.0 (Node.js ${nodeVersion}) MagicMirror/${global.version}`
@ -53,17 +50,12 @@ const CalendarFetcher = function (url, reloadInterval, excludedEvents, maximumEn
if (auth) { if (auth) {
if (auth.method === "bearer") { if (auth.method === "bearer") {
headers.Authorization = `Bearer ${auth.pass}`; headers.Authorization = `Bearer ${auth.pass}`;
} else if (auth.method === "digest") {
fetcher = new digest(auth.user, auth.pass).fetch(url, { headers: headers, agent: httpsAgent });
} else { } else {
headers.Authorization = `Basic ${Buffer.from(`${auth.user}:${auth.pass}`).toString("base64")}`; headers.Authorization = `Basic ${Buffer.from(`${auth.user}:${auth.pass}`).toString("base64")}`;
} }
} }
if (fetcher === null) {
fetcher = fetch(url, { headers: headers, agent: httpsAgent });
}
fetcher fetch(url, { headers: headers, agent: httpsAgent })
.then(NodeHelper.checkFetchStatus) .then(NodeHelper.checkFetchStatus)
.then((response) => response.text()) .then((response) => response.text())
.then((responseData) => { .then((responseData) => {

View file

@ -8,7 +8,6 @@
const stream = require("stream"); const stream = require("stream");
const FeedMe = require("feedme"); const FeedMe = require("feedme");
const iconv = require("iconv-lite"); const iconv = require("iconv-lite");
const fetch = require("fetch");
const Log = require("logger"); const Log = require("logger");
const NodeHelper = require("node_helper"); const NodeHelper = require("node_helper");

51
package-lock.json generated
View file

@ -12,7 +12,6 @@
"dependencies": { "dependencies": {
"colors": "^1.4.0", "colors": "^1.4.0",
"console-stamp": "^3.1.2", "console-stamp": "^3.1.2",
"digest-fetch": "^2.0.3",
"envsub": "^4.1.0", "envsub": "^4.1.0",
"eslint": "^8.48.0", "eslint": "^8.48.0",
"express": "^4.18.2", "express": "^4.18.2",
@ -23,7 +22,6 @@
"luxon": "^1.28.1", "luxon": "^1.28.1",
"module-alias": "^2.2.3", "module-alias": "^2.2.3",
"moment": "^2.29.4", "moment": "^2.29.4",
"node-fetch": "^2.6.12",
"node-ical": "^0.16.1", "node-ical": "^0.16.1",
"socket.io": "^4.7.2" "socket.io": "^4.7.2"
}, },
@ -3432,17 +3430,6 @@
"node": "^14.15.0 || ^16.10.0 || >=18.0.0" "node": "^14.15.0 || ^16.10.0 || >=18.0.0"
} }
}, },
"node_modules/digest-fetch": {
"version": "2.0.3",
"resolved": "https://registry.npmjs.org/digest-fetch/-/digest-fetch-2.0.3.tgz",
"integrity": "sha512-HuTjHQE+wplAR+H8/YGwQjIGR1RQUCEsQcRyp3dZfuuxpSQH4OTm4BkHxyXuzxwmxUrNVzIPf9XkXi8QMJDNwQ==",
"dependencies": {
"base-64": "^0.1.0",
"js-sha256": "^0.9.0",
"js-sha512": "^0.8.0",
"md5": "^2.3.0"
}
},
"node_modules/dir-glob": { "node_modules/dir-glob": {
"version": "3.0.1", "version": "3.0.1",
"resolved": "https://registry.npmjs.org/dir-glob/-/dir-glob-3.0.1.tgz", "resolved": "https://registry.npmjs.org/dir-glob/-/dir-glob-3.0.1.tgz",
@ -7291,44 +7278,6 @@
"isarray": "0.0.1" "isarray": "0.0.1"
} }
}, },
"node_modules/node-fetch": {
"version": "2.7.0",
"resolved": "https://registry.npmjs.org/node-fetch/-/node-fetch-2.7.0.tgz",
"integrity": "sha512-c4FRfUm/dbcWZ7U+1Wq0AwCyFL+3nt2bEw05wfxSz+DWpWsitgmSgYmy2dQdWyKC1694ELPqMs/YzUSNozLt8A==",
"dependencies": {
"whatwg-url": "^5.0.0"
},
"engines": {
"node": "4.x || >=6.0.0"
},
"peerDependencies": {
"encoding": "^0.1.0"
},
"peerDependenciesMeta": {
"encoding": {
"optional": true
}
}
},
"node_modules/node-fetch/node_modules/tr46": {
"version": "0.0.3",
"resolved": "https://registry.npmjs.org/tr46/-/tr46-0.0.3.tgz",
"integrity": "sha512-N3WMsuqV66lT30CrXNbEjx4GEwlow3v6rr4mCcv6prnfwhS01rkgyFdjPNBYd9br7LpXV1+Emh01fHnq2Gdgrw=="
},
"node_modules/node-fetch/node_modules/webidl-conversions": {
"version": "3.0.1",
"resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz",
"integrity": "sha512-2JAn3z8AR6rjK8Sm8orRC0h/bcl/DqL7tRPdGZ4I1CjdF+EaMLmYxBHyXuKL849eucPFhvBoxMsflfOb8kxaeQ=="
},
"node_modules/node-fetch/node_modules/whatwg-url": {
"version": "5.0.0",
"resolved": "https://registry.npmjs.org/whatwg-url/-/whatwg-url-5.0.0.tgz",
"integrity": "sha512-saE57nupxk6v3HY35+jzBwYa0rKSy0XR8JSxZPwgLr7ys0IBzhGviA1/TUGJLmSVqs8pb9AnvICXEuOHLprYTw==",
"dependencies": {
"tr46": "~0.0.3",
"webidl-conversions": "^3.0.0"
}
},
"node_modules/node-ical": { "node_modules/node-ical": {
"version": "0.16.1", "version": "0.16.1",
"resolved": "https://registry.npmjs.org/node-ical/-/node-ical-0.16.1.tgz", "resolved": "https://registry.npmjs.org/node-ical/-/node-ical-0.16.1.tgz",

View file

@ -74,7 +74,6 @@
"dependencies": { "dependencies": {
"colors": "^1.4.0", "colors": "^1.4.0",
"console-stamp": "^3.1.2", "console-stamp": "^3.1.2",
"digest-fetch": "^2.0.3",
"envsub": "^4.1.0", "envsub": "^4.1.0",
"eslint": "^8.48.0", "eslint": "^8.48.0",
"express": "^4.18.2", "express": "^4.18.2",
@ -85,7 +84,6 @@
"luxon": "^1.28.1", "luxon": "^1.28.1",
"module-alias": "^2.2.3", "module-alias": "^2.2.3",
"moment": "^2.29.4", "moment": "^2.29.4",
"node-fetch": "^2.6.12",
"node-ical": "^0.16.1", "node-ical": "^0.16.1",
"socket.io": "^4.7.2" "socket.io": "^4.7.2"
}, },
@ -96,8 +94,7 @@
}, },
"_moduleAliases": { "_moduleAliases": {
"node_helper": "js/node_helper.js", "node_helper": "js/node_helper.js",
"logger": "js/logger.js", "logger": "js/logger.js"
"fetch": "js/fetch.js"
}, },
"engines": { "engines": {
"node": ">=18" "node": ">=18"

View file

@ -10,12 +10,12 @@ describe("App environment", () => {
}); });
it("get request from http://localhost:8080 should return 200", async () => { it("get request from http://localhost:8080 should return 200", async () => {
const res = await helpers.fetch("http://localhost:8080"); const res = await fetch("http://localhost:8080");
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
it("get request from http://localhost:8080/nothing should return 404", async () => { it("get request from http://localhost:8080/nothing should return 404", async () => {
const res = await helpers.fetch("http://localhost:8080/nothing"); const res = await fetch("http://localhost:8080/nothing");
expect(res.status).toBe(404); expect(res.status).toBe(404);
}); });

View file

@ -22,7 +22,7 @@ describe("All font files from roboto.css should be downloadable", () => {
test.each(fontFiles)("should return 200 HTTP code for file '%s'", async (fontFile) => { test.each(fontFiles)("should return 200 HTTP code for file '%s'", async (fontFile) => {
const fontUrl = `http://localhost:8080/fonts/${fontFile}`; const fontUrl = `http://localhost:8080/fonts/${fontFile}`;
const res = await helpers.fetch(fontUrl); const res = await fetch(fontUrl);
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
}); });

View file

@ -1,5 +1,4 @@
const jsdom = require("jsdom"); const jsdom = require("jsdom");
const corefetch = require("fetch");
exports.startApplication = async (configFilename, exec) => { exports.startApplication = async (configFilename, exec) => {
jest.resetModules(); jest.resetModules();
@ -31,7 +30,7 @@ exports.getDocument = () => {
const url = `http://${config.address || "localhost"}:${config.port || "8080"}`; const url = `http://${config.address || "localhost"}:${config.port || "8080"}`;
jsdom.JSDOM.fromURL(url, { resources: "usable", runScripts: "dangerously" }).then((dom) => { jsdom.JSDOM.fromURL(url, { resources: "usable", runScripts: "dangerously" }).then((dom) => {
dom.window.name = "jsdom"; dom.window.name = "jsdom";
dom.window.fetch = corefetch; dom.window.fetch = fetch;
dom.window.onload = () => { dom.window.onload = () => {
global.document = dom.window.document; global.document = dom.window.document;
resolve(); resolve();
@ -80,14 +79,6 @@ exports.waitForAllElements = (selector) => {
}); });
}; };
exports.fetch = (url) => {
return new Promise((resolve) => {
corefetch(url).then((res) => {
resolve(res);
});
});
};
exports.testMatch = async (element, regex) => { exports.testMatch = async (element, regex) => {
const elem = await this.waitForElement(element); const elem = await this.waitForElement(element);
expect(elem).not.toBe(null); expect(elem).not.toBe(null);

View file

@ -10,7 +10,7 @@ describe("ipWhitelist directive configuration", () => {
}); });
it("should return 403", async () => { it("should return 403", async () => {
const res = await helpers.fetch("http://localhost:8181"); const res = await fetch("http://localhost:8181");
expect(res.status).toBe(403); expect(res.status).toBe(403);
}); });
}); });
@ -24,7 +24,7 @@ describe("ipWhitelist directive configuration", () => {
}); });
it("should return 200", async () => { it("should return 200", async () => {
const res = await helpers.fetch("http://localhost:8282"); const res = await fetch("http://localhost:8282");
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
}); });

View file

@ -10,7 +10,7 @@ describe("port directive configuration", () => {
}); });
it("should return 200", async () => { it("should return 200", async () => {
const res = await helpers.fetch("http://localhost:8090"); const res = await fetch("http://localhost:8090");
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
}); });
@ -24,7 +24,7 @@ describe("port directive configuration", () => {
}); });
it("should return 200", async () => { it("should return 200", async () => {
const res = await helpers.fetch("http://localhost:8100"); const res = await fetch("http://localhost:8100");
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
}); });

View file

@ -17,12 +17,12 @@ describe("App environment", () => {
}); });
it("get request from http://localhost:8080 should return 200", async () => { it("get request from http://localhost:8080 should return 200", async () => {
const res = await helpers.fetch("http://localhost:8080"); const res = await fetch("http://localhost:8080");
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
it("get request from http://localhost:8080/nothing should return 404", async () => { it("get request from http://localhost:8080/nothing should return 404", async () => {
const res = await helpers.fetch("http://localhost:8080/nothing"); const res = await fetch("http://localhost:8080/nothing");
expect(res.status).toBe(404); expect(res.status).toBe(404);
}); });
}); });

View file

@ -9,7 +9,7 @@ describe("templated config with port variable", () => {
}); });
it("should return 200", async () => { it("should return 200", async () => {
const res = await helpers.fetch("http://localhost:8090"); const res = await fetch("http://localhost:8090");
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
}); });

View file

@ -14,7 +14,7 @@ describe("Vendors", () => {
Object.keys(vendors).forEach((vendor) => { Object.keys(vendors).forEach((vendor) => {
it(`should return 200 HTTP code for vendor "${vendor}"`, async () => { it(`should return 200 HTTP code for vendor "${vendor}"`, async () => {
const urlVendor = `http://localhost:8080/vendor/${vendors[vendor]}`; const urlVendor = `http://localhost:8080/vendor/${vendors[vendor]}`;
const res = await helpers.fetch(urlVendor); const res = await fetch(urlVendor);
expect(res.status).toBe(200); expect(res.status).toBe(200);
}); });
}); });
@ -22,7 +22,7 @@ describe("Vendors", () => {
Object.keys(vendors).forEach((vendor) => { Object.keys(vendors).forEach((vendor) => {
it(`should return 404 HTTP code for vendor https://localhost/"${vendor}"`, async () => { it(`should return 404 HTTP code for vendor https://localhost/"${vendor}"`, async () => {
const urlVendor = `http://localhost:8080/${vendors[vendor]}`; const urlVendor = `http://localhost:8080/${vendors[vendor]}`;
const res = await helpers.fetch(urlVendor); const res = await fetch(urlVendor);
expect(res.status).toBe(404); expect(res.status).toBe(404);
}); });
}); });

View file

@ -8,13 +8,9 @@ describe("server_functions tests", () => {
let corsResponse; let corsResponse;
let request; let request;
jest.mock("node-fetch");
let nodefetch = require("node-fetch");
let fetchMock; let fetchMock;
beforeEach(() => { beforeEach(() => {
nodefetch.mockReset();
fetchResponseHeadersGet = jest.fn(() => {}); fetchResponseHeadersGet = jest.fn(() => {});
fetchResponseHeadersText = jest.fn(() => {}); fetchResponseHeadersText = jest.fn(() => {});
fetchResponse = { fetchResponse = {
@ -23,10 +19,11 @@ describe("server_functions tests", () => {
}, },
text: fetchResponseHeadersText text: fetchResponseHeadersText
}; };
jest.mock("node-fetch", () => jest.fn()); // eslint-disable-next-line
nodefetch.mockImplementation(() => fetchResponse); fetch = jest.fn();
fetch.mockImplementation(() => fetchResponse);
fetchMock = nodefetch; fetchMock = fetch;
corsResponse = { corsResponse = {
set: jest.fn(() => {}), set: jest.fn(() => {}),