Skip to content

Commit 4c2a373

Browse files
fix(weather): avoid loading state after reconnect (#4121)
When a client reconnects while the backend is still in its rate-limit protection phase, the weather module has no data to show and stays on `Loading...` until the next scheduled API call. This mainly affects server mode setups, where the server keeps running while a remote client temporarily loses its connection and reloads. It was [raised in the forum](https://forum.magicmirror.builders/topic/20218/request-loop-loading...-in-standard-weather-module-open-meteo-after-update/11?_=1777106416020) and is worthy of a fix to improve the user experience. With this PR the node helper caches the last successful `WEATHER_DATA` payload per instance and replays it immediately on reconnect. The client gets its last known state right away instead of waiting for the next fetch. The cache is cleaned up when the provider stops. Tests are included to cover reconnect with and without cached data, and the cleanup path.
1 parent b8548f2 commit 4c2a373

2 files changed

Lines changed: 125 additions & 5 deletions

File tree

defaultmodules/weather/node_helper.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const Log = require("logger");
44

55
module.exports = NodeHelper.create({
66
providers: {},
7+
lastData: {},
78

89
start () {
910
Log.log(`Starting node helper for: ${this.name}`);
@@ -37,6 +38,10 @@ module.exports = NodeHelper.create({
3738
instanceId,
3839
locationName: this.providers[instanceId].locationName
3940
});
41+
// Push cached data immediately so reconnecting clients don't wait for next scheduled fetch
42+
if (this.lastData[instanceId]) {
43+
this.sendSocketNotification("WEATHER_DATA", this.lastData[instanceId]);
44+
}
4045
return;
4146
}
4247

@@ -53,11 +58,9 @@ module.exports = NodeHelper.create({
5358
provider.setCallbacks(
5459
(data) => {
5560
// On data received
56-
this.sendSocketNotification("WEATHER_DATA", {
57-
instanceId,
58-
type: config.type,
59-
data
60-
});
61+
const payload = { instanceId, type: config.type, data };
62+
this.lastData[instanceId] = payload;
63+
this.sendSocketNotification("WEATHER_DATA", payload);
6164
},
6265
(errorInfo) => {
6366
// On error
@@ -101,6 +104,7 @@ module.exports = NodeHelper.create({
101104
Log.log(`Stopping weather provider for instance ${instanceId}`);
102105
provider.stop();
103106
delete this.providers[instanceId];
107+
delete this.lastData[instanceId];
104108
} else {
105109
Log.warn(`No provider found for instance ${instanceId}`);
106110
}
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import Module from "node:module";
2+
import { afterEach, describe, expect, it, vi } from "vitest";
3+
4+
/**
5+
* Creates a fresh weather node helper instance with isolated mocks.
6+
* @returns {Promise<object>} The mocked weather node helper.
7+
*/
8+
async function loadWeatherNodeHelper () {
9+
vi.resetModules();
10+
11+
const loggerMock = {
12+
log: vi.fn(),
13+
warn: vi.fn(),
14+
error: vi.fn()
15+
};
16+
const originalRequire = Module.prototype.require;
17+
18+
Module.prototype.require = function (id) {
19+
if (id === "node_helper") {
20+
return {
21+
create: vi.fn((definition) => definition)
22+
};
23+
}
24+
25+
if (id === "logger") {
26+
return loggerMock;
27+
}
28+
29+
return originalRequire.apply(this, arguments);
30+
};
31+
32+
let helper;
33+
try {
34+
const helperModule = await import("../../../../../defaultmodules/weather/node_helper");
35+
helper = helperModule.default || helperModule;
36+
} finally {
37+
Module.prototype.require = originalRequire;
38+
}
39+
40+
helper.providers = {};
41+
helper.lastData = {};
42+
helper.sendSocketNotification = vi.fn();
43+
44+
return helper;
45+
}
46+
47+
afterEach(() => {
48+
vi.resetAllMocks();
49+
vi.resetModules();
50+
});
51+
52+
describe("weather node_helper reconnect handling", () => {
53+
it("re-sends cached weather data when a client reconnects", async () => {
54+
const helper = await loadWeatherNodeHelper();
55+
const instanceId = "weather-current";
56+
const cachedPayload = {
57+
instanceId,
58+
type: "current",
59+
data: { temperature: 8.5 }
60+
};
61+
62+
helper.providers[instanceId] = { locationName: "Munich, BY" };
63+
helper.lastData[instanceId] = cachedPayload;
64+
65+
await helper.initWeatherProvider({
66+
weatherProvider: "openmeteo",
67+
instanceId,
68+
type: "current"
69+
});
70+
71+
expect(helper.sendSocketNotification).toHaveBeenNthCalledWith(1, "WEATHER_INITIALIZED", {
72+
instanceId,
73+
locationName: "Munich, BY"
74+
});
75+
expect(helper.sendSocketNotification).toHaveBeenNthCalledWith(2, "WEATHER_DATA", cachedPayload);
76+
expect(helper.sendSocketNotification).toHaveBeenCalledTimes(2);
77+
});
78+
79+
it("does not send WEATHER_DATA on reconnect when no cached payload exists", async () => {
80+
const helper = await loadWeatherNodeHelper();
81+
const instanceId = "weather-current";
82+
83+
helper.providers[instanceId] = { locationName: "Munich, BY" };
84+
85+
await helper.initWeatherProvider({
86+
weatherProvider: "openmeteo",
87+
instanceId,
88+
type: "current"
89+
});
90+
91+
expect(helper.sendSocketNotification).toHaveBeenCalledWith("WEATHER_INITIALIZED", {
92+
instanceId,
93+
locationName: "Munich, BY"
94+
});
95+
expect(helper.sendSocketNotification).toHaveBeenCalledTimes(1);
96+
});
97+
98+
it("cleans up provider and cached data when stopping an instance", async () => {
99+
const helper = await loadWeatherNodeHelper();
100+
const instanceId = "weather-current";
101+
const stop = vi.fn();
102+
103+
helper.providers[instanceId] = { stop };
104+
helper.lastData[instanceId] = {
105+
instanceId,
106+
type: "current",
107+
data: { temperature: 8.5 }
108+
};
109+
110+
helper.stopWeatherProvider(instanceId);
111+
112+
expect(stop).toHaveBeenCalledTimes(1);
113+
expect(helper.providers[instanceId]).toBeUndefined();
114+
expect(helper.lastData[instanceId]).toBeUndefined();
115+
});
116+
});

0 commit comments

Comments
 (0)