martin-doyle / node-red-contrib-aedes

MQTT broker for Node-RED based on Aedes
MIT License
62 stars 11 forks source link

Close timeout workaround #48

Closed dbaba closed 3 years ago

dbaba commented 3 years ago

This PR is a workaround for issue #46.

On stopping the flow, the node closes an Aedes broker after receiving clientDisconnect event for all clients. When there's a client not under the control of NR, the node triggers the 1-second timeout and performs the forced shutdown. The major change is that the callback function on the node close event never uses done argument so NR never waits for the broker to be terminated.

The forced shutdown prevents a new Aedes Broker from starting as the port is still in use by the previous broker instance. So in this case, users will see Error: Port 1883 is already in use message. However, the new broker will automatically restart when detecting EADDRINUSE and work fine in the end after the preceding instance is shut down.

dbaba commented 3 years ago

@martin-doyle Any update on this?

martin-doyle commented 3 years ago

As far as I can see the workaround is a 1 second delay on the shutdown. To be honest I would rather have a fix on the MQTT node. If the workaround works for you I am fine. At the moment I am not convinced this is the right way to go.

dbaba commented 3 years ago

This is not a permanenxt fix but just a workaround.

To be honest I would rather have a fix on the MQTT node. Do you know ETA?

So far, this node is not useful in the latest version of NR, which makes, I'd say, the node users unhappy. This is why I created a patch. But you want to wait for NR team to fix it, I respect your choise.