From 4c2a373ae32e933d8f6df249065619d4cedf89f1 Mon Sep 17 00:00:00 2001 From: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com> Date: Mon, 27 Apr 2026 23:15:01 +0200 Subject: [PATCH] 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. --- defaultmodules/weather/node_helper.js | 14 ++- .../default/weather/node_helper_spec.js | 116 ++++++++++++++++++ 2 files changed, 125 insertions(+), 5 deletions(-) create mode 100644 tests/unit/modules/default/weather/node_helper_spec.js diff --git a/defaultmodules/weather/node_helper.js b/defaultmodules/weather/node_helper.js index cda81a51..153ab1c1 100644 --- a/defaultmodules/weather/node_helper.js +++ b/defaultmodules/weather/node_helper.js @@ -4,6 +4,7 @@ const Log = require("logger"); module.exports = NodeHelper.create({ providers: {}, + lastData: {}, start () { Log.log(`Starting node helper for: ${this.name}`); @@ -37,6 +38,10 @@ module.exports = NodeHelper.create({ instanceId, locationName: this.providers[instanceId].locationName }); + // Push cached data immediately so reconnecting clients don't wait for next scheduled fetch + if (this.lastData[instanceId]) { + this.sendSocketNotification("WEATHER_DATA", this.lastData[instanceId]); + } return; } @@ -53,11 +58,9 @@ module.exports = NodeHelper.create({ provider.setCallbacks( (data) => { // On data received - this.sendSocketNotification("WEATHER_DATA", { - instanceId, - type: config.type, - data - }); + const payload = { instanceId, type: config.type, data }; + this.lastData[instanceId] = payload; + this.sendSocketNotification("WEATHER_DATA", payload); }, (errorInfo) => { // On error @@ -101,6 +104,7 @@ module.exports = NodeHelper.create({ Log.log(`Stopping weather provider for instance ${instanceId}`); provider.stop(); delete this.providers[instanceId]; + delete this.lastData[instanceId]; } else { Log.warn(`No provider found for instance ${instanceId}`); } diff --git a/tests/unit/modules/default/weather/node_helper_spec.js b/tests/unit/modules/default/weather/node_helper_spec.js new file mode 100644 index 00000000..2f18e965 --- /dev/null +++ b/tests/unit/modules/default/weather/node_helper_spec.js @@ -0,0 +1,116 @@ +import Module from "node:module"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +/** + * Creates a fresh weather node helper instance with isolated mocks. + * @returns {Promise} The mocked weather node helper. + */ +async function loadWeatherNodeHelper () { + vi.resetModules(); + + const loggerMock = { + log: vi.fn(), + warn: vi.fn(), + error: vi.fn() + }; + const originalRequire = Module.prototype.require; + + Module.prototype.require = function (id) { + if (id === "node_helper") { + return { + create: vi.fn((definition) => definition) + }; + } + + if (id === "logger") { + return loggerMock; + } + + return originalRequire.apply(this, arguments); + }; + + let helper; + try { + const helperModule = await import("../../../../../defaultmodules/weather/node_helper"); + helper = helperModule.default || helperModule; + } finally { + Module.prototype.require = originalRequire; + } + + helper.providers = {}; + helper.lastData = {}; + helper.sendSocketNotification = vi.fn(); + + return helper; +} + +afterEach(() => { + vi.resetAllMocks(); + vi.resetModules(); +}); + +describe("weather node_helper reconnect handling", () => { + it("re-sends cached weather data when a client reconnects", async () => { + const helper = await loadWeatherNodeHelper(); + const instanceId = "weather-current"; + const cachedPayload = { + instanceId, + type: "current", + data: { temperature: 8.5 } + }; + + helper.providers[instanceId] = { locationName: "Munich, BY" }; + helper.lastData[instanceId] = cachedPayload; + + await helper.initWeatherProvider({ + weatherProvider: "openmeteo", + instanceId, + type: "current" + }); + + expect(helper.sendSocketNotification).toHaveBeenNthCalledWith(1, "WEATHER_INITIALIZED", { + instanceId, + locationName: "Munich, BY" + }); + expect(helper.sendSocketNotification).toHaveBeenNthCalledWith(2, "WEATHER_DATA", cachedPayload); + expect(helper.sendSocketNotification).toHaveBeenCalledTimes(2); + }); + + it("does not send WEATHER_DATA on reconnect when no cached payload exists", async () => { + const helper = await loadWeatherNodeHelper(); + const instanceId = "weather-current"; + + helper.providers[instanceId] = { locationName: "Munich, BY" }; + + await helper.initWeatherProvider({ + weatherProvider: "openmeteo", + instanceId, + type: "current" + }); + + expect(helper.sendSocketNotification).toHaveBeenCalledWith("WEATHER_INITIALIZED", { + instanceId, + locationName: "Munich, BY" + }); + expect(helper.sendSocketNotification).toHaveBeenCalledTimes(1); + }); + + it("cleans up provider and cached data when stopping an instance", async () => { + const helper = await loadWeatherNodeHelper(); + const instanceId = "weather-current"; + const stop = vi.fn(); + + helper.providers[instanceId] = { stop }; + helper.lastData[instanceId] = { + instanceId, + type: "current", + data: { temperature: 8.5 } + }; + + helper.stopWeatherProvider(instanceId); + + expect(stop).toHaveBeenCalledTimes(1); + expect(helper.providers[instanceId]).toBeUndefined(); + expect(helper.lastData[instanceId]).toBeUndefined(); + }); +});