Added removeOutgoingStore function. #658
Conversation
Codecov Report
@@ Coverage Diff @@
## master #658 +/- ##
==========================================
+ Coverage 93.16% 93.22% +0.06%
==========================================
Files 8 8
Lines 658 664 +6
Branches 160 160
==========================================
+ Hits 613 619 +6
Misses 45 45
Continue to review full report at Codecov.
|
lib/store.js
Outdated
| Store.prototype.delByMessageId = function (messageId) { | ||
| delete this._inflights[messageId] | ||
| return this | ||
| } |
There was a problem hiding this comment.
This new method is not needed, you can use https://github.com/redboltz/MQTT.js/blob/c35e0a615b44bb235fecffb6b2ae8bdee4556c5a/lib/store.js#L74 and call it with { messageId }.
There was a problem hiding this comment.
If I use del, and matches messageId, undefined cb seems to be called.
Can I update del as follows?
Before
cb(null, packet)After
if (cb) {
cb(null, packet)
}There was a problem hiding this comment.
Thanks, I updated it.
There was a problem hiding this comment.
nodejs 7 and 8 reports an error on travis-ci. I updated { messageId } to { messageId: mid}, and changed the parameter name to mid.
|
Thank you for the comment.
Do you mean README.md? I will do that soon. |
|
Yes, |
Removed `delByMessageId` and use `del`.
|
I added README.md. |
redboltz
force-pushed
the
add_remove_outgoing_store
branch
from
eb9f7f6
to
99a4358
Compare
lib/client.js
Outdated
| * @example client.removeOutgoingStore(client.getLastMessageId()); | ||
| */ | ||
| MqttClient.prototype.removeOutgoingStore = function (messageId) { | ||
| delete this.outgoing[messageId] |
There was a problem hiding this comment.
maybe we should call the callback store in this object with an Error.
lib/client.js
Outdated
| * @example client.removeOutgoingStore(client.getLastMessageId()); | ||
| */ | ||
| MqttClient.prototype.removeOutgoingStore = function (mid) { | ||
| delete this.outgoing[mid] |
There was a problem hiding this comment.
Maybe we should call the outgoing callback (if exists) with an error?
There was a problem hiding this comment.
I agree. It is better to inform the client the result. I updated the code, test, and document.
lib/client.js
Outdated
| * | ||
| * @example client.removeOutgoingStore(client.getLastMessageId()); | ||
| */ | ||
| MqttClient.prototype.removeOutgoingStore = function (mid) { |
There was a problem hiding this comment.
the name could be a bit more meaningful, something like removeOutgoingMessage.
|
Thank you very much! |
I'd like to have removing a message from outgoing store functionality. I connect a MQTT broker using MQTT.js. The broker have permission control functionality. That means a client can publish only if the client have permission. The permission is defined a pair of ClientId (or Username) and topic.
Permission management is out of scope of MQTT spec, but there are some brokers that support it.
When I publish QoS1 message to a topic that is not permitted, then the broker doesn't return puback because the message is not processed. Some brokers immediately disconnect, but other brokers keep connection and report errors using some special topic like
$SYS/error_reporptby publish back.I'm using a latter type broker. After the client received the error, I'd like to remove in-flight publish message. Because it still consuming packet id. In addition, when the client disconnect and connect again, the client will resend the message, and will get the same error.
So I'd like to remove the message.
I implement the function and tests. If it has some problem, I will improve my pull request.