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

aio: DISCONNECTED errors with Chrome + Karma + jasmine #17543

Closed
gkalpak opened this issue Jun 15, 2017 · 2 comments
Closed

aio: DISCONNECTED errors with Chrome + Karma + jasmine #17543

gkalpak opened this issue Jun 15, 2017 · 2 comments

Comments

@gkalpak
Copy link
Member

gkalpak commented Jun 15, 2017

Since upgrading to Chrome 59 on Travis, we started seeing unit test failures due to Chrome getting disconnected from Karma. We have worked around the issue with a39bf8b and 8af203c (which cause a slight slow-down).

(For reference, we are currently using Karma v1.7.0 and jasmine v2.6.3.)


I looked into it a bit more (since this is likely to affect many people given that our setup (jasmine+Karma+Chrome) is common) and here is what I believe is happening:

The Karma server uses socket.io to (among other things) get updates from the browser about the test results. Whenever the browser reports a test result, it refreshes the noActivityTimer, but if it doesn't hear from the browser for 10s it considers it disconnected.

For whatever reason, socket.io uses it's XHR-based polling implementation instead of WebSocket (but the problem would arise with WebSocket too). When the socket wants to emit a result, it will (after some hoops) call its flush() method, which will send the message, unless its Transport (i.e. the polling thingy) is currently not writable.

So, what causes the Transport to not be writable?

What this all means is that once the socket sends a message to the server, it has to wait for the XHR to complete before sending another message.

In the meantime, in Jasmine-land:
Jasmine is running the tests using its QueueRunner. Once it is gone through the whole queue (synchronously in our case), it will call its clearStack() method (and then continue running tests). The clearStack() method is what is passed to the QueueRunner, which (in a nutshell) is based on MessageChannel.

Let's tie everything together :)

Jasmine runs some tests. Clears the stack. The socket sends a message to inform the Karma server about the test result. Jasmine runs more tests. Etc.

Now, if everything ran synchronously, the readystatechange event listener for the polling socket doesn't get a chance to fire and set Transport.writable to true, in order to allow more messages to be sent to the Karma server. Our tests do run synchronously, but what about the clearStack() method? It is not, but...

It turns out that MessageChannel.postMessage() uses a task queue that gets processed before the one XHR events use. As a result, Jasmine will keep running tests and clearing the stack and running more tests, before the first XHR has a chance to complete. Thus, the socket's Transport will be non-writable all the time and Karma will not receive any updates from the browser.

Why didn't it fail before?

It seems that there were some changes in Chrome 58 or 59 wrt .postMessage() (these two seem related, but are also kind of old 😕 : 04055dd, 05a1330). I.e. previously it would use the Timer queue (what setTimeout uses), while now it uses a different queue that is processed before the Timer queue. Consider the following example:

function test() {
  // XHR.onload
  const xhr = new XMLHttpRequest();
  xhr.open('GET', location.href);
  xhr.onload = () => console.log('load');
  xhr.send();   // Assume it completes under 5s.

  // setTimeout
  setTimeout(() => console.log('timeout'));

  // requestAnimationFrame
  requestAnimationFrame(() => console.log('raf'));

  // MessageChannel
  const ch = new MessageChannel();
  const start = Date.now();
  ch.port1.onmessage = () => {
    console.log('channel-XHR.readyState:', xhr.readyState);
    for (let i = 0; i < 1000; ++i) {}
    if (Date.now() - start < 5000) ch.port2.postMessage('bar');   // Run for 5s.
  };
  ch.port2.postMessage('bar');

  // Promise
  Promise.resolve().then(() => console.log('promise'));
}

The output on Chrome 57 is different than on Chrome 59:

Chrome 57:

promise
raf
load
timeout
channel-XHR.readyState: 4 (many times)

Chrome 59:

promise
channel-XHR.readyState: 1 (many times)
raf
timeout
load

Why does beforeEach(done => done()) work?

By having an argument, the callback takes the async path. But because done() is called synchronously, it will be called before completedAsynchronously is set to false, which will in turn run the rest of the queued callbacks in a setTimeout(), giving the XHR time to complete.

@gkalpak gkalpak added this to TODO in docs-infra Jun 15, 2017
@gkalpak
Copy link
Member Author

gkalpak commented Jun 16, 2017

This should be fixed by jasmine/jasmine@c60d669. Closing, since this is not actionable on the Angular side anyway.

@gkalpak gkalpak closed this as completed Jun 16, 2017
@gkalpak gkalpak removed this from TODO in docs-infra Jun 16, 2017
gkalpak referenced this issue in jasmine/jasmine Jun 18, 2017
- Allows the CPU to run other things that used the real `setTimeout`

- Fixes #1327
- See #1334
- Fixes jasmine/gulp-jasmine-browser#48
gkalpak added a commit to gkalpak/angular that referenced this issue Jun 18, 2017
This version fixes the DISCONNECTED errors (described in angular#17543) and removes the
need to the workaround (8af203c).
The relevant jasmine commit is jasmine/jasmine@c60d66994.
gkalpak added a commit to gkalpak/angular that referenced this issue Jun 19, 2017
This version fixes the DISCONNECTED errors (described in angular#17543) and removes the
need to the workaround (8af203c).
The relevant jasmine commit is jasmine/jasmine@c60d66994.
hansl pushed a commit that referenced this issue Jun 19, 2017
This version fixes the DISCONNECTED errors (described in #17543) and removes the
need to the workaround (8af203c).
The relevant jasmine commit is jasmine/jasmine@c60d66994.
hansl pushed a commit that referenced this issue Jun 19, 2017
This version fixes the DISCONNECTED errors (described in #17543) and removes the
need to the workaround (8af203c).
The relevant jasmine commit is jasmine/jasmine@c60d66994.
asnowwolf pushed a commit to asnowwolf/angular that referenced this issue Aug 11, 2017
This version fixes the DISCONNECTED errors (described in angular#17543) and removes the
need to the workaround (8af203c).
The relevant jasmine commit is jasmine/jasmine@c60d66994.
juleskremer pushed a commit to juleskremer/angular that referenced this issue Aug 28, 2017
This version fixes the DISCONNECTED errors (described in angular#17543) and removes the
need to the workaround (8af203c).
The relevant jasmine commit is jasmine/jasmine@c60d66994.
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant