Does not clear outgoing store if no callback is added to publish

[dachrillz]

Hello everyone.

We have been using your library and noticed that the default outgoing store kept growing if no callback is added to the publish function.

The version that we are using according to yarn.lock:
mqtt@^4.2.6: version "4.2.6" resolved "https://registry.yarnpkg.com/mqtt/-/mqtt-4.2.6.tgz#b655547a9cfb3d86bfb398948b8dbb37e2e3bfd0" integrity sha512-GpxVObyOzL0CGPBqo6B04GinN8JLk12NRYAIkYvARd9ZCoJKevvOyCaWK6bdK/kFSDj3LPDnCsJbezzNlsi87Q== dependencies: commist "^1.0.0" concat-stream "^2.0.0" debug "^4.1.1" help-me "^1.0.1" inherits "^2.0.3" minimist "^1.2.5" mqtt-packet "^6.6.0" pump "^3.0.0" readable-stream "^3.6.0" reinterval "^1.1.0" split2 "^3.1.0" ws "^7.3.1" xtend "^4.0.2"

If I publish like this
mqttClient.publish( topic, JSON.stringify(data), {qos: 1});

This leads to a growing outgoing store if i print mqttClient.
console.log({ store: mqttClient.outgoingStore });

{ store: Store { options: { clean: true }, _inflights: Map(9) { 10212 => [Object], 10213 => [Object], 10214 => [Object], 10215 => [Object], 10216 => [Object], 10217 => [Object], 10218 => [Object], 10219 => [Object], 10220 => [Object] } }

If I publish like this
mqttClient.publish( topic, JSON.stringify({ ...data, traceparent }), {qos: 1}, (e) => { console.log("handled publish")} );

The store is cleared in between publishes.

Forgive me if this is something that has been fixed in a newer release. I figured it would be reasonable to report this finding in case it is a bug. If it is required that one has to add a callback to the publish function, then maybe this should be documented more explicitly.

Thank you for your time :)

[YoDaMa]

Thanks for the bug report. Could you repro it on the latest version? Likely this is an issue, since the Store implementation is convoluted. However the workaround seems to be to just add a no op callback, yes? Either way, if you can find and fix the bug we will accept a PR gladly.

[dachrillz]

Hello again,

I did some further work on this.

First of all i tried it with this version according to my package-lock.json (I think it's the latest)

"node_modules/mqtt": { "version": "4.3.6", "resolved": "https://registry.npmjs.org/mqtt/-/mqtt-4.3.6.tgz",

The bug seems to persist in this version as well. However, I think I tracked down why the bug occurs. In the function MqttClient.prototype._handleAck

The following if statement returns before the outgoing message is deleted.
if (!cb || cb === nop) { debug('_handleAck :: Server sent an ack in error. Ignoring.') // Server sent an ack in error, ignore it. return }

This checks out since I don't pass any callback to client.publish. There seems to be a default callback that is created when no other callback is passed to client.publish.
ƒ nop (error) {\n debug('nop ::', error)\n}
Which triggers the if-statement above. Removing the check for 'nop' obviously allows it to progress to the part that deletes the outgoing message.

However, I noticed a recent commit that added this logic
commit

Most likely the bug was introduced here. I'm not entirerly sure how to fix it since I'm not too familiar with the internals of the library.

[YoDaMa]

@BertKleewein could you take a look at this and #1437. It looks like there is an issue that was introduced in the 4.3.6 release potentially...

[BertKleewein]

@dachrillz Sorry that my bug caused you problems. This is fixed for next release: #1443