angular / protractor

E2E test framework for Angular apps
http://www.protractortest.org
MIT License
8.75k stars 2.31k forks source link

Moving to async/await while keeping filter raises 'Connection refused: connect' and 'ECONNREFUSED connect ECONNREFUSED 127.0.0.1:4444' #4706

Open dubzzz opened 6 years ago

dubzzz commented 6 years ago

Bug report

While trying to move away from the promise manager of Selenium, I encountered new random failures which were not occurring before. After analyze it appeared that it might be linked to the filter function offered by Protractor.

Here is an extract of the configuration and scripts I am using. All the code is available on https://github.com/dubzzz/protractor-move-async-await. The localhost:3000 exposes the todolist example of AngularJS official website with a list of a thousand entries.

Using the promise manager:

conf.js

exports.config = {
  seleniumAddress: 'http://localhost:4444/wd/hub',
  specs: ['todo-spec.js'],
  SELENIUM_PROMISE_MANAGER: 1
};

todo-spec.js

browser.get('http://localhost:3000/index.html');
element.all(by.repeater('todo in todoList.todos'))
  .filter(todo => todo.getText().then(label => label.indexOf('#10') !== -1))
  .each(todo => todo.element(by.css('input')).click());

Using async/await:

conf.js

exports.config = {
  seleniumAddress: 'http://localhost:4444/wd/hub',
  specs: ['todo-spec.js'],
  SELENIUM_PROMISE_MANAGER: 0
};

todo-spec.js

await browser.get('http://localhost:3000/index.html');
await element(by.model('todoList.todoText')).sendKeys('write first protractor test');
await element(by.css('[value="add"]')).click();
await element.all(by.repeater('todo in todoList.todos'))
  .filter(todo => todo.getText().then(label => label.indexOf('#10') !== -1))
  .each(todo => todo.element(by.css('input')).click());

The asynchronous version failed with the following error:

1) [async] angularjs homepage todo list should add a todo
  Message:
    Failed: java.net.ConnectException: Connection refused: connect

The full proof off concept is available on my personal Github at https://github.com/dubzzz/protractor-move-async-await

Is the bug coming from an issue in my configuration or is there a real issue?

Thanks in advance

IgorSasovets commented 6 years ago

Hi, @dubzzz ! I investigated this issue. Please take a look at my pull request https://github.com/dubzzz/protractor-move-async-await/pull/1/files. On my mind, tests became more stable. I'll investigate this deeper and then give you one more callback. Now for me it looks that problems are with usage of filter function.

dubzzz commented 6 years ago

@IgorSasovets indeed limiting the number of checkboxes makes it work but it was not my point.

My point is that control flow -based version took 27 seconds to run on my computer with no error at all. While async/await -based timeouts quite immediately (4-5 s) with java.net.ConnectException: Connection refused: connect.

By the way timeouts due to too long runs of protractor appear differently and refer to jasmine.

yannicklerestif commented 6 years ago

It might be a port exhaustion issue: a lot of requests to Selenium server are sent in a short time, each one requesting a new port, the number of which is limited (to 65536 or something like that).

The difference might be that filter sends all requests at once in the case of async /await promises whereas maybe the control flow version stacks them up and and then sends them one by one.

It's pretty easy to check using netstat or whatever port analysis tool you like.

dubzzz commented 6 years ago

Just tried to run a netstat command right after the run of the tests (either success or failure). I got barely the same number of lines TCP 127.0.0.1:4444 dev:49159 TIME_WAIT for both promise manager and async/await (~2.2k lines like this).

CrispusDH commented 6 years ago

You could override filter function to force send requests sequential.

 private makeFnSequential(func: any): any {
    let promise: any = Promise.resolve();

    return (...args) => {
      promise = promise.then(() => {
        return func(...args);
      });

      return promise;
    };
  }
}

use this method to wrap filterFn.

Second option is to run your tests directly on chrome (add to config directConnect: true). In that way selenium server will not use at all, so any java errors :)

CrispusDH commented 6 years ago

It also works incorrectly with all functions which have concurrent requests. Such as map, reduce, each, Promise.all

avient commented 6 years ago

Handled this issue using selenium-server-standalone-3.4.0

webdriver-manager update --versions.standalone=3.4.0

dubzzz commented 6 years ago

@CrispusDH thanks a lot for your answer

Indeed there is the exact same issue when using Promise.all or other Promise.* methods able to run requests in parallel (when called with too many promises).

Nonetheless, I am a bit surprised that this problem did not occur when using the non async/await version of the code. It looks very strange and unintuitive that switching the flag SELENIUM_PROMISE_MANAGER for async/await reveals such issue.

For the moment, I moved to sequential implementations of Promise. and .filter in order to avoid such issue. I will have a look to directConnect, it might be useful.

@avient Thanks for the information, I will give it a try ;)

dubzzz commented 6 years ago

@avient I still have the issue when forcing the use of 3.4.0 instead of my local version 3.11.0

Airpwn0 commented 6 years ago

Hi there, With directConnect: true the issue still persists - Error: ECONNREFUSED connect ECONNREFUSED As @CrispusDH described all these functions could mess flaky connection with parallel requests. Even in async/await way .map could hang up selenium easily. As a workaround when you are working with an array of elements do sync loop

async getTextFromElements(): Promise<string[]> {
        const arrayLocator: ElementArrayFinder = $$();
        const array = new Array();
        const allItems = await arrayLocator.asElementFinders_();
        for (const elem of allItems) {
            const elementText = await elem.getText();
            array.push(elementText);
        }
        return array;
    }

Dont forget to switch off SELENIUM_PROMISE_MANAGER

awarecan commented 6 years ago

That is an issue in selenium-webdriver. Here is my patch to resolve it (only work in protractor/selenium-webdriver 3.6.0)

var fs = require('fs');

var httpIndexFile = 'node_modules/selenium-webdriver/http/index.js';
fs.readFile(httpIndexFile, 'utf8', function (err, data) {
    if (err)
        throw err;

    var result = data.replace(/\(e.code === 'ECONNRESET'\)/g, "(e.code === 'ECONNRESET' || e.code === 'ECONNREFUSED')");
    console.log(`Patching ${httpIndexFile}`)
    fs.writeFileSync(httpIndexFile, result, 'utf8');
});
awarecan commented 6 years ago

Also you can try my another patch for chromeDriver persistence connection issue. I don't have problem for a while after apply both patch

var chromeFile = 'node_modules/selenium-webdriver/chrome.js';
fs.readFile(chromeFile, 'utf8', function (err, data) {
    if (err)
        throw err;

    var result = data.replace(/new http.HttpClient\(url\)/g, "new http.HttpClient(url, new (require('http').Agent)({ keepAlive: true }))");
    console.log(`Patching ${chromeFile}`)
    fs.writeFileSync(chromeFile, result, 'utf8');
});
kylecordes commented 6 years ago

@awarecan Thank you, I had been thinking of doing a similar monkey patch for the TCP connection persistence problem, and them thrilled to see this one line edit addresses both problems.

Sergeyatodh commented 6 years ago

@awarecan Thanks, your work around is pretty easy and more important it does what is supposed to

disophisis commented 5 years ago

@awarecan Has this workaround been integrated as a fix in any version of selenium webdriver? Or are there plans to do so?

awarecan commented 5 years ago

@disophisis the 2nd patch just got merged few days ago. Don't know when it will be released. No update for the first patch.

misakibiu commented 5 years ago

@dubzzz

Hi,

I have the same issue as you. May I ask how you implement the sequential .map or .filter. I tried for-loop, but I am still testing if it works. I am not sure if I implemented it correctly. It is very tricky.

Thanks.

andredesousa commented 3 years ago

Any update?

awarecan commented 3 years ago

@andredesousa no update and protractor is dying. My two patches worked very well in last few years.

You can copy my patches into a js file, for example, scripts/postinstall.js, then add following code block to your package.json

  "scripts": {
    "postinstall": "node scripts/postinstall.js",
  },
andredesousa commented 3 years ago

@awarecan Thank you. I had only used the last script, but both are required. My mistake.