Skip to content

Commit c1aaea5

Browse files
[weather] add error handling to weather fetch functions, including cors (#3791)
Co-authored-by: Kristjan ESPERANTO <[email protected]> fixes #3687
1 parent 3b79791 commit c1aaea5

File tree

5 files changed

+71
-56
lines changed

5 files changed

+71
-56
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ planned for 2026-01-01
1515

1616
- [weather] feat: add configurable forecast date format option (#3918)
1717
- [core] Add new `server:watch` script to run MagicMirror² server-only with automatic restarts when files (defined in `config.watchTargets`) change (#3920)
18+
- [weather] add error handling to fetch functions including cors (#3791)
1819

1920
### Removed
2021

js/server_functions.js

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ function getStartup (req, res) {
3030
* Only the url-param of the input request url is required. It must be the last parameter.
3131
* @param {Request} req - the request
3232
* @param {Response} res - the result
33+
* @returns {Promise<void>} A promise that resolves when the response is sent
3334
*/
3435
async function cors (req, res) {
3536
try {
@@ -40,29 +41,32 @@ async function cors (req, res) {
4041
if (!match) {
4142
url = `invalid url: ${req.url}`;
4243
Log.error(url);
43-
res.send(url);
44+
return res.status(400).send(url);
4445
} else {
4546
url = match[1];
4647

4748
const headersToSend = getHeadersToSend(req.url);
4849
const expectedReceivedHeaders = geExpectedReceivedHeaders(req.url);
49-
5050
Log.log(`cors url: ${url}`);
51-
const response = await fetch(url, { headers: headersToSend });
5251

53-
for (const header of expectedReceivedHeaders) {
54-
const headerValue = response.headers.get(header);
55-
if (header) res.set(header, headerValue);
52+
const response = await fetch(url, { headers: headersToSend });
53+
if (response.ok) {
54+
for (const header of expectedReceivedHeaders) {
55+
const headerValue = response.headers.get(header);
56+
if (header) res.set(header, headerValue);
57+
}
58+
const data = await response.text();
59+
res.send(data);
60+
} else {
61+
throw new Error(`Response status: ${response.status}`);
5662
}
57-
const data = await response.text();
58-
res.send(data);
5963
}
6064
} catch (error) {
6165
// Only log errors in non-test environments to keep test output clean
6266
if (process.env.mmTestMode !== "true") {
63-
Log.error(error);
67+
Log.error(`Error in CORS request: ${error}`);
6468
}
65-
res.send(error);
69+
res.status(500).json({ error: error.message });
6670
}
6771
}
6872

modules/default/utils.js

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,29 @@ async function performWebRequest (url, type = "json", useCorsProxy = false, requ
1717
requestUrl = url;
1818
request.headers = getHeadersToSend(requestHeaders);
1919
}
20-
const response = await fetch(requestUrl, request);
21-
const data = await response.text();
2220

23-
if (type === "xml") {
24-
return new DOMParser().parseFromString(data, "text/html");
25-
} else {
26-
if (!data || !data.length > 0) return undefined;
21+
try {
22+
const response = await fetch(requestUrl, request);
23+
if (response.ok) {
24+
const data = await response.text();
25+
26+
if (type === "xml") {
27+
return new DOMParser().parseFromString(data, "text/html");
28+
} else {
29+
if (!data || !data.length > 0) return undefined;
2730

28-
const dataResponse = JSON.parse(data);
29-
if (!dataResponse.headers) {
30-
dataResponse.headers = getHeadersFromResponse(expectedResponseHeaders, response);
31+
const dataResponse = JSON.parse(data);
32+
if (!dataResponse.headers) {
33+
dataResponse.headers = getHeadersFromResponse(expectedResponseHeaders, response);
34+
}
35+
return dataResponse;
36+
}
37+
} else {
38+
throw new Error(`Response status: ${response.status}`);
3139
}
32-
return dataResponse;
40+
} catch (error) {
41+
Log.error(`Error fetching data from ${url}: ${error}`);
42+
return undefined;
3343
}
3444
}
3545

modules/default/weather/providers/pirateweather.js

Lines changed: 26 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,38 +22,36 @@ WeatherProvider.register("pirateweather", {
2222
lon: 0
2323
},
2424

25-
fetchCurrentWeather () {
26-
this.fetchData(this.getUrl())
27-
.then((data) => {
28-
if (!data || !data.currently || typeof data.currently.temperature === "undefined") {
29-
// No usable data?
30-
return;
31-
}
25+
async fetchCurrentWeather () {
26+
try {
27+
const data = await this.fetchData(this.getUrl());
28+
if (!data || !data.currently || typeof data.currently.temperature === "undefined") {
29+
throw new Error("No usable data received from Pirate Weather API.");
30+
}
3231

33-
const currentWeather = this.generateWeatherDayFromCurrentWeather(data);
34-
this.setCurrentWeather(currentWeather);
35-
})
36-
.catch(function (request) {
37-
Log.error("[weatherprovider.pirateweather] Could not load data ... ", request);
38-
})
39-
.finally(() => this.updateAvailable());
32+
const currentWeather = this.generateWeatherDayFromCurrentWeather(data);
33+
this.setCurrentWeather(currentWeather);
34+
} catch (error) {
35+
Log.error("Could not load data ... ", error);
36+
} finally {
37+
this.updateAvailable();
38+
}
4039
},
4140

42-
fetchWeatherForecast () {
43-
this.fetchData(this.getUrl())
44-
.then((data) => {
45-
if (!data || !data.daily || !data.daily.data.length) {
46-
// No usable data?
47-
return;
48-
}
41+
async fetchWeatherForecast () {
42+
try {
43+
const data = await this.fetchData(this.getUrl());
44+
if (!data || !data.daily || !data.daily.data.length) {
45+
throw new Error("No usable data received from Pirate Weather API.");
46+
}
4947

50-
const forecast = this.generateWeatherObjectsFromForecast(data.daily.data);
51-
this.setWeatherForecast(forecast);
52-
})
53-
.catch(function (request) {
54-
Log.error("[weatherprovider.pirateweather] Could not load data ... ", request);
55-
})
56-
.finally(() => this.updateAvailable());
48+
const forecast = this.generateWeatherObjectsFromForecast(data.daily.data);
49+
this.setWeatherForecast(forecast);
50+
} catch (error) {
51+
Log.error("Could not load data ... ", error);
52+
} finally {
53+
this.updateAvailable();
54+
}
5755
},
5856

5957
// Create a URL from the config and base URL.

tests/unit/functions/server_functions_spec.js

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ describe("server_functions tests", () => {
1616
headers: {
1717
get: fetchResponseHeadersGet
1818
},
19-
text: fetchResponseHeadersText
19+
text: fetchResponseHeadersText,
20+
ok: true
2021
};
2122

2223
fetch = vi.fn();
@@ -26,7 +27,12 @@ describe("server_functions tests", () => {
2627

2728
corsResponse = {
2829
set: vi.fn(() => {}),
29-
send: vi.fn(() => {})
30+
send: vi.fn(() => {}),
31+
status: vi.fn(function (code) {
32+
this.statusCode = code;
33+
return this;
34+
}),
35+
json: vi.fn(() => {})
3036
};
3137

3238
request = {
@@ -91,15 +97,11 @@ describe("server_functions tests", () => {
9197
throw error;
9298
});
9399

94-
let sentData;
95-
corsResponse.send = vi.fn((input) => {
96-
sentData = input;
97-
});
98-
99100
await cors(request, corsResponse);
100101

101102
expect(fetchResponseHeadersText.mock.calls).toHaveLength(1);
102-
expect(sentData).toBe(error);
103+
expect(corsResponse.status).toHaveBeenCalledWith(500);
104+
expect(corsResponse.json).toHaveBeenCalledWith({ error: error.message });
103105
});
104106

105107
it("Fetches with user agent by default", async () => {

0 commit comments

Comments
 (0)