Server factory integration

[getlarge]

• Refactor the protocol decoder

  • change expected arguments ( to be used by aedes-server-factory instead of aedes)
  • split in several sub functions
  • export extractSocketDetails function, usable when not using / or trusting proxy

• Update test, docs, examples and dependancies

related to aedes-changes and aedes-server-factory

[robertsLando]

Suggested change

	The function extract socket details and if aedes-server-factory `trustProxy` option is set to true, it will first parse http headers (x-real-ip | x-forwarded-for) and proxy protocol (v1 and v2) to retrieve information in client.connDetails.
	The function extract socket details and if aedes-server-factory `trustProxy` option is set to true, it will first parse http headers (x-real-ip | x-forwarded-for) and proxy protocol (v1 and v2) to retrieve information in `client.connDetails`.

[robertsLando]

@getlarge Something like:

function setup(options){
var broker;
var server;
var client;

 if(options.broker) {
 broker = aedes(options.broker)
}

if(options.client) {
 client = mqtt.connect(options.client)
}

if(server) {
  server = createServer(broker,options.server)
}

return {broker, client, server};
}

function close(setup) {
 if(client) client.end(true)
 if(broker) broker.close()
 if(server) server.close()
}

test('tcp clients have access to the ipAddress from the socket', function (t) {
  t.plan(2)

var setup = setup(....)

// code here 

t.tearDown(close.bind(t, setup);
})

[robertsLando]

@getlarge seems tests are endless on nodejs 12 /14

[getlarge]

@robertsLando yes i noticed, but what i don't understand is that prior to this commit, it was only node v10 which was hanging.

[getlarge]

Also i tried to run tests locally with node v12 and this issue does not appear, so i'm kind of stuck with this right now.

[robertsLando]

@getlarge From my experience with github actions, when weird things happen in tests the best is to disable parallel. LEt's see how this goes :) (already submitted a commit)

[robertsLando]

@getlarge Seems stucked anyway. It stops here

# tcp proxied (protocol v1) clients buffer contains MQTT packet and proxy header
ok 15 no error
ok 16 no error
ok 17 should be equal

[robertsLando]

@mcollina Is there any change in nodejs 10 to 12-14 that could cause this stuck in tests?

It stucks in this test:

aedes-protocol-decoder/test.js

Line 364 in bea03af

test('tcp proxied (protocol v1) clients buffer contains MQTT packet and proxy header', function (t) {

[getlarge]

The last test in this suite is rather tricky because there are "many" sockets opened, since i tried to reproduce the behavior of a setup with a "real" proxy server.
So maybe a client | server connection could not be closed properly when test is ending ?

[robertsLando]

@getlarge Maybe I got it. I think it could be caused by some leaked handler (open socket). I suggest you to edit your tests in a way that close waits for close callbacks before calling end.

function close(setup, t) {

var toClose = 0
var closed = 0
 if(client) {
toClose++
client.end(true, onDone)
}
 if(broker) {
toClose++
broker.close(onDone)
}
 if(server) {
toClose++
server.close(onDone)
}

function onDone() {
if(toClose === ++closed)
t.end()
}

}

[getlarge]

I will try.
What's really curious, is that the tests work locally with node v12.3.0 but not 12.19.0 (which is the version used in the CI).

[robertsLando]

Also what you could try to do is to create a custom server destroy method as server.close never triggers the callback if there are opened sockets. Or you could you this module: https://github.com/isaacs/server-destroy

[robertsLando]

What's really curious, is that the tests work locally with node v12.3.0 but not 12.19.0 (which is the version used in the CI).

No clue why but if test never ends just means some socket is left opened. So use this strategy and we will for sure find what's wrong here :)

[getlarge]

The destroy method did not help, but it seems the error is caused by this block

  socket.on('end', function (data) {
    proxyClient.end(data, function () {
      proxyClient.connected = false
    })
  })

When commented the test end properly.

[getlarge]

@robertsLando Any idea what that error means at coverall parallel phase ?

[robertsLando]

No clue but try to re enable parallel, that wasn't the problem

[robertsLando]

@getlarge lol, a typo :)

[robertsLando]

When commented the test end properly.

If it's not needed, remove it

[robertsLando]

Aedes 0.44.0 is on npm

[robertsLando]

LGTM

[mcollina]

lgtm

[robertsLando]

@getlarge Should I release a 2.0.0?

[getlarge]

@robertsLando If we strictly follow the semver, then yes you should probably go straight to 2.0.0.

[robertsLando]

@getlarge Sorry for the delay, published on npm now