From f2957f90df66a2392b190e68a923f97c9087892b Mon Sep 17 00:00:00 2001 From: Karsten Hassel Date: Sat, 9 Sep 2023 21:12:31 +0200 Subject: [PATCH] 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 --- CHANGELOG.md | 5 ++ js/fetch.js | 20 -------- js/server_functions.js | 1 - modules/default/calendar/calendarfetcher.js | 10 +--- modules/default/newsfeed/newsfeedfetcher.js | 1 - package-lock.json | 51 ------------------- package.json | 5 +- tests/e2e/env_spec.js | 4 +- tests/e2e/fonts_spec.js | 2 +- tests/e2e/helpers/global-setup.js | 11 +--- tests/e2e/ipWhitelist_spec.js | 4 +- tests/e2e/port_spec.js | 4 +- tests/e2e/serveronly_spec.js | 4 +- tests/e2e/template_spec.js | 2 +- tests/e2e/vendor_spec.js | 4 +- tests/unit/functions/server_functions_spec.js | 11 ++-- 16 files changed, 24 insertions(+), 115 deletions(-) delete mode 100644 js/fetch.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 02b9e783b5..9a1f0603aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 needs nodejs version > `v18`, older release have reached end of life and will not work! + ### Added - Added UV Index support to OpenWeatherMap @@ -20,6 +22,8 @@ _This release is scheduled to be released on 2023-10-01._ ### Removed +- **Breaking Change**: Removed `digest` authentication method from calendar module (which was already broken since release `2.15.0`) + ### Updated - 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 typescript definition for modules - Cleaned up nunjuck templates +- Replace `node-fetch` with internal fetch (#2649) and remove `digest-fetch` ### Fixed diff --git a/js/fetch.js b/js/fetch.js deleted file mode 100644 index cf195993d5..0000000000 --- a/js/fetch.js +++ /dev/null @@ -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; diff --git a/js/server_functions.js b/js/server_functions.js index 8e9d9aa91d..ef418e3244 100644 --- a/js/server_functions.js +++ b/js/server_functions.js @@ -1,7 +1,6 @@ const fs = require("fs"); const path = require("path"); const Log = require("logger"); -const fetch = require("./fetch"); /** * Gets the config. diff --git a/modules/default/calendar/calendarfetcher.js b/modules/default/calendar/calendarfetcher.js index c7b62960d8..3ae9bcd315 100644 --- a/modules/default/calendar/calendarfetcher.js +++ b/modules/default/calendar/calendarfetcher.js @@ -6,9 +6,7 @@ */ const https = require("https"); -const digest = require("digest-fetch"); const ical = require("node-ical"); -const fetch = require("fetch"); const Log = require("logger"); const NodeHelper = require("node_helper"); const CalendarFetcherUtils = require("./calendarfetcherutils"); @@ -39,7 +37,6 @@ const CalendarFetcher = function (url, reloadInterval, excludedEvents, maximumEn clearTimeout(reloadTimer); reloadTimer = null; const nodeVersion = Number(process.version.match(/^v(\d+\.\d+)/)[1]); - let fetcher = null; let httpsAgent = null; let headers = { "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.method === "bearer") { headers.Authorization = `Bearer ${auth.pass}`; - } else if (auth.method === "digest") { - fetcher = new digest(auth.user, auth.pass).fetch(url, { headers: headers, agent: httpsAgent }); } else { 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((response) => response.text()) .then((responseData) => { diff --git a/modules/default/newsfeed/newsfeedfetcher.js b/modules/default/newsfeed/newsfeedfetcher.js index 51d38f83fb..f61867486c 100644 --- a/modules/default/newsfeed/newsfeedfetcher.js +++ b/modules/default/newsfeed/newsfeedfetcher.js @@ -8,7 +8,6 @@ const stream = require("stream"); const FeedMe = require("feedme"); const iconv = require("iconv-lite"); -const fetch = require("fetch"); const Log = require("logger"); const NodeHelper = require("node_helper"); diff --git a/package-lock.json b/package-lock.json index d7432ab870..20c00a4d7c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,7 +12,6 @@ "dependencies": { "colors": "^1.4.0", "console-stamp": "^3.1.2", - "digest-fetch": "^2.0.3", "envsub": "^4.1.0", "eslint": "^8.48.0", "express": "^4.18.2", @@ -23,7 +22,6 @@ "luxon": "^1.28.1", "module-alias": "^2.2.3", "moment": "^2.29.4", - "node-fetch": "^2.6.12", "node-ical": "^0.16.1", "socket.io": "^4.7.2" }, @@ -3432,17 +3430,6 @@ "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": { "version": "3.0.1", "resolved": "https://registry.npmjs.org/dir-glob/-/dir-glob-3.0.1.tgz", @@ -7291,44 +7278,6 @@ "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": { "version": "0.16.1", "resolved": "https://registry.npmjs.org/node-ical/-/node-ical-0.16.1.tgz", diff --git a/package.json b/package.json index 9b0da64144..3f5fafd6b0 100644 --- a/package.json +++ b/package.json @@ -74,7 +74,6 @@ "dependencies": { "colors": "^1.4.0", "console-stamp": "^3.1.2", - "digest-fetch": "^2.0.3", "envsub": "^4.1.0", "eslint": "^8.48.0", "express": "^4.18.2", @@ -85,7 +84,6 @@ "luxon": "^1.28.1", "module-alias": "^2.2.3", "moment": "^2.29.4", - "node-fetch": "^2.6.12", "node-ical": "^0.16.1", "socket.io": "^4.7.2" }, @@ -96,8 +94,7 @@ }, "_moduleAliases": { "node_helper": "js/node_helper.js", - "logger": "js/logger.js", - "fetch": "js/fetch.js" + "logger": "js/logger.js" }, "engines": { "node": ">=18" diff --git a/tests/e2e/env_spec.js b/tests/e2e/env_spec.js index a62ab54448..2a9945a85f 100644 --- a/tests/e2e/env_spec.js +++ b/tests/e2e/env_spec.js @@ -10,12 +10,12 @@ describe("App environment", () => { }); 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); }); 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); }); diff --git a/tests/e2e/fonts_spec.js b/tests/e2e/fonts_spec.js index 160359ecae..706ed4a909 100644 --- a/tests/e2e/fonts_spec.js +++ b/tests/e2e/fonts_spec.js @@ -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) => { const fontUrl = `http://localhost:8080/fonts/${fontFile}`; - const res = await helpers.fetch(fontUrl); + const res = await fetch(fontUrl); expect(res.status).toBe(200); }); }); diff --git a/tests/e2e/helpers/global-setup.js b/tests/e2e/helpers/global-setup.js index b79c1f1f13..d5506024af 100644 --- a/tests/e2e/helpers/global-setup.js +++ b/tests/e2e/helpers/global-setup.js @@ -1,5 +1,4 @@ const jsdom = require("jsdom"); -const corefetch = require("fetch"); exports.startApplication = async (configFilename, exec) => { jest.resetModules(); @@ -31,7 +30,7 @@ exports.getDocument = () => { const url = `http://${config.address || "localhost"}:${config.port || "8080"}`; jsdom.JSDOM.fromURL(url, { resources: "usable", runScripts: "dangerously" }).then((dom) => { dom.window.name = "jsdom"; - dom.window.fetch = corefetch; + dom.window.fetch = fetch; dom.window.onload = () => { global.document = dom.window.document; 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) => { const elem = await this.waitForElement(element); expect(elem).not.toBe(null); diff --git a/tests/e2e/ipWhitelist_spec.js b/tests/e2e/ipWhitelist_spec.js index 2bb2d682a8..07a0425e8d 100644 --- a/tests/e2e/ipWhitelist_spec.js +++ b/tests/e2e/ipWhitelist_spec.js @@ -10,7 +10,7 @@ describe("ipWhitelist directive configuration", () => { }); 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); }); }); @@ -24,7 +24,7 @@ describe("ipWhitelist directive configuration", () => { }); 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); }); }); diff --git a/tests/e2e/port_spec.js b/tests/e2e/port_spec.js index 104b9373dc..f6900a3dd5 100644 --- a/tests/e2e/port_spec.js +++ b/tests/e2e/port_spec.js @@ -10,7 +10,7 @@ describe("port directive configuration", () => { }); 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); }); }); @@ -24,7 +24,7 @@ describe("port directive configuration", () => { }); 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); }); }); diff --git a/tests/e2e/serveronly_spec.js b/tests/e2e/serveronly_spec.js index 78ecba30f5..82d0429b83 100644 --- a/tests/e2e/serveronly_spec.js +++ b/tests/e2e/serveronly_spec.js @@ -17,12 +17,12 @@ describe("App environment", () => { }); 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); }); 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); }); }); diff --git a/tests/e2e/template_spec.js b/tests/e2e/template_spec.js index 0c706c1cc5..3bcc5e4427 100644 --- a/tests/e2e/template_spec.js +++ b/tests/e2e/template_spec.js @@ -9,7 +9,7 @@ describe("templated config with port variable", () => { }); 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); }); }); diff --git a/tests/e2e/vendor_spec.js b/tests/e2e/vendor_spec.js index dff9585810..49d3ab8517 100644 --- a/tests/e2e/vendor_spec.js +++ b/tests/e2e/vendor_spec.js @@ -14,7 +14,7 @@ describe("Vendors", () => { Object.keys(vendors).forEach((vendor) => { it(`should return 200 HTTP code for vendor "${vendor}"`, async () => { const urlVendor = `http://localhost:8080/vendor/${vendors[vendor]}`; - const res = await helpers.fetch(urlVendor); + const res = await fetch(urlVendor); expect(res.status).toBe(200); }); }); @@ -22,7 +22,7 @@ describe("Vendors", () => { Object.keys(vendors).forEach((vendor) => { it(`should return 404 HTTP code for vendor https://localhost/"${vendor}"`, async () => { const urlVendor = `http://localhost:8080/${vendors[vendor]}`; - const res = await helpers.fetch(urlVendor); + const res = await fetch(urlVendor); expect(res.status).toBe(404); }); }); diff --git a/tests/unit/functions/server_functions_spec.js b/tests/unit/functions/server_functions_spec.js index 3548e38a0e..6242c9a9b5 100644 --- a/tests/unit/functions/server_functions_spec.js +++ b/tests/unit/functions/server_functions_spec.js @@ -8,13 +8,9 @@ describe("server_functions tests", () => { let corsResponse; let request; - jest.mock("node-fetch"); - let nodefetch = require("node-fetch"); let fetchMock; beforeEach(() => { - nodefetch.mockReset(); - fetchResponseHeadersGet = jest.fn(() => {}); fetchResponseHeadersText = jest.fn(() => {}); fetchResponse = { @@ -23,10 +19,11 @@ describe("server_functions tests", () => { }, text: fetchResponseHeadersText }; - jest.mock("node-fetch", () => jest.fn()); - nodefetch.mockImplementation(() => fetchResponse); + // eslint-disable-next-line + fetch = jest.fn(); + fetch.mockImplementation(() => fetchResponse); - fetchMock = nodefetch; + fetchMock = fetch; corsResponse = { set: jest.fn(() => {}),