diff --git a/.github/workflows/mqttjs-test.yml b/.github/workflows/mqttjs-test.yml index afb33c76a..b5d18f330 100644 --- a/.github/workflows/mqttjs-test.yml +++ b/.github/workflows/mqttjs-test.yml @@ -30,7 +30,6 @@ jobs: - run: npm ci - name: Node Tests run: npm run test:node - continue-on-error: true env: CI: true DEBUG: "mqttjs" diff --git a/lib/client.js b/lib/client.js index 294cb1149..e90bd2261 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1583,7 +1583,18 @@ MqttClient.prototype._handleAck = function (packet) { const that = this let err - if (!cb || cb === nop) { + // Checking `!cb` happens to work, but it's not technically "correct". + // + // Why? This code assumes that "no callback" is the same as that "we're not + // waiting for responses" (puback, pubrec, pubcomp, suback, or unsuback). + // + // It would be better to check `if (!this.outgoing[messageId])` here, but + // there's no reason to change it and risk (another) regression. + // + // The only reason this code works is becaues code in MqttClient.publish, + // MqttClinet.subscribe, and MqttClient.unsubscribe ensures that we will + // have a callback even if the user doesn't pass one in.) + if (!cb) { debug('_handleAck :: Server sent an ack in error. Ignoring.') // Server sent an ack in error, ignore it. return