From 8e83d3e3f082b66911973a9954decfdf331467a0 Mon Sep 17 00:00:00 2001 From: Bert Kleewein Date: Thu, 10 Mar 2022 11:08:31 -0800 Subject: [PATCH 1/5] fix: fix regression from #1401 and allow CI test failures to break github workflow --- .github/workflows/mqttjs-test.yml | 1 - lib/client.js | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) 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..bcab37636 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1583,7 +1583,7 @@ MqttClient.prototype._handleAck = function (packet) { const that = this let err - if (!cb || cb === nop) { + if (!cb) { debug('_handleAck :: Server sent an ack in error. Ignoring.') // Server sent an ack in error, ignore it. return From ce343b46ed963a672a4d1261549d76e40db4a9a7 Mon Sep 17 00:00:00 2001 From: Bert Kleewein Date: Thu, 10 Mar 2022 11:23:05 -0800 Subject: [PATCH 2/5] temporary rollback to verify break works --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index bcab37636..294cb1149 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1583,7 +1583,7 @@ MqttClient.prototype._handleAck = function (packet) { const that = this let err - if (!cb) { + if (!cb || cb === nop) { debug('_handleAck :: Server sent an ack in error. Ignoring.') // Server sent an ack in error, ignore it. return From 13866be5fb16c9c9b54878a30eb13217f4a648b3 Mon Sep 17 00:00:00 2001 From: Bert Kleewein Date: Thu, 10 Mar 2022 11:29:00 -0800 Subject: [PATCH 3/5] put change back in --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index 294cb1149..bcab37636 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1583,7 +1583,7 @@ MqttClient.prototype._handleAck = function (packet) { const that = this let err - if (!cb || cb === nop) { + if (!cb) { debug('_handleAck :: Server sent an ack in error. Ignoring.') // Server sent an ack in error, ignore it. return From 47ee1eb3b98388fe9336276bc74e89f6ab70d185 Mon Sep 17 00:00:00 2001 From: Bert Kleewein Date: Mon, 14 Mar 2022 15:30:46 -0700 Subject: [PATCH 4/5] add comment to fix --- lib/client.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/lib/client.js b/lib/client.js index bcab37636..ca5c7848d 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1583,6 +1583,17 @@ MqttClient.prototype._handleAck = function (packet) { const that = this let err + // 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.subsribe, 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. From fb3b78e33cdc94347a60a2a7dca96c59ff7cf157 Mon Sep 17 00:00:00 2001 From: Bert Kleewein Date: Mon, 14 Mar 2022 15:48:54 -0700 Subject: [PATCH 5/5] fix misspelling --- lib/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client.js b/lib/client.js index ca5c7848d..e90bd2261 100644 --- a/lib/client.js +++ b/lib/client.js @@ -1592,7 +1592,7 @@ MqttClient.prototype._handleAck = function (packet) { // there's no reason to change it and risk (another) regression. // // The only reason this code works is becaues code in MqttClient.publish, - // MqttClinet.subsribe, and MqttClient.unsubscribe ensures that we will + // 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.')