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.

Sign up for GitHub

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

Fix callback error reporting #821

Merged
merged 6 commits into from
Jul 15, 2019
Merged

Fix callback error reporting #821

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9a98038
fix #791: don't try to use Component.keys()
alexcjohnson Jul 13, 2019
740f8ce
convert "replace" to "dedent" in multiline error messages
alexcjohnson Jul 13, 2019
aa4eac5
changelog for callback error reporting fix
alexcjohnson Jul 13, 2019
1cb18fa
add a 3x retry to browser starting
alexcjohnson Jul 13, 2019
782af07
black test_integration
alexcjohnson Jul 15, 2019
fbec9dc
test nonexistent id with multi-output
alexcjohnson Jul 15, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 17 additions & 11 deletions dash/testing/browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ def __exit__(self, exc_type, exc_val, traceback):
self.driver.quit()
self.percy_runner.finalize_build()
except WebDriverException:
logger.exception("webdriver quit was not successfully")
logger.exception("webdriver quit was not successful")
except percy.errors.Error:
logger.exception("percy runner failed to finalize properly")

Expand Down Expand Up @@ -247,16 +247,22 @@ def open_new_tab(self, url=None):
)

def get_webdriver(self, remote):
return (
getattr(self, "_get_{}".format(self._browser))()
if remote is None
else webdriver.Remote(
command_executor=remote,
desired_capabilities=getattr(
DesiredCapabilities, self._browser.upper()
),
)
)
# occasionally the browser fails to start - give it 3 tries
for i in reversed(range(3)):
try:
return (
getattr(self, "_get_{}".format(self._browser))()
if remote is None
else webdriver.Remote(
command_executor=remote,
desired_capabilities=getattr(
DesiredCapabilities, self._browser.upper()
),
)
)
except WebDriverException:
if not i:
raise
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@byronz I had 3 failures this morning - all from different tests, but with the same message:

WebDriverException: Message: unknown error:
Chrome failed to start: exited abnormally
(unknown error: DevToolsActivePort file doesn't exist)

It's a bit icky to add a blind retry loop like this, but it seemed to work.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had an impression this crash comes always from one specific python version, like 3.7, or 2.7. Might be good to figure out the real root cause, but assuming it's just turbulence from the circleci docker. it's more related to bad timing due to load performance of the selenium server. should we try another approach like the wait.until? with a reasonable small wait time like 0.1s, I believe the next try should work (we can give a more tolerant 1s timeout).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alexcjohnson I created another PR to fix the deprecated message with pytest.raises, and I formatted the file with black. so if you can format this test_integration file in this PR, we could avoid a lot potential merge conflicts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw this in both 2.7 and 3.7. Feel free to dig in more (preferably in another PR so we can get this bugfix merged), but it's not clear to me what we could wait for, seems like this is happening deep inside Selenium.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

agree, we can leave it now in this PR.


def _get_wd_options(self):
options = (
Expand Down