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

Buffering/batching client messages results in out of order events #3684

Open
jginsburgn opened this issue Jun 19, 2021 · 5 comments
Open

Buffering/batching client messages results in out of order events #3684

jginsburgn opened this issue Jun 19, 2021 · 5 comments
Assignees

Comments

@jginsburgn
Copy link
Member

jginsburgn commented Jun 19, 2021

c4ad697 introduced batching to overcome a Safari issue with xhr polling; newer Safari versions do not seem to reproduce this issue anymore.

However, this logic sometimes makes client event messages get to the server out of order. It seems to happen when socket.io upgrades from xhr polling to websockets.

@jginsburgn jginsburgn self-assigned this Jun 19, 2021
@devoto13
Copy link
Collaborator

Hmm, I was under impression that the out-of-order messages were fixed by #3212 and #3487. Are you able to reproduce it using the latest Karma?

Having said that I agree that we should remove this logic once we drop support for old browsers and consequently can solely use WebSockets as a transport protocol. See #3503 for some more ideas on future cleanups.

@jginsburgn
Copy link
Member Author

I tried to make an e2e scenario to reproduce this error without avail: jginsburgn@4d46e84. I am guessing that the error only shows in our infrastructure due to other elements. I was able to reproduce with v6.3.3.

@jginsburgn
Copy link
Member Author

I am investigating on my side to determine if there is another agent making this happen. Otherwise, removing the buffer logic should be the solution. But you say that we need to postpone it for the next major release of Karma? why? The affected Safari version is from 2013.

@devoto13
Copy link
Collaborator

devoto13 commented Jun 22, 2021

While it was initially introduced for Safari, it is also used by other browsers which don't support WebSockets (e.g. IE9-IE10). Maybe unnecessarily, but IMO this change has a moderate risk of introducing regressions in such browsers, which are hard to spot using our test suite. Therefore I'll be more comfortable doing it in the major release, preferably after dropping support for browsers, which don't support WebSockets.

@jginsburgn
Copy link
Member Author

I agree with your assessment!

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

No branches or pull requests

2 participants