connAckPacket.sessionPresent is not checked for mqtt3.1.1

[yyjdelete]

Seems #917 only add check for it when mqtt5.0 is used, but the flag is also available and should also be checked for mqtt3.1
This will make resubscribe and later manually subseibe with the same topic failed if option.clean is set to false but sever already clean the session when reconnected.

MQTT.js/lib/client.js

Line 1453 in 37b12cb

(this.options.clean || (this.options.protocolVersion === 5 && !connack.sessionPresent)) &&

[YoDaMa]

Hey @yyjdelete thanks for identifying this. could you provide a repro example so I can run it and identify this bug and that the fix handles it?

[yyjdelete]

@YoDaMa
You need an your own MQTT broker(an local mosquitto for me) and stop and restart it manually after see Please restart mqtt server now,

var mqtt = require('mqtt')

var cid = 'mqttjs_' + Math.random().toString(16).substr(2, 8)
var i = 0

var test = function(testSessionExpire){
	//test with an local mosquitto or other server
	var client  = mqtt.connect('mqtt://127.0.0.1', {
		clean: false,
		clientId: cid,
		//protocolVersion: 5,
		reconnectPeriod: testSessionExpire ? 1000 : 0
	})

	client.on('connect', function () {
		console.log('connect=>' + (++i))
		client.subscribe('presence', function (err) {
			console.log('presence', err)
			if (!err) {
				client.publish('presence', 'Hello mqtt=>' + (+new Date()))
			}
		})
	})

	client.on('message', function (topic, message) {
		// message is Buffer
		console.log(message.toString())
		if (testSessionExpire)
			console.log('Please restart mqtt server now')
		else
			setTimeout(()=>test(false), 1000)
	})
}
test(true)

As you can see, subscribe didn't report any error, but the second Hello mqtt is missing, seems the subscribe after reconnect is simple ignored, and you can never receive any new message. And that don't happen when I set protocolVersion to 5.
I capture with wireshark (testSessionExpire = true and false) and ensure at least mosquitto can properly set Session Present flag for 3.1.1

connect=>1
presence null
Hello mqtt=>1632933523810
Please restart mqtt server now
connect=>2
presence null

[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