From 8ab96e124fa5e24e504b2a11ae6cf9836d93f5c8 Mon Sep 17 00:00:00 2001
From: koenkk <koenkanters94@gmail.com>
Date: Mon, 4 Dec 2023 20:40:45 +0100
Subject: [PATCH] fix: Don't crash on startup when external converters fails to
 load https://github.com/Koenkk/zigbee2mqtt/issues/20010

---
 lib/extension/externalConverters.ts | 24 ++++++++++++++++++-----
 lib/util/utils.ts                   | 30 +++++++++++++----------------
 test/externalConverters.test.js     |  8 ++++++++
 3 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts
index 2d8fb69648..138d1e063c 100644
--- a/lib/extension/externalConverters.ts
+++ b/lib/extension/externalConverters.ts
@@ -1,7 +1,8 @@
 import * as zhc from 'zigbee-herdsman-converters';
 import * as settings from '../util/settings';
-import utils from '../util/utils';
+import {loadExternalConverter} from '../util/utils';
 import Extension from './extension';
+import logger from '../util/logger';
 
 export default class ExternalConverters extends Extension {
     constructor(zigbee: Zigbee, mqtt: MQTT, state: State, publishEntityState: PublishEntityState,
@@ -9,10 +10,23 @@ export default class ExternalConverters extends Extension {
         restartCallback: () => void, addExtension: (extension: Extension) => Promise<void>) {
         super(zigbee, mqtt, state, publishEntityState, eventBus, enableDisableExtension, restartCallback, addExtension);
 
-        for (const definition of utils.getExternalConvertersDefinitions(settings.get())) {
-            const toAdd = {...definition};
-            delete toAdd['homeassistant'];
-            zhc.addDefinition(toAdd);
+        for (const file of settings.get().external_converters) {
+            try {
+                for (const definition of loadExternalConverter(file)) {
+                    const toAdd = {...definition};
+                    delete toAdd['homeassistant'];
+                    zhc.addDefinition(toAdd);
+                }
+            } catch (error) {
+                logger.error(`Failed to load external converter file '${file}' (${error.message})`);
+                logger.error(
+                    `Probably there is a syntax error in the file or the external converter is not ` +
+                    `compatible with the current Zigbee2MQTT version`);
+                logger.error(
+                    `Note that external converters are not meant for long term usage, it's meant for local ` +
+                    `testing after which a pull request should be created to add out-of-the-box support for the device`,
+                );
+            }
         }
     }
 }
diff --git a/lib/util/utils.ts b/lib/util/utils.ts
index 46d5a1082c..9ebb910720 100644
--- a/lib/util/utils.ts
+++ b/lib/util/utils.ts
@@ -162,25 +162,21 @@ function loadModuleFromFile(modulePath: string): unknown {
     return loadModuleFromText(moduleCode);
 }
 
-function* getExternalConvertersDefinitions(settings: Settings): Generator<ExternalDefinition> {
-    const externalConverters = settings.external_converters;
+export function* loadExternalConverter(moduleName: string): Generator<ExternalDefinition> {
+    let converter;
 
-    for (const moduleName of externalConverters) {
-        let converter;
-
-        if (moduleName.endsWith('.js')) {
-            converter = loadModuleFromFile(data.joinPath(moduleName));
-        } else {
-            converter = require(moduleName);
-        }
+    if (moduleName.endsWith('.js')) {
+        converter = loadModuleFromFile(data.joinPath(moduleName));
+    } else {
+        converter = require(moduleName);
+    }
 
-        if (Array.isArray(converter)) {
-            for (const item of converter) {
-                yield item;
-            }
-        } else {
-            yield converter;
+    if (Array.isArray(converter)) {
+        for (const item of converter) {
+            yield item;
         }
+    } else {
+        yield converter;
     }
 }
 
@@ -439,7 +435,7 @@ function getScenes(entity: zh.Endpoint | zh.Group): Scene[] {
 export default {
     endpointNames, capitalize, getZigbee2MQTTVersion, getDependencyVersion, formatDate, objectHasProperties,
     equalsPartial, getObjectProperty, getResponse, parseJSON, loadModuleFromText, loadModuleFromFile,
-    getExternalConvertersDefinitions, removeNullPropertiesFromObject, toNetworkAddressHex, toSnakeCase,
+    removeNullPropertiesFromObject, toNetworkAddressHex, toSnakeCase,
     parseEntityID, isEndpoint, isZHGroup, hours, minutes, seconds, validateFriendlyName, sleep,
     sanitizeImageParameter, isAvailabilityEnabledForEntity, publishLastSeen, availabilityPayload,
     getAllFiles, filterProperties, flatten, arrayUnique, clone, computeSettingsToChange, getScenes,
diff --git a/test/externalConverters.test.js b/test/externalConverters.test.js
index 5775910ba3..2ac9859257 100644
--- a/test/externalConverters.test.js
+++ b/test/externalConverters.test.js
@@ -134,4 +134,12 @@ describe('Loads external converters', () => {
             mock: 2
         });
     });
+
+    it('Loads external converters with error', async () => {
+        fs.copyFileSync(path.join(__dirname, 'assets', 'mock-external-converter.js'), path.join(data.mockDir, 'mock-external-converter.js'));
+        settings.set(['external_converters'], ['mock-external-converter.js']);
+        zigbeeHerdsmanConverters.addDefinition.mockImplementationOnce(() => {throw new Error('Invalid definition!')});
+        await resetExtension();
+        expect(logger.error).toHaveBeenCalledWith(`Failed to load external converter file 'mock-external-converter.js' (Invalid definition!)`);
+    });
 });
\ No newline at end of file