fix: Rerun onboarding if Z2M start failed after previous onboarding (#27386)

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<void> {

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;

lib/util/onboarding.ts

@@ -396,8 +396,6 @@ async function startOnboardingServer(): Promise<boolean> {
 
                     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<Settings> = {
                             mqtt: {
@@ -540,7 +538,9 @@ export async function onboard(): Promise<boolean> {
 
     // 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) {

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);

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);

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('<a href="http://localhost:8080/">');
     });
 
+    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('<a href="http://localhost:8080/">');
+    });
+
     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 () => {