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

StaleElementReferenceException with Wait* keywords #475

Closed
aaltat opened this issue Jul 30, 2015 · 50 comments
Closed

StaleElementReferenceException with Wait* keywords #475

aaltat opened this issue Jul 30, 2015 · 50 comments

Comments

@aaltat
Copy link
Contributor

aaltat commented Jul 30, 2015

With Chrome 44.0.2403, Chrome driver 2.16 Selenium2Library 1.7.2 and selenium 2.46.1 when I use Wait* keywords, the StaleElementReferenceException might occur. Documentation says that keyword should only fail if condition is not met before the timeout, example for Wait Until Page Contains Element keyword, documentation says: Fails if timeout expires before the element appears.

But if StaleElementReferenceException happens, then I see this:

Selenium2Library.Wait Until Element Is Visible xpath=//span/span[text()="${menu_item}"], timeout=5s, error=Menu item ${menu_item} not found on page!
Documentation:  
Waits until element specified with `locator` is visible.
Start / End / Elapsed:  20150730 14:24:49.623 / 20150730 14:24:50.240 / 00:00:00.617
00:00:00.376KEYWORD: common_library_imports.Selenium2library Common Failure
14:24:49.624    TRACE   Arguments: [ u'xpath=//span/span[text()="XXXXXX"]' | timeout=u'5s' | error=u'Menu item XXXXXX not found on page!' ]   
14:24:49.624    DEBUG   POST http://127.0.0.1:55614/session/7f76eac90d05725647286d5bdf089b4a/elements {"using": "xpath", "sessionId": "7f76eac90d05725647286d5bdf089b4a", "value": "//span/span[text()=\"XXXXXX\"]"}   
14:24:49.829    DEBUG   Finished Request    
14:24:49.831    DEBUG   GET http://127.0.0.1:55614/session/7f76eac90d05725647286d5bdf089b4a/element/0.4323001531884074-3/displayed {"sessionId": "7f76eac90d05725647286d5bdf089b4a", "id": "0.4323001531884074-3"}  
14:24:49.855    DEBUG   Finished Request    
14:24:50.239    FAIL    StaleElementReferenceException: Message: stale element reference: element is not attached to the page document
  (Session info: chrome=44.0.2403.125)
  (Driver info: chromedriver=2.14.313457 (3d645c400edf2e2c500566c9aa096063e707c9cf),platform=Windows NT 6.1 SP1 x86_64)
14:24:50.239    DEBUG   Traceback (most recent call last):
  File "<string>", line 2, in wait_until_element_is_visible
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\keywordgroup.py", line 15, in _run_on_failure_decorator
    return method(*args, **kwargs)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_waiting.py", line 128, in wait_until_element_is_visible
    self._wait_until_no_error(timeout, check_visibility)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_waiting.py", line 237, in _wait_until_no_error
    timeout_error = wait_func(*args)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_waiting.py", line 121, in check_visibility
    visible = self._is_visible(locator)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_element.py", line 723, in _is_visible
    return element.is_displayed()
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webelement.py", line 323, in is_displayed
    return self._execute(Command.IS_ELEMENT_DISPLAYED)['value']
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webelement.py", line 402, in _execute
    return self._parent.execute(command, params)
  File "C:\Python27\lib\site-packages\Selenium2Library\webdrivermonkeypatches.py", line 11, in execute
    result = self._base_execute(driver_command, params)
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 175, in execute
    self.error_handler.check_response(response)
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\errorhandler.py", line 166, in check_response
    raise exception_class(message, screen, stacktrace)  
14:24:49.621    TRACE   Arguments: [ ${menu_item}=u'XXXXXX' | ${timeout}=u'10' ]

and keyword did fail immediately. But I did expect that keyword would simply re-try search for the element if the StaleElementReferenceException or any other selenium related error happens before the timeout.

@HelioGuilherme66
Copy link
Member

Please verify your ChromeDriver version, from the Trace we see:
(Session info: chrome=44.0.2403.125)
(Driver info: chromedriver=2.14.313457 (3d645c400edf2e2c500566c9aa0960

On Thu, Jul 30, 2015 at 1:58 PM, Tatu Aalto notifications@github.com
wrote:

With Chrome 44.0.2403, Chrome driver 2.16 Selenium2Library 1.7.2 and
selenium 2.46.1 when I use Wait* keywords, the
StaleElementReferenceException might occur. Documentation says that keyword
should only fail if condition is not met before the timeout, example for
Wait Until Page Contains Element keyword, documentation says: Fails if
timeout expires before the element appears.

But if StaleElementReferenceException happens, then I see this:

Selenium2Library.Wait Until Element Is Visible xpath=//span/span[text()="${menu_item}"], timeout=5s, error=Menu item ${menu_item} not found on page!
Documentation:
Waits until element specified with locator is visible.
Start / End / Elapsed: 20150730 14:24:49.623 / 20150730 14:24:50.240 / 00:00:00.617
00:00:00.376KEYWORD: common_library_imports.Selenium2library Common Failure
14:24:49.624 TRACE Arguments: [ u'xpath=//span/span[text()="XXXXXX"]' | timeout=u'5s' | error=u'Menu item XXXXXX not found on page!' ]
14:24:49.624 DEBUG POST http://127.0.0.1:55614/session/7f76eac90d05725647286d5bdf089b4a/elements {"using": "xpath", "sessionId": "7f76eac90d05725647286d5bdf089b4a", "value": "//span/span[text()="XXXXXX"]"}
14:24:49.829 DEBUG Finished Request
14:24:49.831 DEBUG GET http://127.0.0.1:55614/session/7f76eac90d05725647286d5bdf089b4a/element/0.4323001531884074-3/displayed {"sessionId": "7f76eac90d05725647286d5bdf089b4a", "id": "0.4323001531884074-3"}
14:24:49.855 DEBUG Finished Request
14:24:50.239 FAIL StaleElementReferenceException: Message: stale element reference: element is not attached to the page document
(Session info: chrome=44.0.2403.125)
(Driver info: chromedriver=2.14.313457 (3d645c400edf2e2c500566c9aa096063e707c9cf),platform=Windows NT 6.1 SP1 x86_64)
14:24:50.239 DEBUG Traceback (most recent call last):
File "", line 2, in wait_until_element_is_visible
File "C:\Python27\lib\site-packages\Selenium2Library\keywords\keywordgroup.py", line 15, in _run_on_failure_decorator
return method(_args, *_kwargs)
File "C:\Python27\lib\site-packages\Selenium2Library\keywords_waiting.py", line 128, in wait_until_element_is_visible
self._wait_until_no_error(timeout, check_visibility)
File "C:\Python27\lib\site-packages\Selenium2Library\keywords_waiting.py", line 237, in _wait_until_no_error
timeout_error = wait_func(*args)
File "C:\Python27\lib\site-packages\Selenium2Library\keywords_waiting.py", line 121, in check_visibility
visible = self._is_visible(locator)
File "C:\Python27\lib\site-packages\Selenium2Library\keywords_element.py", line 723, in _is_visible
return element.is_displayed()
File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webelement.py", line 323, in is_displayed
return self._execute(Command.IS_ELEMENT_DISPLAYED)['value']
File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webelement.py", line 402, in _execute
return self._parent.execute(command, params)
File "C:\Python27\lib\site-packages\Selenium2Library\webdrivermonkeypatches.py", line 11, in execute
result = self._base_execute(driver_command, params)
File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 175, in execute
self.error_handler.check_response(response)
File "C:\Python27\lib\site-packages\selenium\webdriver\remote\errorhandler.py", line 166, in check_response
raise exception_class(message, screen, stacktrace)
14:24:49.621 TRACE Arguments: [ ${menu_item}=u'XXXXXX' | ${timeout}=u'10' ]

and keyword did fail immediately. But I did expect that keyword would
simply re-try search for the element if the StaleElementReferenceException
or any other selenium related error happens before the timeout.


Reply to this email directly or view it on GitHub
#475.

@aaltat
Copy link
Contributor Author

aaltat commented Jul 31, 2015

Ah, did accidentally copy from wrong log file. Here is the correct example:

KEYWORD: Selenium2Library.Wait Until Element Is Visible ${sponsor_selector}, timeout=15, error=Sponsor table is not visible
Documentation:  
Waits until element specified with `locator` is visible.
Start / End / Elapsed:  20150728 12:28:26.380 / 20150728 12:28:26.884 / 00:00:00.504
00:00:00.221KEYWORD: common_library_imports.Selenium2library Common Failure
12:28:26.381    TRACE   Arguments: [ u"xpath=//b[contains(text(),'YYYYY')]" | timeout=u'15' | error=u'Sponsor table is not visible' ]    
12:28:26.382    DEBUG   POST http://127.0.0.1:52328/session/1966b838b325c06df0e2070139bd9cb0/elements {"using": "xpath", "sessionId": "1966b838b325c06df0e2070139bd9cb0", "value": "//b[contains(text(),'YYYYY')]"}  
12:28:26.647    DEBUG   Finished Request    
12:28:26.648    DEBUG   GET http://127.0.0.1:52328/session/1966b838b325c06df0e2070139bd9cb0/element/0.9413065502885729-1/displayed {"sessionId": "1966b838b325c06df0e2070139bd9cb0", "id": "0.9413065502885729-1"}  
12:28:26.659    DEBUG   Finished Request    
12:28:26.883    FAIL    StaleElementReferenceException: Message: stale element reference: element is not attached to the page document
  (Session info: chrome=44.0.2403.107)
  (Driver info: chromedriver=2.16.333243 (0bfa1d3575fc1044244f21ddb82bf870944ef961),platform=Windows NT 6.1 SP1 x86_64)
12:28:26.883    DEBUG   Traceback (most recent call last):
  File "<string>", line 2, in wait_until_element_is_visible
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\keywordgroup.py", line 15, in _run_on_failure_decorator
    return method(*args, **kwargs)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_waiting.py", line 128, in wait_until_element_is_visible
    self._wait_until_no_error(timeout, check_visibility)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_waiting.py", line 237, in _wait_until_no_error
    timeout_error = wait_func(*args)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_waiting.py", line 121, in check_visibility
    visible = self._is_visible(locator)
  File "C:\Python27\lib\site-packages\Selenium2Library\keywords\_element.py", line 723, in _is_visible
    return element.is_displayed()
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webelement.py", line 326, in is_displayed
    return self._execute(Command.IS_ELEMENT_DISPLAYED)['value']
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webelement.py", line 447, in _execute
    return self._parent.execute(command, params)
  File "C:\Python27\lib\site-packages\Selenium2Library\webdrivermonkeypatches.py", line 11, in execute
    result = self._base_execute(driver_command, params)
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\webdriver.py", line 193, in execute
    self.error_handler.check_response(response)
  File "C:\Python27\lib\site-packages\selenium\webdriver\remote\errorhandler.py", line 181, in check_response
    raise exception_class(message, screen, stacktrace)

@aaltat
Copy link
Contributor Author

aaltat commented Jul 31, 2015

I was reading the code and I do not quite understand, where the other selenium errors (like NoSuchElementException) are suppressed, but I was thinking could this resolve the problem: https://github.com/aaltat/robotframework-selenium2library/commit/9df1e8e132c63c67c771d9b0f45582fc4b35ca6c

Just a raise a discussion and me to figure out, am I in the right track?

@zephraph
Copy link
Member

zephraph commented Aug 1, 2015

Hey @emanlove, can you help me take a look at this? I know there were some similar issues floating around when you were first implementing using web elements as locators.

@aaltat, I'm actually on vacation right now. As soon as I get home I'll try to help you resolve this.

@aaltat
Copy link
Contributor Author

aaltat commented Aug 1, 2015

My idea is not actually good, because it wont fix the other Wait* keywords. Therefore finding a better solution is needed. In my case the StaleElementReferenceException happen most often with Wait Until Element Is Visible keyword, because I use it also quite often.

@zephraph I returned from my holidays this week, so enjoy yours. Shutting down the email (+other social media apps) usually helps :-)

@emanlove
Copy link
Member

emanlove commented Aug 2, 2015

Actually @aaltat I was thinking something similar, although I would have first tried catching the error up within wait_until_element_is_visible. Not saying it should be here instead; it's just where I would have tried it first.

I am not sure it is proper to exclude this error in all the Wait* keywords. I think one needs to ask what is a proper error and maybe what is not. This was part of the discussion we had in the users group. Under some of the Wait* keywords I could see that a StaleElementReferenceException is an error that would/should cause an error and in others it could be ignored till the timeout period has completed.

@emanlove
Copy link
Member

emanlove commented Aug 2, 2015

[Thinking out loud...] What if _waiting._wait_until_no_error() had an additional parameter for errors to ignore...?

@aaltat
Copy link
Contributor Author

aaltat commented Aug 2, 2015

@emanlove Well, sounds a good idea to me...

@emanlove
Copy link
Member

emanlove commented Aug 3, 2015

I am wandering if there is a better method for waiting. Looking at some of the methods within @rickpc's angular extended library I see he usage of the webdriver wait (which by the way has a argument for ignoring specified errors).

@aaltat
Copy link
Contributor Author

aaltat commented Aug 3, 2015

Those methods do look similar as what the selenium API now days recommends to use with explicit waits: http://selenium-python.readthedocs.org/en/latest/waits.html?highlight=waiting#explicit-waits And there is ready made conditions that would suite in many of our needs (just an assumption), like:visibility_of_element_located. But I do not know the codebase that well to estimate, how big change that would be and would it require to raise the minimum required selenium version.

@emanlove
Copy link
Member

emanlove commented Aug 3, 2015

Concerning the minimum required selenium version, I see several points in the CHANGES log,

Selenium 2.45.0
* Pass info to TimeoutException in WebDriverWait

# --- We currently require Selenium 2.32 or greater ---#

Selenium 2.26
* Added new expected_conditions support module to be used with WebDriverWait

Selenium 2.20
* fix webdriverwait to execute at least once when using 0 timeout

Selenium 2.4
* Added WebDriverWait as a support package

I think we can get by with what we currently require. The change in 2.45.0 looks to just add debuging information on a TimeoutException.

@aaltat
Copy link
Contributor Author

aaltat commented Aug 3, 2015

If we can get by using current selenium version, which way you would like to proceed?

  1. Use existing S2L wait logic and just do some sort of try/ expect on required places in Wait* keywords
  2. Change the internal S2L waits to use selenium waits

I can think many pros/cons for each way.

@zephraph
Copy link
Member

zephraph commented Aug 3, 2015

Just my two cents, but I'd prefer getting as close to the webdriver as possible, so I'd vote 2. Granted, that may come with a greater risk than option 1.

@aaltat
Copy link
Contributor Author

aaltat commented Aug 3, 2015

If I could freely choose, I would go also for option 2, Also from the same reason which @zephraph mentions. Also I think, it could make the code much easier to read and maintain. At least for me the waiting/finding logic in S2L was not so easy to read and understand. Of course the option 2, is lot more work and my gut feeling says that there are many things which will cause a pain in...

Of course the option 1 would be relatively easy to do, but somehow I feel that the maintenance burden would grow with option 1.

@emanlove
Copy link
Member

emanlove commented Aug 3, 2015

I too think that 2 is the better option. We should prototype something up and see what the difference is. I think also a good explanation of what the wait is doing all the way through would be good to have.

@aaltat
Copy link
Contributor Author

aaltat commented Aug 4, 2015

OK, I will try to muster up some sort of prototype, perhaps I get something done at the end of the week. But I was thinking that this means the separation between the Waiting* and other keywords in means how the element is located. Other keywords would still use the existing functionality to locate/interact with elements but Waiting* keywords would be moved closer to the selenium 2 world. Just trying to limit the scope, because I do not want to do rewrite for the whole library...

@aaltat
Copy link
Contributor Author

aaltat commented Aug 5, 2015

@emanlove @zephraph It seems @rtomac did give me Collaborator access, is there some sort dev channel where you ppl discuss and decide things or how this works?

@zephraph
Copy link
Member

zephraph commented Aug 5, 2015

Well, I created the slack channel and am always available through there. @emanlove, you should definitely give it a go too if you haven't already.

@HelioGuilherme66
Copy link
Member

There is an instant messaging site with nice features like file posting and
channels creation, public or private:
https://robotframework.slack.com/

You will have to create an account, after that you may request access at
https://robotframework-slack.herokuapp.com/ (status button on github's RF
projects)

On Wed, Aug 5, 2015 at 7:50 PM, Tatu Aalto notifications@github.com wrote:

@emanlove https://github.com/emanlove @zephraph
https://github.com/zephraph It seems @rtomac https://github.com/rtomac
did give me Collaborator access, is there some sort dev channel where you
ppl discuss and decide things or how this works?


Reply to this email directly or view it on GitHub
#475 (comment)
.

@aaltat
Copy link
Contributor Author

aaltat commented Aug 24, 2015

I finally found some time to prototype this.

--- a/src/Selenium2Library/keywords/_waiting.py
+++ b/src/Selenium2Library/keywords/_waiting.py
@@ -1,11 +1,22 @@
 import time
 import robot
 from keywordgroup import KeywordGroup
+from selenium.webdriver.common.by import By
+from selenium.webdriver.support.ui import WebDriverWait
+from selenium.webdriver.support import expected_conditions as EC
+

 class _WaitingKeywords(KeywordGroup):

     # Public

+    def __init__(self):
+        wd_starategies = {
+            'id': By.ID,
+            'xpath': By.XPATH
+        }
+        self._wd_strategies = wd_starategies
+
     def wait_for_condition(self, condition, timeout=None, error=None):
         """Waits until the given `condition` is true or `timeout` expires.

@@ -81,7 +92,7 @@ class _WaitingKeywords(KeywordGroup):
         """
         if not error:
             error = "Element '%s' did not appear in <TIMEOUT>" % locator
-        self._wait_until(timeout, error, self._is_element_present, locator)
+        self._wd_wait_untill(locator, timeout)

     def wait_until_page_does_not_contain_element(self, locator, timeout=None, error=None):
         """Waits until element specified with `locator` disappears from current page.
@@ -243,3 +254,22 @@ class _WaitingKeywords(KeywordGroup):
     def _format_timeout(self, timeout):
         timeout = robot.utils.timestr_to_secs(timeout) if timeout is not None else self._timeout_in_secs
         return robot.utils.secs_to_timestr(timeout)
+
+    def _wd_wait_untill(self, locator, timeout):
+        timeout = robot.utils.timestr_to_secs(timeout) if timeout is not None \
+            else self._timeout_in_secs
+        prefix, criteria = self._parse_locator(locator)
+        print prefix, criteria
+        wd_by = self._wd_strategies[prefix]
+        WebDriverWait(self._current_browser(), timeout).\
+            until(EC.presence_of_element_located((wd_by, criteria)))
+
+    def _parse_locator(self, locator):
+        prefix = None
+        criteria = locator
+        if not locator.startswith('//'):
+            locator_parts = locator.partition('=')
+            if len(locator_parts[1]) > 0:
+                prefix = locator_parts[0]
+                criteria = locator_parts[2].strip()
+        return (prefix, criteria)

The above could perhaps work, but I did found some issues.

  1. The selenium By class only supports:
    ID = "id"
    XPATH = "xpath"
    LINK_TEXT = "link text"
    PARTIAL_LINK_TEXT = "partial link text"
    NAME = "name"
    TAG_NAME = "tag name"
    CLASS_NAME = "class name"
    CSS_SELECTOR = "css selector"

and our implementation is much wider. I would need extend the By class someway, but first I need to read, what our all strategies actually do and how they do it.

  1. Would need to consider should the ElementFinder internal _parse_locator function should be public.Now I did just copy paste it.

  2. I need to find way to support custom error message.

@aaltat
Copy link
Contributor Author

aaltat commented Oct 23, 2015

I have been thinking this issue and more I think it more reluctant I have come to change the existing keyword functionality. It feels like a big rewrite and those things don't usually end well.

So could we find an another way to implement the webdriver native waiting? Could we create duplicate keyword, like: Wait Until Page Contains 2 or something to implement this functionality in smaller and more controlled commits. Or any good idea how to introduce the change in the library in controlled manner?

@pekkaklarck
Copy link
Member

I don't like the idea of having Wait Until Page Contains 2 in the library, especially if it works like Wait Until Page Contains but is more reliably. If your idea is to have it as a temporary keyword until corner cases are resolved, I think it would be better to work in a branch.

@aaltat
Copy link
Contributor Author

aaltat commented Oct 23, 2015

I also do not like the idea having two keywords, with same functionality. Nor I do not like the idea to make new arguments to define which way the search is made. I can not think a good way to change the functionality, without doing a quite a big re-write.

Perhaps best way would to write the new underlying functionality to separate classes and not to aim for re-write. Perhaps even do one keyword at the time.

@manycoding
Copy link

I'm experiencing the same problem with Firefox.
Firefox 41.0.2
Robot Framework 2.9.1
robotframework-selenium2library (1.7.4)
selenium (2.47.1)

Selenium2Library . Wait Until Element Is Visible //div[contains(@id, "table_grid_TreeGrid")]//table[@Class = "gridxRowTable"]

StaleElementReferenceException: Message: Element not found in the cache - perhaps the page has changed since it was looked up
Stacktrace:
at fxdriver.cache.getElementAt (resource://fxdriver/modules/web-element-cache.js:9348)
at Utils.getElementAt (file:///tmp/tmp89g1qk/webdriver-py-profilecopy/extensions/fxdriver@googlecode.com/components/command-processor.js:8942)
at WebElement.isElementDisplayed (file:///tmp/tmp89g1qk/webdriver-py-profilecopy/extensions/fxdriver@googlecode.com/components/command-processor.js:12168)
at DelayedCommand.prototype.executeInternal_/h (file:///tmp/tmp89g1qk/webdriver-py-profilecopy/extensions/fxdriver@googlecode.com/components/command-processor.js:12643)
at fxdriver.Timer.prototype.setTimeout/<.notify (file:///tmp/tmp89g1qk/webdriver-py-profilecopy/extensions/fxdriver@googlecode.com/components/command-processor.js:623)

@ombre42
Copy link
Contributor

ombre42 commented Nov 18, 2015

This issue is similar to #173.
I worked around the issue years ago by ignoring certain exceptions while waiting - see #173 for code.
In most projects, we are ignoring InvalidSelectorException, StaleElementReferenceException, and NoSuchElementException during the Wait * keywords.
My workaround was to modify S2L by monkey patching in a wait that ignores exceptions (configurable via a keyword).
If needed, the ignored exceptions could be temporarily changed because Set Ignored Wait Exceptions returns the old ignored exceptions. It turned out never to be needed. As used, the ignored exceptions are set after opening the browser and never touched.
Ignored exceptions could be by type or by type and also with a message matching a regex.
The patch is probably a bit over-engineered, but I haven't had to touch it since I wrote it :)

I would like to know in what waits it is not appropriate to ignore certain exceptions where in others it would be. Wait For Condition does not need to ignore exceptions like StaleElementReferenceException, but they are not going to occur anyways, so no harm done in my opinion.

@bartkl
Copy link

bartkl commented Jan 31, 2018

I was just wondering if this is a decent enough patch for now, until the problem is solved more elegantly:

***************
*** 17,24 ****
--- 17,25 ----
  import time

  from SeleniumLibrary.base import LibraryComponent, keyword
  from SeleniumLibrary.errors import ElementNotFound
+ from selenium.common.exceptions import StaleElementReferenceException
  from SeleniumLibrary.utils import is_noney, is_truthy, secs_to_timestr


  class WaitingKeywords(LibraryComponent):
***************
*** 227,234 ****
--- 228,238 ----
                  if condition():
                      return
              except ElementNotFound as err:
                  not_found = str(err)
+             except StaleElementReferenceException:
+                 continue
              else:
                  not_found = None
              time.sleep(0.2)
          raise AssertionError(not_found or error)

Or am I missing a reason why this is a terrible idea?

[Edit]: As requested by @emanlove here's a little more context of the code snippet:

    def _wait_until_worker(self, condition, timeout, error):
        max_time = time.time() + timeout
        not_found = None
        while time.time() < max_time:
            try:
                if condition():
                    return
            except ElementNotFound as err:
                not_found = str(err)
            except StaleElementReferenceException:
                continue
            else:
                not_found = None
            time.sleep(0.2)
        raise AssertionError(not_found or error)

(This is in the waiting.py file)

@emanlove
Copy link
Member

It's been a long while since I have reviewed this issue and I see this thread has gotten really long. That said, in general, it seems to me like a bad idea to generically and blindly handle state element exceptions within the core library. I suspect different users will have different needs, desires, and method to handle when it comes to stale elements.

@bartkl Can you show a little more or add a link to here this by line in the code? I'd just like to see a few lines above this..

@pekkaklarck
Copy link
Member

pekkaklarck commented Jan 31, 2018

At some point I thought it would be great if SL could always handle stale elements automatically, but nowadays I tend to agree with @boakley who generally considers them bugs in the application that should not be silenced. With Wait ... keywords that also gracefully handle elements missing altogether the situation is somewhat different, though, and I feel it would be OK to do the change proposed by @bartkl. I don't know the domain well enough to have a strong opinion about this, though, and would be fine @aaltat making the decision one way or the other.

@emanlove and others interested to see where the proposed change would be can go to https://github.com/robotframework/SeleniumLibrary/blob/master/src/SeleniumLibrary/keywords/waiting.py#L222

@boakley
Copy link
Contributor

boakley commented Jan 31, 2018 via email

@aaltat
Copy link
Contributor Author

aaltat commented Feb 1, 2018

I agree that for this problem, finding a solution that would suite for all is impossible. But I think that what inexperienced users are doing, using WUKS or sleep, is not good solution either. If one is working with a application, that has a clear state, like an loading indicator in the UI, then suppressing StaleElementReferenceException is not right course of action. But I have also worked with applications where knowing the state was next to impossible and I really hate those kind of legacy applications. But for those type of legacy applications, just retrying is mostly best course of action.

But if we blindly allow users to suppress the error, then we deny them the change to improve and learn. Failing provides best way to learn. In my opinion, blindly applying this for all Wait... keywords, even with a toggle, is wrong and we should not do it. But is there a other solution which we could provide for the users?

@bartkl
Copy link

bartkl commented Feb 1, 2018

It's good to be wary when editing core functionality like this, and I agree there should be a good consideration as to what possible use cases and scenarios are involved.

In my (limited) experience I have encountered exclusively situations where it is desirable for the Wait keywords to catch the StaleElementReferenceException and fetch a fresh element from the DOM. Of course, I could be wrong, but I can't really imagine an counterexample either. Does someone else maybe know a reasonable use case where the current behavior is desired?

@aaltat I agree with your statement in general, but -and correct me if I'm wrong- I feel that in the case of elements going stale during Wait keywords there's not really a decent solution to handle it. If during a wait an element is changed (to some state other than the expected end state), it raises the error and as far as I know there's nothing the enduser can do. I could use Execute JavaScript return document.GetElementById("element-id").innerHTML == "expected value" (for example), skipping the staleness problem entirely, but this is not very user-friendly and elegant.

All in all, I guess I agree with @pekkaklarck that it seems (like him and possibly even more so, I'd like to be careful) the Wait keywords are a notable exception to the general caution of not editing at the core.

@pekkaklarck
Copy link
Member

The reason I think Wait ... keywords could ignore the stale element errors is that they already ignore errors related to elements not existing in the first place. In practice these keywords execute conditions like self.find_element(locator).is_visible(). At the moment it's fine if find_element() fails due to element not existing, but it's a failure if element exists but is invalidated before calling is_visible().

In my opinion element not existing and element being invalid are so close to each others that they should be handled the same way. They both cause a failure with normal keywords, and I see no harm in both of them being ignored with wait keywords. If I've understood it correctly, now wait keyword passing or failing may depend on timing.

As I wrote above, I think these two error situations should be handled the same way. That could be easily and explicitly done like this:

    def _wait_until_worker(self, condition, timeout, default_error):
        max_time = time.time() + timeout
        error = None
        while time.time() < max_time:
            try:
                if condition():
                    return
            except (ElementNotFound, StaleElementReferenceException) as err:
                error = str(err)
            else:
                error = None
            time.sleep(0.2)
        raise AssertionError(error or default_error)

PS: That str(err) should actually be unicode(err) to avoid non-ASCII errors with Python 2.

@pekkaklarck
Copy link
Member

An obvious benefit of wait keywords handling state element errors like proposed above is that they could be used to workaround stale element problems. As @aaltat commented, handling them "correctly" with legacy applications can be really hard.

@petr-muller
Copy link

User's perspective: I've just spent a year in a shop dealing with apps written using a framework which was basically a stale element exception generator (SAPUI5, may you burn in hell). We used the SL monkeypatched to do exactly what Pekka posted above, and it was totally fine for us. We somehow knew the behavior is sort-of wrong but we were not able to fix the framework that caused the problems (different team, big corp). As you say, especially with Wait keywords it's really a nuisance because when you are waiting on some transition, you kinda expect funky things to happen in the meantime in the DOM.

@aaltat
Copy link
Contributor Author

aaltat commented Feb 1, 2018

Perhaps we could implement the change for Wait... keywords, to handle the this problem as similarly as when element is not in the DOM. To get this merged to the master, we need:

  1. Document the change in the library documentation
  2. Write test for the change. Bulk of the should go to the unit level. It would be great to see at least one test in the acceptance level. I agree understand that it might be darn difficult and cause lot of timing issues.
  3. No toggle for the feature.

Anyone who would like to do the above, specially the help on point two is predicated.

@aaltat aaltat added this to the v3.2.0 milestone Feb 15, 2018
@aaltat aaltat modified the milestones: v3.2.0, v3.3.0 Mar 6, 2018
@mterzo
Copy link

mterzo commented Aug 24, 2018

This doesn't just occur on waits. I applied the above patch that handles the wait, but still have a problem when I was pulling data out of a table cell and got the same error.

The web application I'm testing is an angular based. I have a huge amount of data that I'm filtering using two-way data binding. As I'm inputting search terms, javascript is fltering this data and displaying it in a table. If I find that what I'm looking for is in the table, I then move forward trying to get a value out of a cell. The table could still be re-written on me giving this same error.

I've worked with @Byteme8199 on this, he's been doing the angular work for the application, and if there's any questions on on the details of the javascript he should be able to help.

@aaltat
Copy link
Contributor Author

aaltat commented Aug 27, 2018

The error can happen with any keyword which interacts with an element. I have been thinking about the problem and for me it feels that writing test to solve the problem is actually quite difficult in the acceptance level. Because of how browsers, timing and life. Therefore this problem is easier to solve in the unit test level. Mocking element and deciding what element does is easier in the unit level.

@aaltat aaltat modified the milestones: v3.2.0, v3.3.0 Sep 13, 2018
@bergstenarn
Copy link

Holding my thumbs that a fix for this long-runner makes it into v3.3.0. That would be awesome. Thanks @aaltat and everybody else for your dedicated work with RF.

@aaltat
Copy link
Contributor Author

aaltat commented Dec 23, 2018

Closing issue because it is replaced by #1270

@aaltat aaltat closed this as completed Dec 23, 2018
@aaltat aaltat removed this from the v3.3.0 milestone Dec 23, 2018
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