Skip to content

Commit

Permalink
Improve detection of crashes and hangs with WebDriver.
Browse files Browse the repository at this point in the history
This makes crash report work with WebKitWebDriver:
 * When the WebKitWebProcess crashes, WebKitWebDriver will raise an
   InvalidSessionIdException <https://webkit.org/b/207565> like the
   Chrome WebDriver does. So we make WebDriverProtocol::is_alive()
   check for that exception.

It also fixes detecting when the WebDriver has hanged:
 * The call to is_alive() can hang if WebDriver itself has hanged
   or its unresponsive. If this call takes more than 5 seconds then
   the external timeout from the testrunner will be fired and testrunner
   will force a termination of the test marking it as external-timeout
   instead of crash.
 * The current logic tried to mark the test as crashing if socket
   timeout fired, but that timeout value was much higher than the
   grace period of 5 seconds, so this didn't worked.
 * We modify the socket timeout to 2 seconds, which should be more
   than enough to check if the WebDriver its still alive and allows
   to complete the check during the testrunner 5 seconds of
   extra_timeout.
  • Loading branch information
clopez authored and jgraham committed Feb 20, 2020
1 parent 1e5bdf2 commit 58988ab
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 6 deletions.
12 changes: 8 additions & 4 deletions tools/wptrunner/wptrunner/executors/base.py
Expand Up @@ -163,10 +163,14 @@ def run(self):
self.result = False, ("INTERNAL-ERROR", "%s.run_func didn't set a result" %
self.__class__.__name__)
else:
message = "Executor hit external timeout (this may indicate a hang)\n"
# get a traceback for the current stack of the executor thread
message += "".join(traceback.format_stack(sys._current_frames()[executor.ident]))
self.result = False, ("EXTERNAL-TIMEOUT", message)
if self.protocol.is_alive():
message = "Executor hit external timeout (this may indicate a hang)\n"
# get a traceback for the current stack of the executor thread
message += "".join(traceback.format_stack(sys._current_frames()[executor.ident]))
self.result = False, ("EXTERNAL-TIMEOUT", message)
else:
self.logger.info("Browser not responding, setting status to CRASH")
self.result = False, ("CRASH", None)
elif self.result[1] is None:
# We didn't get any data back from the test, so check if the
# browser is still responsive
Expand Down
12 changes: 10 additions & 2 deletions tools/wptrunner/wptrunner/executors/executorwebdriver.py
Expand Up @@ -301,11 +301,19 @@ def teardown(self):
self.webdriver = None

def is_alive(self):
socket_previous_timeout = socket.getdefaulttimeout()
try:
# Get a simple property over the connection
# Get a simple property over the connection, with 2 seconds of timeout
# that should be more than enough to check if the WebDriver its
# still alive, and allows to complete the check within the testrunner
# 5 seconds of extra_timeout we have as maximum to end the test before
# the external timeout from testrunner triggers.
socket.setdefaulttimeout(2)
self.webdriver.window_handle
except (socket.timeout, client.UnknownErrorException):
except (socket.timeout, client.UnknownErrorException, client.InvalidSessionIdException):
return False
finally:
socket.setdefaulttimeout(socket_previous_timeout)
return True

def after_connect(self):
Expand Down

0 comments on commit 58988ab

Please sign in to comment.