client.reconnect() will not proper set client.disconnecting to false

[yyjdelete]

reconnect will set disconnecting and disconnected to false, but it will call end later and reset them to true, even event connect is already fired. And this make later _checkDisconnecting() always failed.

MQTT.js/lib/client.js

Lines 912 to 915 in 37b12cb

	that.disconnecting = false
	that.disconnected = false
	that._deferredReconnect = null
	that._reconnect()

MQTT.js/lib/client.js

Line 855 in 37b12cb

this.disconnecting = true

var client  = mqtt.connect('ws://xxxx/mqtt', {'username':'test'});

    var i = 0;
client.on('connect', function() {
    console.log(client.disconnecting)
    console.log(client.disconnected)
    console.log(client.connected)
    setTimeout(() => client.reconnect(), 1000);
});

times 0
disconnecting:  false
disconnected:  undefined
connected:  true
---
times 1
disconnecting:  true
disconnected:  true
connected:  false
---
times 2
disconnecting:  false
disconnected:  false
connected:  false

2. Another question, _setupPingTimer() is skipped when disconnected is true, so maybe I can get connection without ping for the second try? I didn't found any other code to set disconnected to false, but pingTimer maybe already broken when pingResp is already false in last connection.
   

   MQTT.js/lib/client.js

   Lines 1481 to 1489 in 37b12cb

   	MqttClient.prototype._onConnect = function (packet) {
   	if (this.disconnected) {
   	this.emit('connect', packet)
   	return
   	}
   	var that = this
   	this._setupPingTimer()

[rodrigobrochado]

The disconnected property should be already deprecated, following issue #690, but seems to be abandoned.

BTW, I'm having some instabilities with the client, but I'm testing some boosts on the broker settings to see if it fixes them. While trying to find some solution, I've also take a look on the code and this part took my attention:

MQTT.js/lib/client.js

Lines 1551 to 1555 in d8be59e

	MqttClient.prototype._onConnect = function (packet) {
	if (this.disconnected) {
	this.emit('connect', packet)
	return
	}

If disconnected is deprecated, the condition there should be changed to !this.connected and I doubt the goal of the return statement, although I didn't reviewed it exhaustively.
Don't know if it helps you @yyjdelete , but just to say that I'm also having issues and have some doubts in client.js

It'll be great if you have some time to comment @mcollina :)

[YoDaMa]

@rodrigobrochado I'm working to aggregate all this good feedback so we can make sure it gets addressed in the "vNext" (major version update) of this library. Could you provide a discussion commend here: #1324, with a summary of what you want done from this issue. It would be helpful.

[rodrigobrochado]

Thanks @YoDaMa. I think the huge work you've done in #1318, #1325 and are doing on vNext will probably fix this issue.

My client was misbehaving and seemed to be related to the broker rejecting packages when it was full. When digging on the client source code, I've come to the strange behavior of the disconnected property, but that was vanished on #1318.
Thanks for your work!

[robertsLando]

MQTT 5.0.0 BETA is now available! Try it out and give us feedback: npm i mqtt@beta. It may fix your issues

[github-actions]

This is an automated message to let you know that this issue has
gone 365 days without any activity. In order to ensure that we work
on issues that still matter, this issue will be closed in 14 days.

If this issue is still important, you can simply comment with a
"bump" to keep it open.

Thank you for your contribution.

[github-actions]

This issue was automatically closed due to inactivity.