Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

External Converters Loading Stopping at errors #26763

Open
ivanfmartinez opened this issue Mar 16, 2025 · 5 comments
Open

External Converters Loading Stopping at errors #26763

ivanfmartinez opened this issue Mar 16, 2025 · 5 comments
Labels
problem Something isn't working

Comments

@ivanfmartinez
Copy link
Contributor

What happened?

After upgrading to 2.x one of my devices stopped working (As it was a device that i did not use normally I did not see this before), the device shows as unsupported but no error for this external converter are show on the log.

But there was an error related to an old device that I did not use anymore.

After some checking I found that when one external converter fails to load the others are not loaded.

Looking at the code as this is a generic JS class the problem can happen at other places

Here de load of all JS files a single for iterating over the files, with no error check

private async loadFiles(): Promise<void> {

Here the load of a single external converter which log the error and throw and this is cancelling the load all loop

protected async loadJS(name: string, module: ModuleExports): Promise<void> {

What did you expect to happen?

Only the faulty external converter should not be load and all others can be loaded.

How to reproduce it (minimal and precise)

Create 2 external converters that are valid, check on the log what order they are loaded.

Change the first one to have an error.

restart z2m, it should load one and skip the one with error, but none are loaded.

The test code are using only valid converters and loading one at a time. Should have option to load all and have invalid converter also

expect(mockMQTTPublishAsync).toHaveBeenCalledWith(

Zigbee2MQTT version

2.1.3

Adapter firmware version

20230507

Adapter

sonoff

Setup

docker container, but I think this will happen on any setup

Debug log

This error was for the external converter with error, but the problem is that others that should be loaded after this are skipped.

[2025-03-16 17:46:23] error:    z2m: Failed to call 'ExternalConverters' 'start' (/app/data/extension/TS0225_sensor_presenca.js:34
                e.occupancy(), e.illuminance_lux(),
                                 ^

TypeError: e.illuminance_lux is not a function
    at /app/data/extension/TS0225_sensor_presenca.js:34:20
    at Script.runInContext (node:vm:149:12)
    at Script.runInNewContext (node:vm:154:17)
    at runInNewContext (node:vm:310:38)
    at ExternalConverters.loadModuleFromText (/app/lib/extension/externalJS.ts:187:24)
    at ExternalConverters.loadFiles (/app/lib/extension/externalJS.ts:156:52)
    at processTicksAndRejections (node:internal/process/task_queues:105:5)
    at ExternalConverters.start (/app/lib/extension/externalJS.ts:45:9)
    at Controller.callExtensions (/app/lib/controller.ts:368:17)
    at Controller.start (/app/lib/controller.ts:186:9))


@ivanfmartinez ivanfmartinez added the problem Something isn't working label Mar 16, 2025
@mrmaximas
Copy link

@ivanfmartinez please read this #24198
Renamed illuminance_lux to illuminance, this breaks in two ways: illuminance_lux has been removed and the value of illuminance changes.

@ivanfmartinez
Copy link
Contributor Author

@mrmaximas the problem that I related is not about the illuminance, is about not loading other converters after one converter gives exception.

That converter with illuminance errors is just an older converter that I dont need anymore, but as they still on the directory it breaks the load of other converter.

I'm not reporting the error with illuminance converter, I just showed an example that is the only error show, but the other converter that are correct are not loaded.

@mrmaximas
Copy link

mrmaximas commented Mar 17, 2025

@ivanfmartinez how about this?

External converters and extensions
The external_converters setting is no longer used. Instead all external converters inside data/external_converters directory are now automatically loaded. Make sure to move all your external converters to this directory.
External extensions are now loaded from data/external_extensions instead of data/extension. Make sure to rename your data/extension directory (if present) to data/external_extensions.
The constructor signature for external extensions now matches that of internal extensions (with settings and logger as extras as before). The updated signature can be inspected here

@ivanfmartinez
Copy link
Contributor Author

ivanfmartinez commented Mar 17, 2025

@mrmaximas I'm using the .js files in the data/external_converters directory since I migrate to 2.x series. You still did not understand what I reported here.

I had 3 js files in that directory data/external_conversters directory

The first one was loaded correctly because was OK.
The second one was not load because the illuminance error and throw one exception at loading
The third one WAS NOT LOAD because of the error on the SECOND the loading STOPPED.

AFTER I removed the file with error or resolved the error on that file the other file was loaded correctly.

I have showed that the code that load all exception is a single for loop which does not check for exceptions and the loading of single JS throw an error, and this will break the for loop that try to load all js files, without an explicity message indicating that the loading of all js files stopped I spend some time to understand what is happening.

It should not stop loading other js when one have errors, or at least should give a clear indication that will not load others.
As I remember on 1.x series this not happen, when I had errors others external did not stopped working (or maybe my errors happened only on last definitions)

In my case now I understand the case, but someone else can be in this case later, and I understand that this should be at least

  • DOCUMENTED
    • If you have one extension with error - maybe others will not be loaded
  • CHANGED
    • If one extesion has an error, log the error and keep loading others

For reference this is my timelime why I did not find that on first upgrade to 2.x
- I have made the migraton from the definition to directory without problem, after first upgrade to 2.x series. I have migrated all my js files
- But I have tested only the devices that I use currently.
- I did not realize that one device was not correctly detected because that device was not used in the hot days, now I have to use that device and it does not worked
- Yesterday I found that the device are not working and showing as unsupported on z2m
- I enabled the debug to find the error and and found the problem that was with a definition that I really did not use anymore but as they still have the .js extesion the current process try to load it (before I have to explicity indicate wich js to load).

Nerivec added a commit to Nerivec/zigbee2mqtt that referenced this issue Mar 17, 2025
Nerivec added a commit to Nerivec/zigbee2mqtt that referenced this issue Mar 17, 2025
@Koenkk
Copy link
Owner

Koenkk commented Mar 18, 2025

Nerivec added a commit to Nerivec/zigbee2mqtt that referenced this issue Mar 18, 2025
Nerivec added a commit to Nerivec/zigbee2mqtt that referenced this issue Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
problem Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants