Refined null callback handling.

redboltz · redboltz · commit aa5647daf539 · 2018-08-29T22:36:04.000+09:00
Added unit test.
diff --git a/lib/client.js b/lib/client.js
@@ -953,7 +953,7 @@ default:
 
 for now i just suppressed the warnings
 */
-MqttClient.prototype._handlePublish = function (packet, done) {
+MqttClient.prototype._handlePublish = function (packet, done = function () {}) {
   var topic = packet.topic.toString()
   var message = packet.payload
   var qos = packet.qos
@@ -964,7 +964,7 @@ MqttClient.prototype._handlePublish = function (packet, done) {
     case 2:
       this.incomingStore.put(packet, function (err) {
         if (err) {
-          return done && done(err)
+          return done(err)
         }
         that._sendPacket({cmd: 'pubrec', messageId: mid}, done)
       })
@@ -974,7 +974,7 @@ MqttClient.prototype._handlePublish = function (packet, done) {
       this.emit('message', topic, message, packet)
       this.handleMessage(packet, function (err) {
         if (err) {
-          return done && done(err)
+          return done(err)
         }
         // send 'puback' if the above 'handleMessage' method executed
         // successfully.
@@ -1076,7 +1076,7 @@ MqttClient.prototype._handleAck = function (packet) {
  * @param {Object} packet
  * @api private
  */
-MqttClient.prototype._handlePubrel = function (packet, callback) {
+MqttClient.prototype._handlePubrel = function (packet, callback = function () {}) {
   var mid = packet.messageId
   var that = this
 
@@ -1087,11 +1087,11 @@ MqttClient.prototype._handlePubrel = function (packet, callback) {
       that.emit('message', pub.topic, pub.payload, pub)
       that.incomingStore.put(packet, function (err) {
         if (err) {
-          return callback && callback(err)
+          return callback(err)
         }
         that.handleMessage(pub, function (err) {
           if (err) {
-            return callback && callback(err)
+            return callback(err)
           }
           that._sendPacket(comp, callback)
         })
diff --git a/test/abstract_client.js b/test/abstract_client.js
@@ -973,6 +973,115 @@ module.exports = function (server, config) {
       }
     })
 
+    it('should handle error with async incoming store in QoS 2 `handlePublish` method', function (done) {
+      function AsyncStore () {
+        if (!(this instanceof AsyncStore)) {
+          return new AsyncStore()
+        }
+      }
+      AsyncStore.prototype.put = function (packet, cb) {
+        setTimeout(function () {
+          cb(new Error('Error'))
+        }, 200)
+      }
+      var store = new AsyncStore()
+      var client = connect({incomingStore: store})
+
+      client._handlePublish({
+        messageId: 1,
+        topic: 'test',
+        payload: 'test',
+        qos: 2
+      }, function () {
+        done()
+        client.end()
+      })
+    })
+
+    it('should handle error with async incoming store in QoS 2 `handlePubrel` method', function (done) {
+      function AsyncStore () {
+        if (!(this instanceof AsyncStore)) {
+          return new AsyncStore()
+        }
+      }
+      AsyncStore.prototype.put = function (packet, cb) {
+        setTimeout(function () {
+          cb(new Error('Error'))
+        }, 200)
+      }
+      AsyncStore.prototype.get = function (packet, cb) {
+        setTimeout(function () {
+          cb(null, {cmd: 'publish'})
+        }, 200)
+      }
+      var store = new AsyncStore()
+      var client = connect({incomingStore: store})
+
+      client._handlePubrel({
+        messageId: 1,
+        qos: 2
+      }, function () {
+        done()
+        client.end()
+      })
+    })
+
+    it('should handle success with async incoming store in QoS 2 `handlePubrel` method', function (done) {
+      var putComplete = false
+      function AsyncStore () {
+        if (!(this instanceof AsyncStore)) {
+          return new AsyncStore()
+        }
+      }
+      AsyncStore.prototype.put = function (packet, cb) {
+        setTimeout(function () {
+          putComplete = true
+          cb(null)
+        }, 200)
+      }
+      AsyncStore.prototype.get = function (packet, cb) {
+        setTimeout(function () {
+          cb(null, {cmd: 'publish'})
+        }, 200)
+      }
+      var store = new AsyncStore()
+      var client = connect({incomingStore: store})
+
+      client._handlePubrel({
+        messageId: 1,
+        qos: 2
+      }, function () {
+        putComplete.should.equal(true)
+        done()
+        client.end()
+      })
+    })
+
+    it('should handle error with async incoming store in QoS 1 `handlePublish` method', function (done) {
+      function AsyncStore () {
+        if (!(this instanceof AsyncStore)) {
+          return new AsyncStore()
+        }
+      }
+      AsyncStore.prototype.put = function (packet, cb) {
+        setTimeout(function () {
+          cb(null, 'Error')
+        }, 200)
+      }
+      var store = new AsyncStore()
+      var client = connect({incomingStore: store})
+
+      client._handlePublish({
+        messageId: 1,
+        topic: 'test',
+        payload: 'test',
+        qos: 1
+      }, function () {
+        done()
+        client.end()
+      })
+    })
+
     it('should not send a `pubcomp` if the execution of `handleMessage` fails for messages with QoS `2`', function (done) {
       var store = new Store()
       var client = connect({incomingStore: store})
