karma icon indicating copy to clipboard operation
karma copied to clipboard

Buffering/batching client messages results in out of order events

Open jginsburgn opened this issue 3 years ago • 5 comments

https://github.com/karma-runner/karma/commit/c4ad69709103110a066ae1d9652af69e42434c6b 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 avatar Jun 19 '21 00:06 jginsburgn

Hmm, I was under impression that the out-of-order messages were fixed by https://github.com/karma-runner/karma/pull/3212 and https://github.com/karma-runner/karma/pull/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 https://github.com/karma-runner/karma/issues/3503 for some more ideas on future cleanups.

devoto13 avatar Jun 19 '21 13:06 devoto13

I tried to make an e2e scenario to reproduce this error without avail: https://github.com/jginsburgn/karma/commit/4d46e8423373e6f5d1ba56cf3645c2ca4834dda8. 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 avatar Jun 21 '21 19:06 jginsburgn

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.

jginsburgn avatar Jun 22 '21 00:06 jginsburgn

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.

devoto13 avatar Jun 22 '21 08:06 devoto13

I agree with your assessment!

jginsburgn avatar Jun 22 '21 15:06 jginsburgn