Improvement to debugging early errors when loading nodes

Core Development Feature Requests
1

Hi, I managed to break something when messing around and I was reminded of an annoying oversight in loader.js.

Around lines 78 to 98, there are a couple of try/catch sections but the catch does not really show anything useful.

Here is an example from my recent log:

Welcome to Node-RED
===================
0|Node-RED |
18 Jul 10:19:34 - [info] Node-RED version: v4.1.0-beta.2
18 Jul 10:19:34 - [info] Node.js  version: v22.15.0
18 Jul 10:19:34 - [info] Windows_NT 10.0.26100 x64 LE

18 Jul 10:19:35 - [info] Loading palette nodes
TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at Object.dirname (node:path:739:5)
    at loadNodeLocales (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:234:35)
    at loadPluginConfig (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:331:11)
    at loadModuleTypeFiles (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:83:31)
    at loadModuleFiles (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:122:60)
    at load (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:43:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5) {
  code: 'ERR_INVALID_ARG_TYPE'
}
18 Jul 10:19:35 - [info] Node-RED Contrib Theme Collection version: v4.0.11

As you can see, this does not tell me which node/plugin failed to load.

Thankfully, there is a very easy fix for this:

            try {
                let promise;
                if (type === "nodes") {
                    promise = loadNodeConfig(things[thingName]);
                } else if (type === "plugins") {
                    promise = loadPluginConfig(things[thingName]);
                }
                promises.push(
                    promise.then(
                        (function() {
                            const n = thingName;
                            return function(nodeSet) {
                                things[n] = nodeSet;
                                return nodeSet;
                            }
                        })()
                    ).catch(err => {console.log(`"${thingName}"`, err)})
                );
            } catch(err) {
                console.log(`"${thingName}"`, err)
            }
        }

Notice the addition of thingName to the log output. Which means that we now get to see what node/plugin caused the issue:

Welcome to Node-RED
===================
0|Node-RED |
18 Jul 10:29:20 - [info] Node-RED version: v4.1.0-beta.2
18 Jul 10:29:20 - [info] Node.js  version: v22.15.0
18 Jul 10:29:20 - [info] Windows_NT 10.0.26100 x64 LE

18 Jul 10:29:21 - [info] Loading palette nodes
"ti-dummy-runtime-plugin" TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at Object.dirname (node:path:739:5)
    at loadNodeLocales (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:234:35)
    at loadPluginConfig (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:331:11)
    at loadModuleTypeFiles (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:83:31)
    at loadModuleFiles (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:122:60)
    at load (D:\src\nr\node_modules\@node-red\registry\lib\loader.js:43:12)
    at process.processTicksAndRejections (node:internal/process/task_queues:105:5) {
  code: 'ERR_INVALID_ARG_TYPE'
}
18 Jul 10:29:22 - [info] Node-RED Contrib Theme Collection version: v4.0.11

A nice easy fix. :smiley:

2

While debugging all this, I've also found some small bugs in loader.js.

I've tested both of the proposed fixes.

loadPluginConfig

This function tries to set both plugin.file and plugin.template. At least one of these is needed for the call to loadNodeLocales(plugin). This can fail if the plugin js is in a file *.cjs rather than *.js even though *.cjs is a perfectly valid filename.

At the least, I believe loadPluginConfig should throw a meaningful error if neither file nor template properties exist.

Ideally, the *.js file check should be expanded to look for *.cjs if the first check fails. Possibly also look for *.mjs if needed to allow for future use of modules.

Example fix:

    var jsFile = file.replace(/\.[^.]+$/,".js");
    var htmlFile = file.replace(/\.[^.]+$/,".html");
    if (fs.existsSync(jsFile)) {
        plugin.file = jsFile;
    } else {
        jsFile = file.replace(/\.[^.]+$/,".cjs")
        if (fs.existsSync(jsFile)) {
            plugin.file = jsFile;
        }
    }
    if (fs.existsSync(htmlFile)) {
        plugin.template = htmlFile;
    }
    if (!plugin.file && !plugin.template) {
        throw new Error(`Plugin file not found: "${file}"`);
    }

    await loadNodeLocales(plugin)

loadNodeConfig

This is a little simpler but fails for a similar but not quite identical issue. In this case, the file property is OK but the template property fails because, again, the replace assumes a source filename in the pattern *.js. This is actually an easier fix because we can easily extend the regex.

Old regex: /\.js$/i
New regex: /\.(js|cjs)$/i

3

Please raise on github - either as an issue, or even a PR given you have proposed specific code changes. The forum isn't the place to report bugs.

2 Likes
4

I wasn't sure it was a bug - the initial part is an enhancement request rather than a bug report.

However, I will report the bug.

Done:

2 Likes