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

Check for StaleElementReferenceException #396

Merged
merged 1 commit into from
Aug 27, 2018

Conversation

iffy
Copy link
Contributor

@iffy iffy commented Apr 29, 2015

I don't know how to test this -- especially since I can't duplicate the problem all the time. I just know that the monkey-patched version I am running has not yet raised this error and continues to work.

I'm happy to make modifications (including adding tests) with some guidance.

@andrewsmedina
Copy link
Member

@iffy this exceptions is raised when the page is modified. Why ignore it?

@iffy
Copy link
Contributor Author

iffy commented Apr 30, 2015

@andrewsmedina I don't know? :)

Here's my experience: I was using is_text_present to determine if a certain page had loaded. It would sometimes fail with StaleElementReferenceException but not always. Without this patch, I need to change this:

if not browser.is_text_present(text):
    # do something
    pass
else:
    # do something else
    pass

to something like this code, which pretty much duplicates the guts of is_text_present:

found = False
endtime = time.time() + browser.wait_time
while time.time() < endtime:
    try:
        if browser.is_text_present(text):
            found = True
            break
    except StaleElementReferenceException:
        pass
if found:
    # do something
    pass
else:
    # do something else
    pass

I'm not sure when I would want the StaleElementReferenceException to stop the is_text_present check (maybe other people have other use cases I can't think of).

Maybe a better solution would be to let the caller choose which exceptions to ignore? e.g.

browser.is_text_present(text, ignore_exceptions=(StaleElementReferenceException, SomeOtherException))

@jaraco
Copy link
Contributor

jaraco commented Apr 28, 2018

This pull request is superior to #450 as it addresses the issue both in is_text_present and is_text_not_present.

I have encountered this issue also and it occurs in a race condition when the find is called in one microsecond and the page loads in the next microsecond and then .text is called on that element which is no longer in the dom. This patch is the right one.

@jaraco
Copy link
Contributor

jaraco commented May 13, 2018

@andrewsmedina Can we expect a merge with this change anytime soon?

@jaraco
Copy link
Contributor

jaraco commented May 13, 2018

I tried creating a workaround by subclassing the browser object:

class Browser(splinter.Browser):
	workaround_396 = jaraco.functools.retry(
		cleanup=functools.partial(time.sleep, 1),
		trap=selenium.common.exceptions.StaleElementReferenceException,
		retries=2,
	)

	@workaround_396
	def is_text_present(self, *args, **kwargs):
		return super().is_text_present(*args, **kwargs)

	@workaround_396
	def is_text_not_present(self, *args, **kwargs):
		return super().is_text_not_present(*args, **kwargs)

But because Browser isn't a class (despite the signal in its casing), that fails with TypeError: function() argument 1 must be code, not str.

@jaraco
Copy link
Contributor

jaraco commented May 13, 2018

Ultimately, I ended up bypassing the splinter.Browser interface and instead patch the intended driver explicitly:

class Browser(splinter.browser.ChromeWebDriver):
	workaround_396 = jaraco.functools.retry(
		cleanup=functools.partial(time.sleep, 1),
		trap=selenium.common.exceptions.StaleElementReferenceException,
		retries=2,
	)

	@workaround_396
	def is_text_present(self, *args, **kwargs):
		return super().is_text_present(*args, **kwargs)

	@workaround_396
	def is_text_not_present(self, *args, **kwargs):
		return super().is_text_not_present(*args, **kwargs)

I'm not yet sure this approach works, but I'm hopeful it will.

@andrewsmedina andrewsmedina merged commit faef868 into cobrateam:master Aug 27, 2018
@andrewsmedina
Copy link
Member

@iffy thanks for contribute

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

Successfully merging this pull request may close these issues.

3 participants