From 0343805d971dbc35263b5e5c79f733e188865584 Mon Sep 17 00:00:00 2001 From: sam detweiler Date: Wed, 5 Nov 2025 21:23:19 +0100 Subject: [PATCH 1/2] add error handling to weather fetch functions, including cors --- CHANGELOG.md | 1 + js/server_functions.js | 22 +++++++++++++--------- modules/default/utils.js | 29 +++++++++++++++++++---------- 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8db9047010..bf0cf264ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ planned for 2026-01-01 - [weather] feat: add configurable forecast date format option (#3918) - [core] Add new `server:watch` script to run MagicMirror² server-only with automatic restarts when files (defined in `config.watchTargets`) change (#3920) +- [weather] add error handling to fetch functions including cors (#3791) ### Changed diff --git a/js/server_functions.js b/js/server_functions.js index 4223204d2c..0415f8c9b7 100644 --- a/js/server_functions.js +++ b/js/server_functions.js @@ -41,28 +41,32 @@ async function cors (req, res) { url = `invalid url: ${req.url}`; Log.error(url); res.send(url); - } else { - url = match[1]; - - const headersToSend = getHeadersToSend(req.url); - const expectedReceivedHeaders = geExpectedReceivedHeaders(req.url); + return; + } + url = match[1]; - Log.log(`cors url: ${url}`); - const response = await fetch(url, { headers: headersToSend }); + const headersToSend = getHeadersToSend(req.url); + const expectedReceivedHeaders = geExpectedReceivedHeaders(req.url); + Log.log(`cors url: ${url}`); + const response = await fetch(url, { headers: headersToSend }); + if (response.ok) { for (const header of expectedReceivedHeaders) { const headerValue = response.headers.get(header); if (header) res.set(header, headerValue); } const data = await response.text(); res.send(data); + } else { + res.status(response.status).json({ message: response.statusText }); } + } catch (error) { // Only log errors in non-test environments to keep test output clean if (process.env.mmTestMode !== "true") { - Log.error(error); + Log.error(`Error in CORS request: ${error}`); } - res.send(error); + res.status(500).json({ error: error.message }); } } diff --git a/modules/default/utils.js b/modules/default/utils.js index 2d6e43ab10..cc9b7a1813 100644 --- a/modules/default/utils.js +++ b/modules/default/utils.js @@ -17,19 +17,28 @@ async function performWebRequest (url, type = "json", useCorsProxy = false, requ requestUrl = url; request.headers = getHeadersToSend(requestHeaders); } - const response = await fetch(requestUrl, request); - const data = await response.text(); - if (type === "xml") { - return new DOMParser().parseFromString(data, "text/html"); - } else { - if (!data || !data.length > 0) return undefined; + try { + const response = await fetch(requestUrl, request); + if (response.ok) { + const data = await response.text(); + + if (type === "xml") { + return new DOMParser().parseFromString(data, "text/html"); + } else { + if (!data || !data.length > 0) return undefined; - const dataResponse = JSON.parse(data); - if (!dataResponse.headers) { - dataResponse.headers = getHeadersFromResponse(expectedResponseHeaders, response); + const dataResponse = JSON.parse(data); + if (!dataResponse.headers) { + dataResponse.headers = getHeadersFromResponse(expectedResponseHeaders, response); + } + return dataResponse; + } + } else { + throw new Error(`Response status: ${response.status}`); } - return dataResponse; + } catch (error) { + Log.error(`Error fetching data from ${url}: ${error}`); } } From e9fb1cd2baeaf285466e94a050ce5d92d18288f3 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Wed, 5 Nov 2025 22:39:33 +0100 Subject: [PATCH 2/2] Refactor and get tests work again --- js/server_functions.js | 38 ++++++------- modules/default/utils.js | 1 + .../weather/providers/pirateweather.js | 54 +++++++++---------- tests/unit/functions/server_functions_spec.js | 18 ++++--- 4 files changed, 56 insertions(+), 55 deletions(-) diff --git a/js/server_functions.js b/js/server_functions.js index 0415f8c9b7..a7eba8c358 100644 --- a/js/server_functions.js +++ b/js/server_functions.js @@ -30,6 +30,7 @@ function getStartup (req, res) { * Only the url-param of the input request url is required. It must be the last parameter. * @param {Request} req - the request * @param {Response} res - the result + * @returns {Promise} A promise that resolves when the response is sent */ async function cors (req, res) { try { @@ -40,27 +41,26 @@ async function cors (req, res) { if (!match) { url = `invalid url: ${req.url}`; Log.error(url); - res.send(url); - return; - } - url = match[1]; - - const headersToSend = getHeadersToSend(req.url); - const expectedReceivedHeaders = geExpectedReceivedHeaders(req.url); - Log.log(`cors url: ${url}`); - - const response = await fetch(url, { headers: headersToSend }); - if (response.ok) { - for (const header of expectedReceivedHeaders) { - const headerValue = response.headers.get(header); - if (header) res.set(header, headerValue); - } - const data = await response.text(); - res.send(data); + return res.status(400).send(url); } else { - res.status(response.status).json({ message: response.statusText }); + url = match[1]; + + const headersToSend = getHeadersToSend(req.url); + const expectedReceivedHeaders = geExpectedReceivedHeaders(req.url); + Log.log(`cors url: ${url}`); + + const response = await fetch(url, { headers: headersToSend }); + if (response.ok) { + for (const header of expectedReceivedHeaders) { + const headerValue = response.headers.get(header); + if (header) res.set(header, headerValue); + } + const data = await response.text(); + res.send(data); + } else { + throw new Error(`Response status: ${response.status}`); + } } - } catch (error) { // Only log errors in non-test environments to keep test output clean if (process.env.mmTestMode !== "true") { diff --git a/modules/default/utils.js b/modules/default/utils.js index cc9b7a1813..c4ca66714f 100644 --- a/modules/default/utils.js +++ b/modules/default/utils.js @@ -39,6 +39,7 @@ async function performWebRequest (url, type = "json", useCorsProxy = false, requ } } catch (error) { Log.error(`Error fetching data from ${url}: ${error}`); + return undefined; } } diff --git a/modules/default/weather/providers/pirateweather.js b/modules/default/weather/providers/pirateweather.js index 0117fc0a32..48a909c692 100644 --- a/modules/default/weather/providers/pirateweather.js +++ b/modules/default/weather/providers/pirateweather.js @@ -22,38 +22,36 @@ WeatherProvider.register("pirateweather", { lon: 0 }, - fetchCurrentWeather () { - this.fetchData(this.getUrl()) - .then((data) => { - if (!data || !data.currently || typeof data.currently.temperature === "undefined") { - // No usable data? - return; - } + async fetchCurrentWeather () { + try { + const data = await this.fetchData(this.getUrl()); + if (!data || !data.currently || typeof data.currently.temperature === "undefined") { + throw new Error("No usable data received from Pirate Weather API."); + } - const currentWeather = this.generateWeatherDayFromCurrentWeather(data); - this.setCurrentWeather(currentWeather); - }) - .catch(function (request) { - Log.error("[weatherprovider.pirateweather] Could not load data ... ", request); - }) - .finally(() => this.updateAvailable()); + const currentWeather = this.generateWeatherDayFromCurrentWeather(data); + this.setCurrentWeather(currentWeather); + } catch (error) { + Log.error("Could not load data ... ", error); + } finally { + this.updateAvailable(); + } }, - fetchWeatherForecast () { - this.fetchData(this.getUrl()) - .then((data) => { - if (!data || !data.daily || !data.daily.data.length) { - // No usable data? - return; - } + async fetchWeatherForecast () { + try { + const data = await this.fetchData(this.getUrl()); + if (!data || !data.daily || !data.daily.data.length) { + throw new Error("No usable data received from Pirate Weather API."); + } - const forecast = this.generateWeatherObjectsFromForecast(data.daily.data); - this.setWeatherForecast(forecast); - }) - .catch(function (request) { - Log.error("[weatherprovider.pirateweather] Could not load data ... ", request); - }) - .finally(() => this.updateAvailable()); + const forecast = this.generateWeatherObjectsFromForecast(data.daily.data); + this.setWeatherForecast(forecast); + } catch (error) { + Log.error("Could not load data ... ", error); + } finally { + this.updateAvailable(); + } }, // Create a URL from the config and base URL. diff --git a/tests/unit/functions/server_functions_spec.js b/tests/unit/functions/server_functions_spec.js index 469e2292f8..5383500ae6 100644 --- a/tests/unit/functions/server_functions_spec.js +++ b/tests/unit/functions/server_functions_spec.js @@ -16,7 +16,8 @@ describe("server_functions tests", () => { headers: { get: fetchResponseHeadersGet }, - text: fetchResponseHeadersText + text: fetchResponseHeadersText, + ok: true }; fetch = vi.fn(); @@ -26,7 +27,12 @@ describe("server_functions tests", () => { corsResponse = { set: vi.fn(() => {}), - send: vi.fn(() => {}) + send: vi.fn(() => {}), + status: vi.fn(function (code) { + this.statusCode = code; + return this; + }), + json: vi.fn(() => {}) }; request = { @@ -91,15 +97,11 @@ describe("server_functions tests", () => { throw error; }); - let sentData; - corsResponse.send = vi.fn((input) => { - sentData = input; - }); - await cors(request, corsResponse); expect(fetchResponseHeadersText.mock.calls).toHaveLength(1); - expect(sentData).toBe(error); + expect(corsResponse.status).toHaveBeenCalledWith(500); + expect(corsResponse.json).toHaveBeenCalledWith({ error: error.message }); }); it("Fetches with user agent by default", async () => {