Do not silently discard undelivered messages #34
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
The plugin no longer discards messages which it fails to send if the server disconnects and sends some content prior to the disconnection. Example situation before the fix: 1. A connection is established between a server and the plugin. 2. The plugin delivers some messages to the server. 2.1) It may or may not read something that it received from the server. 2.2) It sends the payload to the server. 3. The server sends some content back and disconnects. 4. The plugin tries to deliver some other messages to the server. 4.1) It tries to read (if r.any?) and expects to get an EOFError in case the server just died. 4.2) No error is thrown as some content just arrived from the server. It is discarded. 4.3) The first payload is written to the socket, no error being thrown. 4.4) The next payload finally fails with EOFError upon reading from the socket and an retry follows. This commit fixes the 4.1 and 4.2 phases. The reading is now performed repeatedly while there's still something to read left so that we don't miss the EOFError exception.
|
@jsvd, @ph, @danhermann is it possible to get an update on this? The v6.0.0 still silently drops the first message after a reconnect. |
|
💚 CLA has been signed |
jsmucr
force-pushed
the
patch-1
branch
4 times, most recently
from
9f09957
to
7246be9
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in 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.
The plugin no longer discards messages which it fails to send if the server disconnects and sends some content prior to the disconnection.
Example situation before the fix:
2.1) It may or may not read something that it received from the server.
2.2) It sends the payload to the server.
4.1) It tries to read (if r.any?) and expects to get an EOFError in case the server just died.
4.2) No error is thrown as some content just arrived from the server. It is discarded.
4.3) The first payload is written to the socket, no error being thrown.
4.4) The next payload finally fails with EOFError upon reading from the socket and an retry follows.
This commit fixes the 4.1 and 4.2 phases. The reading is now performed repeatedly while there's still something to read left so that we don't miss the EOFError exception.