Use a nonce to guarantee message responses#7
Closed
jhobz wants to merge 4 commits intosatanch:mainfrom
Closed
Conversation
This is to coincide with a change to the LiveSplit.Server repo
See LiveSplit/LiveSplit.Server#39 for more details
At very fast polling intervals, the socket will sometimes send more than one message from the server in a single 'data' event. This change gracefully handles these edge cases, resulting in successful tests at as fast as polling every 5ms.
LiveSplit.Server is not guaranteed to respond to messages in the same order they are received. This change uses the "nonce" field in the new JSON messaging standard to provide a unique identifier against which to match incoming and outgoing messages. Currently, `this._openRequests` is not actually used for anything. But it does provide a useful method for investigating errors or commands that never receive a response. It also may be desired to change the listening behavior of this library to use a single listener that looks up the appropriate Promise resolver in `this._openRequests` instead of the current behavior -- subscribing a new listener for every message.
Owner
|
This PR is pretty outdated. Let's coma back to this issue when server changes would be applied. |
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
LiveSplit.Server is not guaranteed to respond to messages in the same order they are received. This change uses the "nonce" field in the new JSON messaging standard to provide a unique identifier against which to match incoming and outgoing messages.
Currently, the class member
this._openRequestsadded by this change is not actually used for anything. But it does provide a useful method for investigating errors or commands that never receive a response.It also may be desired in the future to change the listening behavior of this library to use a single listener that looks up the appropriate Promise resolver in
this._openRequestsinstead of the current behavior -- subscribing a new listener for every message. Another alternative would be to extendEventEmitter2instead ofEventEmitter, which would allow for namespacing events and subscribing listeners to only events emitted with their nonce. But as this would create a new dependency and be a larger change, I felt it made more sense to make the simpler change first.Note that this PR depends on #6.