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

WebElementCondition not instanceof Condition #5560

Closed
forrest-rival opened this issue Mar 2, 2018 · 25 comments
Closed

WebElementCondition not instanceof Condition #5560

forrest-rival opened this issue Mar 2, 2018 · 25 comments
Assignees
Labels

Comments

@forrest-rival
Copy link

Meta -

OS:
OSX 10.13.3

Selenium Version:
selenium-webdriver@4.0.0-alpha.1

Browser:
Chrome

Browser Version:
Version 64.0.3282.186 (Official Build) (64-bit)

Expected Behavior -

driver.wait() should accept a WebElementCondition, i.e. driver.wait(until.elementLocated(By.css(selector)))

Actual Behavior -

the above example throws the error TypeError: Wait condition must be a promise-like object, function, or a Condition object

Steps to reproduce -

driver.wait(until.elementLocated(By.css(selector)))

This is some serious JavaScript weirdness:
Object.getPrototypeOf(condition) === WebElementCondition {}
yet condition instanceof WebElementCondition === false
weird, right?

I was able to get the case to pass by adding condition.constructor.name === 'WebElementCondition' to if (condition instanceof Condition) in the WebDriver's wait function.

@jleyba jleyba self-assigned this Mar 2, 2018
@jleyba
Copy link
Contributor

jleyba commented Mar 3, 2018

Huh...I can't reproduce this. Any chance you're using selenium-webdriver along with another package that might add some version skew?

@forrest-rival
Copy link
Author

What's the best way to verify this? I don't see any sign of another version in my dependency lock file.

@ptrhvns
Copy link

ptrhvns commented Mar 9, 2018

I'm also getting this error. I just started a node project, so my package.json is still small (I've tried with node 8.10.0 too):

{
  "engines" : {
    "node" : "9.8.0"
  },
  "devDependencies": {
    "babel-jest": "^22.4.1",
    "babel-preset-env": "^1.6.1",
    "jest": "^22.4.2",
    "prettier": "^1.11.1",
    "selenium-webdriver": "^4.0.0-alpha.1"
  },
  "babel": {
    "presets": [
      "env"
    ]
  },
  "jest": {
    "transform": {
      "^.+\\.jsx?$": "babel-jest"
    }
  }
}

My test is doing this at the time of the error:

  // ...
  await webdriver.get(BASE_URL);
  await webdriver.wait(until.elementLocated(By.css('body')));
  // ...

@jleyba
Copy link
Contributor

jleyba commented Mar 9, 2018

Are you transpiling with babel? I wonder if that's mangling the class hierarchy

@ptrhvns
Copy link

ptrhvns commented Mar 9, 2018

@jleyba, yes, I am transpiling, and I was wondering if that's causing the problem too. However, the line the error was coming from (node_modules/selenium-webdriver/lib/webdriver.js:809:13 in selenium-webdriver 4.0.0-alpha.1) is testing that the condition passed in (i.e. the return value of until.elementLocated(By.css('body')) in my code) has an fn property. It's hard to imagine that babel would be eliminating a property on an object, but I could be wrong.

By the way, I worked-around the problem by passing in my own async function to the wait function instead of the WebElementCondition returned by until.elementLocated(By.css('body')):

await webdriver.wait(async driver => await driver.findElement(By.css('body')));

@forrest-rival
Copy link
Author

@tekhne additional context; since the WebElementCondition isn't being recognized as an instanceof Condition, fn doesn't get assigned a function, fails the next block, and throws the error you're seeing

@ptrhvns
Copy link

ptrhvns commented Mar 9, 2018

@forrest-rival oh okay. I think I see that now. Thanks. I guess that points back towards Babel, I guess?

@jleyba
Copy link
Contributor

jleyba commented Mar 9, 2018

I guess we don't need to be so strict and could change this:

    let fn = /** @type {!Function} */(condition);
    if (condition instanceof Condition) {
      message = message || condition.description();
      fn = condition.fn;
    }

to something like

let fn = condition;
if (condition
    && typeof condition.description === 'function'
    && typeof condition.fn === 'function') {
  message = message || (condition.description() + '')
  fn = condition.fn;
}

There's a second instanceof check that probably isn't working either (but isn't an issue if you await the result of wait()):

https://github.com/SeleniumHQ/selenium/blob/master/javascript/node/selenium-webdriver/lib/webdriver.js#L845

@forrest-rival
Copy link
Author

@jleyba instead of changing the code in wait(), maybe consider adding a custom hasInstance static method to the Condition and WebCondition classes.

static [Symbol.hasInstance](condition) {
  return condition
    && typeof condition.description === 'string'
    && typeof condition.fn === 'function';
}

instanceof will then use this static method to compare an instance to that class and no changes would be necessary in wait()

@forrest-rival
Copy link
Author

@jleyba bumping since you probably don't need the R-awaiting answer label anymore

@nobody-famous
Copy link

I'm seeing the same behavior with different weirdness. I see it depending on whether one of my require calls is relative or absolute, i.e. require('./dir/file.js') gives the wait condition error but require('c:/path/to/dir/file.js') works fine.

I started with webdriver 4.0.0-alpha.1, then downgraded to 3.6.0 and still see the problem.

I'm running on windows with firefox and node version 8.11.1.

If it would help, I could zip up my files that are seeing the issue and upload those.

@afsmith92
Copy link
Contributor

@forrest-rival are you planning on creating a PR if @jleyba is okay with your proposal? I'm happy to help with this issue if needed.

@forrest-rival
Copy link
Author

@afsmith92 I wasn't planning on it, but gladly will if it helps. I believe the snippet I posted is all that's required here.

afsmith92 added a commit to afsmith92/selenium that referenced this issue May 29, 2018
Update WebElementCondition and Condition with custom
static hasInstance methods so that `instanceof` checks
are more lenient and don't cause issues with Babel.

Fixes SeleniumHQ#5560
@afsmith92
Copy link
Contributor

@forrest-rival I went ahead and created a PR. Thank you for digging into this and finding a fix.

afsmith92 added a commit to afsmith92/selenium that referenced this issue May 31, 2018
Update WebElementCondition and Condition with custom
static hasInstance methods so that `instanceof` checks
are more lenient and don't cause issues with Babel.

Fixes SeleniumHQ#5560
jleyba pushed a commit that referenced this issue Jun 1, 2018
* Loosen WebElementCondition instance checks

Update WebElementCondition and Condition with custom
static hasInstance methods so that `instanceof` checks
are more lenient and don't cause issues with Babel.

Fixes #5560
@geoffreyyip
Copy link

Hey guys, when can I expect to see this change live in the npm package? I noticed the published version was 7 months ago

@ekucks21
Copy link

ekucks21 commented Sep 7, 2018

In my case, the issue seemed to be caused by npm bringing in two versions of selenium-webdriver, one directly under node_modules/ and one under node_modules/selenium-cucumber-js/node_modules. The ugly workaround for now is to have a postinstall that removes the one under selenium-cucumber-js.

@tienwei
Copy link

tienwei commented Nov 5, 2018

If anyone stumbled on the issue, my colleague came up a workaround.
You need to define a condition and then pass an async function in driver.wait. Take an example as follows:

const condition = until.elementLocated(By.name('loader'))
driver.wait(async driver => condition.fn(driver), 10000, 'Loading failed.')

Hope this helps.

@CarlosUrbina
Copy link

If anyone stumbled on the issue, my colleague came up a workaround.
You need to define a condition and then pass an async function in driver.wait. Take an example as follows:

const condition = until.elementLocated(By.name('loader'))
driver.wait(async driver => condition.fn(driver), 10000, 'Loading failed.')

Hope this helps.

Thanks! This helped a lot.

@smartamus
Copy link

When I run my scrape locally, everything works. When I upload it as a package to an AWS Lambda function, with the same version (4.0.0 alpha), it produces this error. Any idea why locally it would work fine, but on lamdba, it will trigger this?

barancev added a commit that referenced this issue Jan 30, 2019
This reverts commit 5ec8094.

This commit was intended to fix a babel-related issue #5560.

Unfortunately it broke 'x instanceof WebElementCondition' check
that returns true even if 'x' is an instance of the 'Condition' class
(a base class for 'WebElementCondition'). All waits that return
a boolean value or a string are affected.
@barancev
Copy link
Member

I've reverted the change proposed in PR #5968 because it broke x instanceof WebElementCondition check that returns true even if x is an instance of the Condition class (a base class for WebElementCondition). All waits that return a boolean value or a string are affected.

Looking for a better solution...

@barancev barancev reopened this Jan 30, 2019
shs96c pushed a commit to shs96c/selenium that referenced this issue Feb 18, 2019
This reverts commit 5ec8094.

This commit was intended to fix a babel-related issue SeleniumHQ#5560.

Unfortunately it broke 'x instanceof WebElementCondition' check
that returns true even if 'x' is an instance of the 'Condition' class
(a base class for 'WebElementCondition'). All waits that return
a boolean value or a string are affected.
@whyman
Copy link

whyman commented Sep 2, 2019

Do we have any plan on what the next steps are now the change has been backed out?

This feels like a pretty critical thing to fix -- I'm happy to take a stab at putting something together if we can agree an approach

@Elephant-Vessel
Copy link

Elephant-Vessel commented Sep 5, 2019

I guess this is explaining the problem: nodejs/node#13408

This maybe is triggered when the npm index.js is importing both until and webdriver, while until is requiring webdriver simultaneously? Just a guess.

@dillonbarker-te
Copy link

Is there an update on this, I am still seeing the issue in rc-2.

@titusfortner
Copy link
Member

Unable to reproduce in latest Selenium version, no further issues reported.

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests