From d697c29c7be5e7e49d82d25f355a26875ac383a3 Mon Sep 17 00:00:00 2001 From: Nerivec <62446222+Nerivec@users.noreply.github.com> Date: Mon, 12 May 2025 21:33:55 +0200 Subject: [PATCH] fix: Rerun onboarding if Z2M start failed after previous onboarding (#27386) --- lib/controller.ts | 2 ++ lib/types/api.ts | 2 ++ lib/util/onboarding.ts | 6 +++--- lib/util/settings.ts | 16 +++++++++++++++ test/controller.test.ts | 25 ++++++++++++++++++++++++ test/onboarding.test.ts | 43 ++++++++++++++++++++++++++++++++++------- 6 files changed, 84 insertions(+), 10 deletions(-) diff --git a/lib/controller.ts b/lib/controller.ts index b71208a4e..e4293d127 100644 --- a/lib/controller.ts +++ b/lib/controller.ts @@ -163,6 +163,8 @@ export class Controller { logger.info("Zigbee2MQTT started!"); this.sdNotify = await initSdNotify(); + + settings.setOnboarding(false); } @bind async enableDisableExtension(enable: boolean, name: string): Promise { diff --git a/lib/types/api.ts b/lib/types/api.ts index a40e0db67..06cd6fb58 100644 --- a/lib/types/api.ts +++ b/lib/types/api.ts @@ -69,6 +69,8 @@ export interface Zigbee2MQTTGroupOptions { export interface Zigbee2MQTTSettings { version?: number; + /** only used internally during startup, removed on successful Z2M start */ + onboarding?: true; homeassistant: { enabled: boolean; discovery_topic: string; diff --git a/lib/util/onboarding.ts b/lib/util/onboarding.ts index b4651b6a3..d250d5538 100644 --- a/lib/util/onboarding.ts +++ b/lib/util/onboarding.ts @@ -396,8 +396,6 @@ async function startOnboardingServer(): Promise { req.on("end", () => { const result = parse(body) as unknown as OnboardSettings; - console.log(JSON.stringify(currentSettings)); - console.log(JSON.stringify(result)); const frontendEnabled = result.frontend_enabled === "on"; const updatedSettings: RecursivePartial = { mqtt: { @@ -540,7 +538,9 @@ export async function onboard(): Promise { // use `configuration.yaml` file to detect "brand new install" // env allows to re-run onboard even with existing install - if (!process.env.Z2M_ONBOARD_NO_SERVER && (process.env.Z2M_ONBOARD_FORCE_RUN || !confExists)) { + if (!process.env.Z2M_ONBOARD_NO_SERVER && (process.env.Z2M_ONBOARD_FORCE_RUN || !confExists || settings.get().onboarding)) { + settings.setOnboarding(true); + const success = await startOnboardingServer(); if (!success) { diff --git a/lib/util/settings.ts b/lib/util/settings.ts index f122fed68..3b23749f6 100644 --- a/lib/util/settings.ts +++ b/lib/util/settings.ts @@ -179,6 +179,22 @@ export function writeMinimalDefaults(): void { loadSettingsWithDefaults(); } +export function setOnboarding(value: boolean): void { + const settings = getPersistedSettings(); + + if (value) { + if (!settings.onboarding) { + settings.onboarding = value; + + write(); + } + } else if (settings.onboarding) { + delete settings.onboarding; + + write(); + } +} + export function write(): void { const settings = getPersistedSettings(); const toWrite: KeyValue = objectAssignDeep({}, settings); diff --git a/test/controller.test.ts b/test/controller.test.ts index 5ef2d22b2..e50b01201 100644 --- a/test/controller.test.ts +++ b/test/controller.test.ts @@ -87,6 +87,7 @@ describe("Controller", () => { }); it("Start controller", async () => { + settings.setOnboarding(true); settings.set(["advanced", "transmit_power"], 14); await controller.start(); expect(ZHController).toHaveBeenCalledWith({ @@ -121,6 +122,7 @@ describe("Controller", () => { {retain: true, qos: 0}, ); expect(mockMQTTPublishAsync).toHaveBeenCalledWith("zigbee2mqtt/remote", stringify({brightness: 255}), {retain: true, qos: 0}); + expect(settings.get().onboarding).toBeUndefined(); }); it("Start controller with specific MQTT settings", async () => { @@ -339,6 +341,16 @@ describe("Controller", () => { expect(mockExit).toHaveBeenCalledTimes(1); }); + it("Start controller fails after onboarding", async () => { + settings.setOnboarding(true); + mockZHController.start.mockImplementationOnce(() => { + throw new Error("failed"); + }); + await controller.start(); + expect(mockExit).toHaveBeenCalledTimes(1); + expect(settings.get().onboarding).toStrictEqual(true); + }); + it("Start controller fails due to MQTT connect error", async () => { mockMQTTConnectAsync.mockImplementationOnce(() => { throw new Error("addr not found"); @@ -350,6 +362,19 @@ describe("Controller", () => { expect(mockExit).toHaveBeenCalledWith(1, false); }); + it("Start controller fails due to MQTT connect error after onboarding", async () => { + settings.setOnboarding(true); + mockMQTTConnectAsync.mockImplementationOnce(() => { + throw new Error("addr not found"); + }); + await controller.start(); + await flushPromises(); + expect(mockLogger.error).toHaveBeenCalledWith("MQTT failed to connect, exiting... (addr not found)"); + expect(mockExit).toHaveBeenCalledTimes(1); + expect(mockExit).toHaveBeenCalledWith(1, false); + expect(settings.get().onboarding).toStrictEqual(true); + }); + it("Start controller and stop with restart", async () => { await controller.start(); await controller.stop(true); diff --git a/test/onboarding.test.ts b/test/onboarding.test.ts index 6b3376233..2b8649b2c 100644 --- a/test/onboarding.test.ts +++ b/test/onboarding.test.ts @@ -67,6 +67,7 @@ const SETTINGS_MINIMAL_DEFAULTS = { homeassistant: { enabled: settings.defaults.homeassistant!.enabled, }, + onboarding: true, }; const SAMPLE_SETTINGS_INIT = { @@ -123,6 +124,7 @@ const SAMPLE_SETTINGS_SAVE = { homeassistant: { enabled: true, }, + onboarding: true, }; const SAMPLE_SETTINGS_SAVE_PARAMS = { @@ -441,7 +443,7 @@ describe("Onboarding", () => { expect(postHtml).toContain("You can close this page"); }); - it("rerun onboard via ENV and sets given settings", async () => { + it("reruns onboard via ENV and sets given settings", async () => { // data.removeConfiguration(); process.env.Z2M_ONBOARD_FORCE_RUN = "1"; @@ -466,6 +468,30 @@ describe("Onboarding", () => { expect(postHtml).toContain(''); }); + it("reruns onboard on failed start", async () => { + // data.removeConfiguration(); + settings.setOnboarding(true); + + let p; + const [getHtml, postHtml] = await new Promise<[string, string]>((resolve, reject) => { + mockHttpOnListen.mockImplementationOnce(async () => { + try { + resolve(await runOnboarding(SAMPLE_SETTINGS_SAVE_PARAMS, false, false)); + } catch (error) { + reject(error); + } + }); + + p = onboard(); + }); + + await expect(p).resolves.toStrictEqual(true); + expect(data.read()).toStrictEqual(SAMPLE_SETTINGS_SAVE); + expect(getHtml).toContain("No device found"); + expect(getHtml).toContain("generate_network"); + expect(postHtml).toContain(''); + }); + it("sets given settings - no frontend redirect", async () => { data.removeConfiguration(); @@ -579,7 +605,7 @@ describe("Onboarding", () => { }); await expect(p).resolves.toStrictEqual(false); - expect(data.read()).toStrictEqual(SAMPLE_SETTINGS_INIT); + expect(data.read()).toStrictEqual(Object.assign({}, SAMPLE_SETTINGS_INIT, {onboarding: true})); expect(getHtml).toContain("No device found"); expect(postHtml).toContain("adapter must be equal to one of the allowed values"); }); @@ -622,11 +648,14 @@ describe("Onboarding", () => { const p = onboard(); await expect(p).resolves.toStrictEqual(true); - expect(data.read()).toStrictEqual( - Object.assign({}, SETTINGS_MINIMAL_DEFAULTS, { - mqtt: {server: process.env.ZIGBEE2MQTT_CONFIG_MQTT_SERVER, base_topic: SETTINGS_MINIMAL_DEFAULTS.mqtt.base_topic}, - }), - ); + + const expected = Object.assign({}, SETTINGS_MINIMAL_DEFAULTS, { + mqtt: {server: process.env.ZIGBEE2MQTT_CONFIG_MQTT_SERVER, base_topic: SETTINGS_MINIMAL_DEFAULTS.mqtt.base_topic}, + }); + // @ts-expect-error mock + delete expected.onboarding; + + expect(data.read()).toStrictEqual(expected); }); it("handles configuring onboarding with config ENV overrides", async () => {