Registration schema `select` option not implemented properly in 3.x

[LongLiveCHIEF]

Using version: 3.2.0

I have the following manifest:

//manifest.js
var manifest = {
  connections:[
    {
      host: appname.host,
      port: appname.port.web,
      labels: ['web']
    },
    {
      host: appname.host,
      port: appname.port.client,
      labels: ['app-client']
    }
  ],
  registrations: [
    {
      plugin: {
        register:'../services/app-client',
        options: {
          select: ['app-client']
        }
      }
    }
  ]
};

I am getting an error in my plugin when trying to use server.listener:

//services/app-client/index.js
var SocketIO = require('socket.io');

exports.register = function appClient(server, options, next){

  var io = SocketIO(server.listener);

  io.sockets.on('connection', function(socket){
    console.log("new connection!");
    socket.emit("app_connected", {
      msg: "app client connected"
    })
  });

  next();
}

Here's the error:

if ('object' == typeof srv && !srv.listen) {

TypeError: Cannot read property 'listen' of null

However, if I change the connection passed to SocketIO(server.select('app-client').listener), then everything works as expected. The point of select in the registration options is so that you don't have to use select inside your plugin, making it adaptable across your code.

While the existing tests do allow the select option in the schema, I don't see that they actually test that the individual server.connection object is passed to the plugin. It looks like it still provides the entire server.settings object to the plugin instead.

[mtharrison]

The select option is in the wrong place in your manifest, so your root server object is being given to the plugin, hence the error and the need for you to select manually inside the plugin.

The plugin.options object is reserved for the options passed directly to the plugin (i.e the second parameter in your exports.register function). select, prefix etc need to go one level above i.e.:

//manifest.js
var manifest = {
    connections:[
        {
            host: appname.host,
            port: appname.port.web,
            labels: ['web']
        },
        {
            host: appname.host,
            port: appname.port.client,
            labels: ['app-client']
        }
    ],
    registrations: [
        {
            plugin: {
                register:'../services/app-client',
                options: {
                    something: 'else'   // passed to plugin as options.something
                }
            },
            options: {
                select: ['app-client']   // used internally by hapi when registering plugin
            }
        }
    ]
};

If you don't need any plugin options, you can make it a bit more concise:

//manifest.js
var manifest = {
    connections:[
        {
            host: appname.host,
            port: appname.port.web,
            labels: ['web']
        },
        {
            host: appname.host,
            port: appname.port.client,
            labels: ['app-client']
        }
    ],
    registrations: [
        {
            plugin: '../services/app-client',
            options: {
                select: ['app-client']
            }
        }
    ]
};

[LongLiveCHIEF]

There's something definitely odd going on here still, even after I update to the concise code you describe. The server always exits immediately. If I log out server.connections, I get 1 or 2, based on whether or not I apply the select option... but server.listener is always null.

[mtharrison]

The above works for me. Perhaps try to create a repo on here with a minimal example showing the problem and I'll take a look. It's hard to debug any further without seeing the full thing.

[LongLiveCHIEF]

I created a repo, and wound up discovering that my error was due to the use of port 80 on my mac, which requires the user to be running as root.

I spent 4 hours working through this, just to discover I should select a different port :weary:

Side note, the server exits cleanly with a 0 exit code, even though it couldn't establish a connection, and even if I was using error handling.

[mtharrison]

You need to check for the server.start() error, which will be present if you're listening to a port you're not allowed, and throw it, you'll get a non-zero then:

const Glue = require('glue');

const manifest = {
    connections:[ { port: 80 } ]
};

Glue.compose(manifest, { relativeTo: __dirname }, (err, server) => {

    if (err) {
        throw err;
    }

    server.start((err) => {

        if (err) {
            throw err;     // check for me
        }
    });
});

Output from running:

$ node index.js
/Users/.../index.js:16
            throw err;
            ^

Error: listen EACCES 0.0.0.0:80
    at Object.exports._errnoException (util.js:837:11)
    at exports._exceptionWithHostPort (util.js:860:20)
    at Server._listen2 (net.js:1218:19)
    at listen (net.js:1267:10)
    at net.js:1376:9
    at doNTCallback3 (node.js:440:9)
    at process._tickCallback (node.js:346:17)
    at Function.Module.runMain (module.js:477:11)
    at startup (node.js:117:18)
    at node.js:951:3

$ echo $?
1

[LongLiveCHIEF]

yup. you're right. I was only handling the Compose((err, server) => {}) error. It's been a long week already. There are days you catch these user mistakes in 10 minutes... and then days where it takes you 4 hours. :-)

[lock[bot]]

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.