Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix rpc message order #311

Merged
merged 2 commits into from
Mar 14, 2023
Merged

Conversation

turuslan
Copy link
Contributor

Looks like original code expected subscription events to arrive earlier than subscription id, e.g.:

{subscription: 1, result: ...}
{subscription: 1}
{subscription: 1, result: ...}

But there were two problems:

  1. removing processed messages was skipping some messages
  2. always reading before checking queued messages delayed those messages processing

Signed-off-by: turuslan <turuslan.devbox@gmail.com>
Signed-off-by: turuslan <turuslan.devbox@gmail.com>
@arjanz
Copy link
Member

arjanz commented Jan 13, 2023

Thanks for the PR, I will have to run some tests to be sure that there are no unwanted side effects.

As you can imagine it was a bit of a struggle to make the async behavior of the websocket RPC play nice in a sync environment. Changes I already made where the introduction of the message queue and a13577e to fix race conditions between the subscription id and the first event of that subscription id.

There were sometimes still unexpected results though, so I'm looking forward to review this change, hoping to fix those issues.

@arjanz arjanz merged commit 4f5e425 into polkascan:master Mar 14, 2023
@turuslan turuslan deleted the fix/rpc-message-order branch March 14, 2023 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants