From 45a4978ca8a1db663d31028cbbb20ce9d492e8fa Mon Sep 17 00:00:00 2001
From: Koen Kanters <koenkanters94@gmail.com>
Date: Sun, 3 Oct 2021 12:29:54 +0200
Subject: [PATCH] Improve MQTT error handling.
 https://github.com/Koenkk/zigbee2mqtt/issues/8956

---
 lib/mqtt.ts             | 26 +++++++++-----------------
 test/bridge.test.js     | 12 ------------
 test/controller.test.js | 10 ++++------
 3 files changed, 13 insertions(+), 35 deletions(-)

diff --git a/lib/mqtt.ts b/lib/mqtt.ts
index 313f77c9..73144b88 100644
--- a/lib/mqtt.ts
+++ b/lib/mqtt.ts
@@ -113,10 +113,6 @@ export default class MQTT {
         }
     }
 
-    isConnected(): boolean {
-        return this.client && !this.client.reconnecting;
-    }
-
     async publish(topic: string, payload: string, options: MQTTOptions={},
         base=settings.get().mqtt.base_topic, skipLog=false, skipReceive=true,
     ): Promise<void> {
@@ -129,25 +125,21 @@ export default class MQTT {
 
         this.eventBus.emitMQTTMessagePublished({topic, payload, options: {...defaultOptions, ...options}});
 
-        if (!this.isConnected()) {
-            if (!skipLog) {
-                logger.error(`Not connected to MQTT server!`);
-                logger.error(`Cannot send message: topic: '${topic}', payload: '${payload}`);
-            }
-            return;
-        }
-
-        if (!skipLog) {
-            logger.info(`MQTT publish: topic '${topic}', payload '${payload}'`);
-        }
-
         const actualOptions: mqtt.IClientPublishOptions = {...defaultOptions, ...options};
         if (settings.get().mqtt.force_disable_retain) {
             actualOptions.retain = false;
         }
 
         return new Promise((resolve) => {
-            this.client.publish(topic, payload, actualOptions, () => resolve());
+            this.client.publish(topic, payload, actualOptions, (err) => {
+                if (!err && !skipLog) {
+                    logger.info(`MQTT published: topic '${topic}', payload '${payload}'`);
+                } else if (err) {
+                    logger.error(`MQTT failed to publish: topic: '${topic}', payload: '${payload}`);
+                }
+
+                resolve();
+            });
         });
     }
 }
diff --git a/test/bridge.test.js b/test/bridge.test.js
index 67e7c46e..04f74538 100644
--- a/test/bridge.test.js
+++ b/test/bridge.test.js
@@ -94,18 +94,6 @@ describe('Bridge', () => {
         expect(logger.info).toHaveBeenCalledTimes(1);
     });
 
-    it('Shouldnt log to MQTT when not connected', async () => {
-        logger.setTransportsEnabled(true);
-        MQTT.mock.reconnecting = true;
-        MQTT.publish.mockClear();
-        logger.info.mockClear();
-        logger.error.mockClear();
-        logger.info("this is a test");
-        expect(MQTT.publish).toHaveBeenCalledTimes(0);
-        expect(logger.info).toHaveBeenCalledTimes(1);
-        expect(logger.error).toHaveBeenCalledTimes(0);
-    });
-
     it('Should publish groups on startup', async () => {
         await resetExtension();
         logger.setTransportsEnabled(true);
diff --git a/test/controller.test.js b/test/controller.test.js
index 0af5975e..18f66454 100644
--- a/test/controller.test.js
+++ b/test/controller.test.js
@@ -150,18 +150,16 @@ describe('Controller', () => {
         controller.mqtt.client.reconnecting = false;
     });
 
-    it('Dont publish to mqtt when client is unavailable', async () => {
+    it('Log when MQTT publish fails', async () => {
         await controller.start();
         await flushPromises();
         logger.error.mockClear();
-        controller.mqtt.client.reconnecting = true;
+        MQTT.mock.publish.mockImplementationOnce((topic, message, options, cb) => cb(true));
         const device = controller.zigbee.resolveEntity('bulb');
         await controller.publishEntityState(device, {state: 'ON', brightness: 50, color_temp: 370, color: {r: 100, g: 50, b: 10}, dummy: {1: 'yes', 2: 'no'}});
         await flushPromises();
-        expect(logger.error).toHaveBeenCalledTimes(2);
-        expect(logger.error).toHaveBeenCalledWith("Not connected to MQTT server!");
-        expect(logger.error).toHaveBeenCalledWith("Cannot send message: topic: 'zigbee2mqtt/bulb', payload: '{\"brightness\":50,\"color\":{\"b\":10,\"g\":50,\"r\":100},\"color_temp\":370,\"dummy\":{\"1\":\"yes\",\"2\":\"no\"},\"linkquality\":99,\"state\":\"ON\"}");
-        controller.mqtt.client.reconnecting = false;
+        expect(logger.error).toHaveBeenCalledTimes(1);
+        expect(logger.error).toHaveBeenCalledWith("MQTT failed to publish: topic: 'zigbee2mqtt/bulb', payload: '{\"brightness\":50,\"color\":{\"b\":10,\"g\":50,\"r\":100},\"color_temp\":370,\"dummy\":{\"1\":\"yes\",\"2\":\"no\"},\"linkquality\":99,\"state\":\"ON\"}");
     });
 
     it('Load empty state when state file does not exist', async () => {