From ab3a9da63036bc4a86e1daa328b1f6849cdf4841 Mon Sep 17 00:00:00 2001 From: Koen Kanters Date: Mon, 4 Oct 2021 20:51:09 +0200 Subject: [PATCH] Revert "Improve MQTT error handling. https://github.com/Koenkk/zigbee2mqtt/issues/8956" This reverts commit 45a4978ca8a1db663d31028cbbb20ce9d492e8fa. --- lib/mqtt.ts | 26 +++++++++++++++++--------- test/bridge.test.js | 12 ++++++++++++ test/controller.test.js | 10 ++++++---- 3 files changed, 35 insertions(+), 13 deletions(-) diff --git a/lib/mqtt.ts b/lib/mqtt.ts index 73144b884..313f77c9c 100644 --- a/lib/mqtt.ts +++ b/lib/mqtt.ts @@ -113,6 +113,10 @@ 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 { @@ -125,21 +129,25 @@ 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, (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(); - }); + this.client.publish(topic, payload, actualOptions, () => resolve()); }); } } diff --git a/test/bridge.test.js b/test/bridge.test.js index 04f745386..67e7c46e4 100644 --- a/test/bridge.test.js +++ b/test/bridge.test.js @@ -94,6 +94,18 @@ 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 18f664542..0af5975ec 100644 --- a/test/controller.test.js +++ b/test/controller.test.js @@ -150,16 +150,18 @@ describe('Controller', () => { controller.mqtt.client.reconnecting = false; }); - it('Log when MQTT publish fails', async () => { + it('Dont publish to mqtt when client is unavailable', async () => { await controller.start(); await flushPromises(); logger.error.mockClear(); - MQTT.mock.publish.mockImplementationOnce((topic, message, options, cb) => cb(true)); + controller.mqtt.client.reconnecting = 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(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\"}"); + 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; }); it('Load empty state when state file does not exist', async () => {