SignalK / signalk-server

An implementation of a Signal K central server for boats.
http://signalk.org
Apache License 2.0
304 stars 152 forks source link

app.getFeatures broken #1777

Open VladimirKalachikhin opened 3 weeks ago

VladimirKalachikhin commented 3 weeks ago

Signal K Server version 2.8.3

The server plugin function app.getFeatures does not work. Code:

plugin.start = function (options, restartPlugin) {
  let features = app.getFeatures();
};

Result:

TypeError: app.getPluginsList is not a function
    at Server.<anonymous> (/usr/local/lib/node_modules/signalk-server/lib/index.js:94:36)
    at Generator.next (<anonymous>)
    at /usr/local/lib/node_modules/signalk-server/lib/index.js:26:71
    at new Promise (<anonymous>)
    at __awaiter (/usr/local/lib/node_modules/signalk-server/lib/index.js:22:12)
    at Object.Server.app.getFeatures (/usr/local/lib/node_modules/signalk-server/lib/index.js:91:40)
    at Object.plugin.start (/home/stager/projects/webNav/signalk/galadrielcacheskcharts/index.js:29:20)
    at doPluginStart (/usr/local/lib/node_modules/signalk-server/lib/interfaces/plugins.js:305:20)
    at doRegisterPlugin (/usr/local/lib/node_modules/signalk-server/lib/interfaces/plugins.js:459:13)
    at registerPlugin (/usr/local/lib/node_modules/signalk-server/lib/interfaces/plugins.js:258:13)
    at /usr/local/lib/node_modules/signalk-server/lib/interfaces/plugins.js:209:13
    at Array.forEach (<anonymous>)
    at startPlugins (/usr/local/lib/node_modules/signalk-server/lib/interfaces/plugins.js:208:85)
    at Object.start (/usr/local/lib/node_modules/signalk-server/lib/interfaces/plugins.js:46:13)
    at /usr/local/lib/node_modules/signalk-server/lib/index.js:503:50
    at /usr/local/lib/node_modules/signalk-server/node_modules/lodash/lodash.js:4967:15
    at Function.forIn (/usr/local/lib/node_modules/signalk-server/node_modules/lodash/lodash.js:13018:11)
    at startInterfaces (/usr/local/lib/node_modules/signalk-server/lib/index.js:480:22)
    at Server.<anonymous> (/usr/local/lib/node_modules/signalk-server/lib/index.js:328:17)
    at Generator.next (<anonymous>)
    at fulfilled (/usr/local/lib/node_modules/signalk-server/lib/index.js:23:58)
    at processTicksAndRejections (node:internal/process/task_queues:96:5)
tkurki commented 3 weeks ago

@panaaj any ideas?

panaaj commented 3 weeks ago

The issue is due to calling getFeatures() synchronously in the start up function. It fails because the plugin manager is still starting plugins and the ability to return a list of plugins is not yet available.

A work around for invoking getFeatures() in the plugin start() function is to use setTimeout().

start() {
   setTimeout( async () => await server.getFeatures(), 500)
}

Will add a test to getFeatures() so it can fail gracefully and throw a meaningful error to the caller.

VladimirKalachikhin commented 3 weeks ago

A real workaround is:

setImmediate(()=>app.getFeatures());

but it's pointless.

The real code could be

setImmediate(() => {let features = app.getFeatures();});

but!:

setImmediate(()=>{let features = app.getFeatures(); app.debug(features instanceof Promise)});

In fact .getFeatures() return Promise when as the documentation says "Returns an object detailing the available APIs and Plugins."

OK, if the .getFeatures() returns Promise instead result, then the code should be:

app.getFeatures().then((features)=>{app.debug('features list:',features)});

but it's get TypeError: app.getPluginsList is not a function That is the problem.

So a real real workaround is:

setImmediate(() => {app.getFeatures().then((features)=>{app.debug('features list:',features)});});

That is, to start the asynchronous function only on the next turn of the main cycle. It seems to me that this is nonsense.

tkurki commented 3 weeks ago

This is kinda tricky: plugin A provides feature 1 if there is feature 2 available and feature 2 is provided by plugin B, that needs to be started. Somewhat similar to systemd dependencies with requires and wants.

We have already a similar method for plugins to "announce" properties:

The semantic difference to regular events is that listeners get the set of all emitted values since server startup, not just single values. A poor man's event broker with retain = true, if you will.

We could change the feature detection api to use app properties API, forcing plugins to react to features coming available.

This would also cover the case where a plugin providing a feature is activated later, after server startup.