fix: Don't crash on startup when external converters fails to load #2…

…0010
Koenkk · Dec 4, 2023 · 8ab96e1 · 8ab96e1
1 parent 483acc3
commit 8ab96e1
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 22 deletions.
diff --git a/lib/extension/externalConverters.ts b/lib/extension/externalConverters.ts
@@ -1,18 +1,32 @@
 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,
         eventBus: EventBus, enableDisableExtension: (enable: boolean, name: string) => Promise<void>,
         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
@@ -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
@@ -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!)`);
+    });
 });