server side disconnect handling #926
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #926 +/- ##
==========================================
- Coverage 93.75% 93.57% -0.18%
==========================================
Files 8 8
Lines 912 919 +7
Branches 239 240 +1
==========================================
+ Hits 855 860 +5
- Misses 57 59 +2
Continue to review full report at Codecov.
|
|
Hm, what wrong? I added test. |
|
@mcollina, do you know what wrong? codcov speaks all new code strings covered by tests..I must write some tests more? |
|
I don't know, and I would not worry. |
imZack
added a commit
to imZack/MQTT.js
that referenced
this pull request
Feb 7, 2020
* Bumped v2.8.2. * Added checks for double subscription issue * Added test cases for duplicate subs * Updated dependencies. * bumped v2.9.0 * revert publish/subscribe * the default value must be set for an empty options parameter * changed const to var * use xtend instead of extend * Bumped v2.9.1. * Add basic compile/connect/sub/pub TypeScript test * Add @types/node * Fixes mqttjs#415 * Update dependencies. * Bumped v2.9.2. * removed prepublishOnly script * Fixes mqttjs#648 * Bumped v2.9.3. * Added removeOutgoingStore function. * Added removeOutgoingStore to the document. Removed `delByMessageId` and use `del`. * Fixed object literal. Renamed parameter name to `mid` from `messageId`. * Changed the API name from `removeOutgoingStore` to `removeOutgoingMessage`. * Called outgoing callback with Error('Message removed') if the message is removed. * Bumped v2.10.0. * add support for weapp * Removed jshint comment * Removed tests for wx from test/mqtt.js * Bumped 2.11.0. * add .npmrc Prevent npm from creating a package-lock.json file on install. * Add a flag to control re-subscribe functionality. * Add resubscribe type. * Remove resubscribe topics if suback error is received. * Bumped v2.12.0 * Enable handleMessage backpressure support for QoS 2 flows * allow to publish binary message * Bumped v2.12.1. * Use test.mosquitto.org for typescript tests * Tests for handleMessage under all QoS flows * Fix code style complaints * Bumped 2.13.0. * update eslint to avoid a deprecated option ```space-after-keywords``` has actually been deprecated. This new configuration uses new option. * Fixed 3 test failures caused by improper use of should.js assertion API to check for an empty array * Added an additional assertion to check if the returned result is an Array * Improved QoS 1 message handling in a way that the pubacks are sent only if the handleMessage and other message handlers are executed successfully * Removed obsolete option 'autoAcknowledge', which was committed mistakenly as part of the default option list * remove eslint * Fixed flaky tests * Improved 'puback' handling first proposed in the PR mqttjs#697 as per the feedback received and added unit tests corresponding to the aforesaid improvement * Ignore path when connect as MQTTS. * Add unit tests. 'should validate successfully the CA using URI with path' fails if this PR is not applied. * Removed useless .jscsrc and .jshintrc * Bumped v2.13.1. * Allowed not to send a 'pubcomp' when 'handleMessage' method throws an error, and added test cases to test the functionality * Refactored all instances that previously used 'assert' to use 'should' * Fix mqttjs#706. Add `reconnect()` function. Clear incomingStore and outgoingStore only if `clean` is true. * Fixed disconnected and disconnecting status management. * Replace `pubrel` with `pubcomp`. In QoS2 sequence, when the client receives `publish`, the messege is stored into `incomingStore` and sends pack `pubrec`. Then the server sends `pubrel` to the client. Finally, the client calls `handleMessage` with stored message. If no errors are hannpened, `pubcomp` will be sent, but any errors are happened, `pubcomp` shouldn't be sent. This test is for the error case, so I replaced the description. * Removed conditional `Store#close()` calling from `mqtt.Client#end()`. Added `options` that contains `clean` to `mqtt.Store()`. If `clean` is set, `_inflights` is not cleaned when `mqtt.Store#close()` is called. * Used xtend for default store options. * Fixed flaky test, updated deps * added node 9 to .travis.yml * Bumped v2.14.0 * Rewrote the test-case that handles transactionality of QoS 2 messages… (mqttjs#712) * Rewrote the test-case that handles transactionality of QoS 2 messages when the execution of 'handleMessage' fails to avoid potentially flaky results * Fixed issues reported by JSLint * Improved handling of callbacks in an event where handler fails * Improved the way resources are cleaned up after using them * Add mqtt-localforage-store link * add support to allow empty string clientId * add tests * close all mqtt client for test cases * Formatting corrections in README.md Correct some minor markdown formatting problems in the README file. * fix a bug that you can not reconnect on wechat * change close order * fix travis code style checking * Updated tls client example Added protocol ssl. Without protocol parameter thos example doesn't work. * Fixed resource management on manual reconnect. Store._inflights set to {} instead of null to prepare manual reconnect. Deferred the timing of manual reconnect calling if reconnect() called during disconnecting process. NOTE: reconnect() function usually called from 'close' or 'error' handler. * Updated deferring condition. Defer reconnecting only the cae if during disconnecting but not disconnected yet. Added test for NOT deferring reconnect(). * Added '_' prefix to deferredReconnect. Renew `Store` on reconnect instead of setting Store._inflights to `{}` on `Store.close()`. * Removed constructor call. Added `reconnect()` restriction to documents. Updated `reconnect()` tests. They are for `clean: false`. Added `connect()` again tests. They are for `clean: true`. * Added `opts` parameter to `reconnect()`. That includes `incomingStore` and `outgoingStore` similar to `connect()`. If stores passed, `reconnect()` uses them, otherwise renew default `Store`. * Updated `reconnect()` typescript documents. Updated `reconnect()` JavaScript comment. * Fixed mqttjs#735. Added Store to index.js. * Added test for Store creation via mqtt. * Updated test description. * Updated dependencies and more stable tests * Do not stack overflow if a TCP frame contains too many PUBLISH * added safe-buffer as a dev dependency * Bumped v2.15.0 * Publish returns an error in the callback when store.put fails * Extra bracket removed after conflicts in merge request * Remove unnecessary semicolons * Remove unnecessary semicolons * Bumped v2.15.1. * update mocha to v4.x - uses `--exit` to hack around non-exiting process * fix issue while ending a disconnected client * removed --growl option from mocha * Bumped v2.15.2 * typo correction in README.md On line 49, I replaced 'id' by 'is'... that's it... * add non regression test and fix issue * Bumped 2.15.3. * end event added and tested * end event documentation * updated dependencies * Bumped v2.16.0. * Force callbacks when pingresp not received or when end called with true. * Bumped dependencies * Bumped v2.17.0. * Use protocol from server list if available. * Add typescript definition for protocol in servers. * Fixed: weapp url should not set port, remove port from weapp url builder * Fixed: weapp url should not set port, remove port from weapp url builder * Fixed: weapp's real device environment can only send data of type String or ArrayBuffer * Update: weapp url add port number when not 80 or 443 * Use mqtt-packet typings. * Updated dependencies * Bumped v2.18.0 * mqtt 5 init * fixes, tests, docs * docs * update packages * remove document in weixin env * remove useless urlModule * revert version * Bumped v2.18.1. * fixes by review, merged new version * tests * Setup default protocol when not defined in servers. Modifty unit test. * Bumped v2.18.2. * Fix typo Wexin -> Weixin * merge with master, custom reasonCodes * revert update packages * revert test * extend test for just check connection * Solves issue mqttjs#810: 1: Set minimum of this.nextId initialisation to 1: ``` this.nextId = Math.max(1, Math.floor(Math.random() * 65535)) ``` 2. Missing slot 65536 is not the case, the previous function was the following: ``` var id = this.nextId++ // Ensure 16 bit unsigned int: if (id === 65535) { this.nextId = 1 } return id ``` Which results into the following behaviour: 1. nextId is 65535 2. id becomes 65535 3. nextId becomes 65536 4. if(id === 65535) sets nextId to 1 while nextId is already 65536 It would be different with var id = ++this.nextId. Nevertheless, it's hard to read and not very explicit, id is a copy of nextId while nextId is changed which is not really clear (devil is here definitively in the detail) therefore I added a merge request to change the implementation to with the purpose of easier readability: ``` var id = this.nextId++ if(this.nextId === 65536) { this.nextId = 1 } return id ``` * Fixed invalid protocol definition in TLS client example. * Updated dev dependencies * Bumped v2.18.3 * fixes by review * fix type in removeOutgoingMessage(mid) doc * Improved example with subscribe callback. I have setup a small mqtt cluster with 2 nodes on Kubernetes. If I try to run your example I sometime miss the `Hello mqtt` message, and the sample app just keep running. If I change the example by moving the publish inside the callback I always get the message and the app correctly close. Can this be caused by the fact that the publish doesn't wait the subscribe to be sucesfully? Maybe I connect to another node of the cluster or some concurrency issue? Can you confirm that it is better to wait for the subscriber before sending the message? I know that this is probably a not real world scenario, but... * Fixed resend functionality. Even if reconnectPeriod is 0, client should resend publish/purel packet when clean is false. So I removed `that.options.reconnectPeriod > 0` guard condition from the connect handler. Added resend from pubrel functionality. In the following scenario, resend message should be pubrel. 1. Client connects to the broker with clean session false. 2. Client publishes QoS 2 message. 3. Broker receives the message and send pubrec to the client. 4. Client receives the pubrec message and send pubrel to the broker. 5. Client disconnects from the broker. 6. Client reconnects to the broker. 7. Client receives connack from the broker. 8. Client should resend pubrel automatically. When `_sendPacket()` is called, if the packet type is `pubrel` then call `storeAndSend()` instead of `send`acket()`. Added tests for them. * Bumped v2.18.4. * Fixed mqttjs#854. Added preserving publish order functionality to `Store` using `es6-map`. `es6-map` can preserve insertion order. If target environment is ES6, `es6-map` is native ES6 Map, otherwise use fallback implementation that has the same interface as ES6 Map. Updated some tests that depends on Store's internal structure (_inflights). Added test to check publish order when reconnects. * Added the comment why es6-map is needed. * Fixed tests on Mac OS X * Removed nsp * Bumped v2.18.5. * Proper use of `'readable'` event. * destroy the ongoing stream in case of a disconnect * Bumped v2.18.6. * Added packet type (cmd) for existing replace test for store. Removed duplicated test. * Bumped v2.18.7 * updated version * Added error handling to imcomingStore.put callback in _handlePublish. Added asynchronous result callback function and error handling in _handlePubrel similar to _handlePublish. * Notice about MQTT 5.0 added to README * Refined null callback handling. Added unit test. * Fixed the way to provide default empty function. Default paramter is introduced since ES6. Replaced the way that supports older ES, * Replaced setTimeout() with process.nextTick() * Replaced function () {} with nop * bumped v2.18.8 * Delayed emit timing of `connect` event. Emit `connect` event after processing of `outgoingStore` is completed. Problem: When client publish new message during proccessing of publish/pubrel messages in `outgoingStore`, the newly published message interrupts the process. As a result, messages are not sent by published time order. Outcomes: Above message order problem is fixed by this commit. Added unit test verifies this case. * Modified `_onConnect()` as top level function. Called `this._setupPingTimer()` from `_onConnect()`. Modified `on('connect')` handler for resubscribe as top level function. The function is called from `_onConnect()`. * Passed packet to `_onConnect()`. * Change `this.firstConnection` to private member. * Added Node 10 to .travis.yml * update branch * Add new callback called when message is put into `outgoingStore`. Added new callback `cbStorePut` to `publish()`. `cbStorePut` is called when message is put into `outgoingStore`. Problem: When disconnection occures right after `publish()` but `callback` is not called, then reconnect, client can't know if the message is completely stored into `outgoingStore`. Outcome: This commit fixes above problem. Client can know that message has been put into `outgoingStore` when `cbStorePut` is called. * Pass callback `cbStorePut` to `publish` method as one of options. * Update TypeScript declaration files for `cbStorePut`. * fix properties mqtt 5 in subscribe * fix bug in weixin min program and add support to ali min program (mqttjs#898) * add support for Ali Mini Program * remove test for wx and ali Mini Program * refactor lib/connect/ali.js * rewrite wx.js * fix wx.js * remove unuse console, and fix connect/index.js for alis * revert ws.js * fix ali.js * remove weapp-test script in package.json * fix README.md * fix ali.js to adapt IDE * fix: delete completed incoming QOS 2 messages (mqttjs#893) * fix: delete completed incoming QOS 2 messages * delay deleting incoming QOS 2 messages from store until PUBCOMP has been sent * verify handshake order of QOS 2 * ensure QOS 2 message is only handled once, when sending pubcomp fails and multiple pubrel are received * add test to check incoming store is empty after QOS 2 handshake * Update deps fix ci fix 9errors (mqttjs#903) * Updated some dependencies, removed old node versions from .travis.yml * Fix mqttjs#901 errors. Replace echo stream close implementation. * Fix publish interrupt during stored packets processing. (mqttjs#902) * Fix publish interrupt during stored packets processing. If `publish()` is called during stored packets processing, store the packet id into `packetIdsDuringStoreProcessing` kvs. The key is packet id and the value is boolean (true: prossesed, false: not processed). At this time, the value is false. If the packet is actually sent, the value is set to true. When stream process reaches the end, check all of `packetIdsDuringStoreProcessing` are processed, if it doens't, try again flowing process from the beginning. In this process, already processed (but not removed yet because puback is not receieved) packets should be skipped. In order to do that, check `packetIdsDuringStoreProcessing` value. If it is true, skip it. * Add '_' prefix to internal properties. * Refactoring to increase codecov. * fix bug in weapp (mqttjs#913) * add support for Ali Mini Program * remove test for wx and ali Mini Program * refactor lib/connect/ali.js * rewrite wx.js * fix wx.js * remove unuse console, and fix connect/index.js for alis * revert ws.js * fix ali.js * remove weapp-test script in package.json * fix README.md * fix ali.js to adapt IDE * fix ali.js and wx.js * fix wx.js * fix: change let to var * Did you mean 'Support'? (mqttjs#915) * fix (mqttjs#917) * server side disconnect handling (mqttjs#926) * perform nextTick work only if work needs to be done (mqttjs#931) * perform nextTick work only if work needs to be done, call done() immediately if not. * check if done() is valid * resubscribe mqtt5 fix (mqttjs#946) * resubscribe mqtt5 fix * for loop rebase * process disconnect packet w/o full destroy the connection (mqttjs#937) * process disconnect packet w/o full destroy the connection * Update README.md Co-Authored-By: Matteo Collina <matteo.collina@gmail.com> * Fixed mqttjs#952. (mqttjs#953) * Fixed mqttjs#952. Conditional flush `outgoing` on close. Scenario: 1. The client connect to the server. 2. The client sends subscribe to the server. 3. The server destroys the client connection before suback sending. 4. The client detect `close` event, then reconnects to the server. At the step4, `outgoing` still stored the callback for subscribe. However, it has never called because server doen't send corresponding suback. The same thing happens on unsubscribe. So I defined subscribe/unsubscribe as volatile. The volatile type of `outgoing` entries should be cleared when `close` from the server is detected. On the contrary, QoS1 and QoS2 publish is not volatile. Because they are resent after reconnection. And then, callback in the `store` is called. This behavior shouldn't be changed. So I added `volatile` flag to `outgoing` element. * Fixed cb accessing code. If `outgoing[mid]` doesn't match, then accessing `outgoing[mid].cb` causes `Cannot read property` error. So added checking code. Fixed outgoing assignment at `_onConnect` function. * Bumped v3.0.0 * fix: set default servername in tls connect (mqttjs#954) * fix: set default servername in tls connect * fix: unit tests for tls servername option * Revert "fix: set default servername in tls connect (mqttjs#954)" This reverts commit 6eab2ef. * Correct "nedbb" typo in README.md * docs: minor style improvements * Update README.md * Update README.md Co-authored-by: Matteo Collina <matteo.collina@gmail.com> Co-authored-by: Gavin D'mello <dmellogavin5000@gmail.com> Co-authored-by: Yohei Onishi <yohei@perxtech.com> Co-authored-by: Nicholas Dudfield <ndudfield@gmail.com> Co-authored-by: Takatoshi Kondo <redboltz@gmail.com> Co-authored-by: taoqf <tao_qiufeng@126.com> Co-authored-by: Brandon Smith <brandon@16cards.com> Co-authored-by: Miao Wang <shankerwangmiao@users.noreply.github.com> Co-authored-by: Iman Tabrizian <tabrizian@outlook.com> Co-authored-by: Prabath Abeysekara <prabathabeysekara@gmail.com> Co-authored-by: Vinícius Borriello <vinicius.bo@gmail.com> Co-authored-by: TechnicalSoup <ben.werbowyj@gmail.com> Co-authored-by: zarej <zarej@svrljig.net> Co-authored-by: Divya Venkataramani <v.divya110@gmail.com> Co-authored-by: Christopher Hiller <boneskull@boneskull.com> Co-authored-by: gautaz <gautaz@users.noreply.github.com> Co-authored-by: Donatien <donatienloret@gmail.com> Co-authored-by: Weston Catron <westonsoccer2000@gmail.com> Co-authored-by: 三点 <zousandian@gmail.com> Co-authored-by: Rodrigo Saboya <saboya@users.noreply.github.com> Co-authored-by: scarry1992 <scarry92@yandex.ru> Co-authored-by: yingye <imyingye@gmail.com> Co-authored-by: Cbdy <cbdyzj@jianzhao.org> Co-authored-by: num-lock <num.lock@gmx.net> Co-authored-by: Amin Ahmed Khan <aminahmedkhan@gmail.com> Co-authored-by: Davide Icardi <davide.icardi@gmail.com> Co-authored-by: Masaaki Fujiwara <40011629+ogis-fujiwara@users.noreply.github.com> Co-authored-by: SyMind <dacongsama@live.com> Co-authored-by: Joe Lynch <joe.b.lynch@gmail.com> Co-authored-by: 0xflotus <0xflotus@gmail.com> Co-authored-by: Simon Vogl <svogl@users.noreply.github.com> Co-authored-by: Dara Hayes <dara.hayes@redhat.com> Co-authored-by: Evan Summers <evan.summers@vocovo.com> Co-authored-by: Yoseph Maguire <yoseph.maguire@gmail.com> Co-authored-by: Behnam Mohammadi <itten@live.com>
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Disconnect MQTT 5.0. Can be sent from server side.