From 920a352d31b6ece450345ad2f09b8e4532bce7e5 Mon Sep 17 00:00:00 2001 From: vdelendik Date: Sun, 12 Sep 2021 14:59:13 +0300 Subject: [PATCH] #1361: draft implementation of isAlive method for CarinaDriver in IDriverPool --- .../foundation/webdriver/CarinaDriver.java | 13 ++- .../foundation/webdriver/IDriverPool.java | 19 +++-- .../decorator/ExtendedWebElement.java | 2 +- .../webdriver/listener/DriverListener.java | 80 +++++++++++++------ 4 files changed, 83 insertions(+), 31 deletions(-) diff --git a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/CarinaDriver.java b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/CarinaDriver.java index 93853bc2f1..7bb257e37a 100644 --- a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/CarinaDriver.java +++ b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/CarinaDriver.java @@ -11,14 +11,17 @@ public class CarinaDriver { private Device device; private Phase phase; private long threadId; + private boolean isAlive; - public CarinaDriver(String name, WebDriver driver, Device device, Phase phase, long threadId) { + public CarinaDriver(String name, WebDriver driver, Device device, Phase phase, long threadId) { super(); this.name = name; this.driver = driver; this.device = device; this.phase = phase; this.threadId = threadId; + + this.isAlive = true; } public WebDriver getDriver() { @@ -44,4 +47,12 @@ public Phase getPhase() { protected void setThreadId(long threadId) { this.threadId = threadId; } + + public boolean isAlive() { + return isAlive; + } + + public void setAlive(boolean isAlive) { + this.isAlive = isAlive; + } } diff --git a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/IDriverPool.java b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/IDriverPool.java index 437b4d73a7..90e6126e31 100644 --- a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/IDriverPool.java +++ b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/IDriverPool.java @@ -135,14 +135,14 @@ default public WebDriver getDriver(String name, DesiredCapabilities capabilities } /** - * Get driver by sessionId. + * Get Carina driver by sessionId. * * @param sessionId * - session id to be used for searching a desired driver * - * @return default WebDriver + * @return default CarinaDriver */ - public static WebDriver getDriver(SessionId sessionId) { + public static CarinaDriver getCarinaDriver(SessionId sessionId) { for (CarinaDriver carinaDriver : driversPool) { WebDriver drv = carinaDriver.getDriver(); if (drv instanceof EventFiringWebDriver) { @@ -154,11 +154,11 @@ public static WebDriver getDriver(SessionId sessionId) { if (drvSessionId != null) { if (sessionId.equals(drvSessionId)) { - return drv; + return carinaDriver; } } } - throw new DriverPoolException("Unable to find driver using sessionId artifacts. Returning default one!"); + throw new DriverPoolException("Unable to find driver using sessionId!"); } /** @@ -238,7 +238,14 @@ default public void quitDriver(String name) { throw new RuntimeException("Unable to find driver '" + name + "'!"); } - quitDriver(carinaDrv, false); + if (carinaDrv.isAlive()) { + //do actual quit only if driver still alive + quitDriver(carinaDrv, false); + } else { + POOL_LOGGER.error("Unable to do quit over died driver: " + carinaDrv.getName() + " for thread: " + carinaDrv.getThreadId()); + } + + driversPool.remove(carinaDrv); POOL_LOGGER.debug("after quitDriver: " + driversPool); diff --git a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/decorator/ExtendedWebElement.java b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/decorator/ExtendedWebElement.java index d7b9f42896..385696a002 100644 --- a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/decorator/ExtendedWebElement.java +++ b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/decorator/ExtendedWebElement.java @@ -233,7 +233,7 @@ public ExtendedWebElement(WebElement element, String name) { // that's the only place to use DriverPool to get driver. try { //try to search securely in driver pool by sessionId - this.driver = IDriverPool.getDriver(sessionId); + this.driver = IDriverPool.getCarinaDriver(sessionId).getDriver(); } catch (DriverPoolException ex) { // seems like driver started outside of IDriverPool so try to register it as well this.driver = (WebDriver) tempSearchContext; diff --git a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/listener/DriverListener.java b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/listener/DriverListener.java index 90c6722819..51e8bc56a9 100644 --- a/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/listener/DriverListener.java +++ b/carina-webdriver/src/main/java/com/qaprosoft/carina/core/foundation/webdriver/listener/DriverListener.java @@ -24,6 +24,7 @@ import org.openqa.selenium.OutputType; import org.openqa.selenium.WebDriver; import org.openqa.selenium.WebElement; +import org.openqa.selenium.remote.RemoteWebDriver; import org.openqa.selenium.support.events.EventFiringWebDriver; import org.openqa.selenium.support.events.WebDriverEventListener; import org.slf4j.Logger; @@ -31,6 +32,7 @@ import com.qaprosoft.carina.core.foundation.report.ReportContext; import com.qaprosoft.carina.core.foundation.utils.FileManager; +import com.qaprosoft.carina.core.foundation.webdriver.CarinaDriver; import com.qaprosoft.carina.core.foundation.webdriver.IDriverPool; import com.qaprosoft.carina.core.foundation.webdriver.Screenshot; import com.zebrunner.agent.core.registrar.Artifact; @@ -152,26 +154,15 @@ public void beforeScript(String script, WebDriver driver) { @Override public void onException(Throwable thr, WebDriver driver) { - // [VD] make below code as much safety as possible otherwise potential recursive failure could occur with driver related issue. - // most suspicious are capture screenshots, generating dumps etc - if (thr == null - || thr.getMessage() == null - || thr.getMessage().contains("Method has not yet been implemented") - || thr.getMessage().contains("Expected to read a START_MAP but instead have: END. Last 0 characters read") - || thr.getMessage().contains("Unable to determine type from: <. Last 1 characters read") - || thr.getMessage().contains("script timeout") - || thr.getMessage().contains("javascript error: Cannot read property 'outerHTML' of null") - || thr.getMessage().contains("javascript error: Cannot read property 'scrollHeight' of null") - || thr.getMessage().contains("Method is not implemented") - || thr.getMessage().contains("An element could not be located on the page using the given search parameters") - || thr.getMessage().contains("no such element: Unable to locate element") - // carina has a lot of extra verifications to solve all stale reference issue and finally perform an action so ignore such exception in listener! - || thr.getMessage().contains("StaleElementReferenceException") - || thr.getMessage().contains("stale_element_reference.html")) { + if (thr == null || thr.getMessage() == null) { // do nothing return; } - + + CarinaDriver carinaDriver = IDriverPool.getCarinaDriver(((RemoteWebDriver) driver).getSessionId()); + + String message = thr.getMessage(); + // handle use-case when application crashed on iOS but tests continue to execute something because doesn't raise valid exception // Example: @@ -183,21 +174,64 @@ public void onException(Throwable thr, WebDriver driver) { // stacktrace information) // TODO: investigate if we run @AfterMethod etc system events after this crash - if (thr.getMessage().contains("is not running, possibly crashed")) { + if (message.contains("is not running, possibly crashed")) { throw new RuntimeException(thr); } + + if (message.contains("Session ID is null. Using WebDriver after calling quit") + || message.contains("A session is either terminated or not started") + || message.contains("invalid session id") + || message.contains("Session does not exist") + || message.contains("not found in active sessions") + || message.contains("Session timed out or not found") + || message.contains("Unable to determine type from: <. Last 1 characters read") + || message.contains("not available and is not among the last 1000 terminated sessions") + || message.contains("cannot forward the request") + || message.contains("connect ECONNREFUSED") + || message.contains("was terminated due to") // FORWARDING_TO_NODE_FAILED, CLIENT_STOPPED_SESSION, PROXY_REREGISTRATION, TIMEOUT, BROWSER_TIMEOUT etc + || message.contains("no such window: window was already closed") + || message.contains("Error communicating with the remote browser. It may have died") + || message.contains("chrome not reachable") + || message.contains("cannot forward the request Connect to") + || message.contains("Could not proxy command to remote server. Original error:") // Error: socket hang up, Error: read ECONNRESET etc + || message.contains("Could not proxy command to the remote server. Original error:") // Different messages on some Appium versions + || message.contains("Driver connection refused")) { + // mark driver as not alive anymore! + LOGGER.error("Mark current driver as died!"); + + carinaDriver.setAlive(false); + return; + } + + // [VD] make below code as much safety as possible otherwise potential recursive failure could occur with driver related issue. + // most suspicious are capture screenshots, generating dumps etc + if (message.contains("Method has not yet been implemented") + || message.contains("Expected to read a START_MAP but instead have: END. Last 0 characters read") + || message.contains("Unable to determine type from: <. Last 1 characters read") + || message.contains("script timeout") + || message.contains("javascript error: Cannot read property 'outerHTML' of null") + || message.contains("javascript error: Cannot read property 'scrollHeight' of null") + || message.contains("Method is not implemented") + || message.contains("An element could not be located on the page using the given search parameters") + || message.contains("no such element: Unable to locate element") + // carina has a lot of extra verifications to solve all stale reference issue and finally perform an action so ignore such exception in listener! + || message.contains("StaleElementReferenceException") + || message.contains("stale_element_reference.html")) { + // do nothing + return; + } // hopefully castDriver below resolve root cause of the recursive onException calls but keep below to ensure if (thr.getStackTrace() != null && (Arrays.toString(thr.getStackTrace()) .contains("com.qaprosoft.carina.core.foundation.webdriver.listener.DriverListener.onException") || Arrays.toString(thr.getStackTrace()).contains("Unable to capture screenshot due to the WebDriverException"))) { - LOGGER.error("Do not generate screenshot for invalid driver!"); + LOGGER.warn("Do not generate recursive screenshots!"); // prevent recursive crash for onException return; } - LOGGER.debug("DriverListener->onException starting..." + thr.getMessage()); + LOGGER.debug("DriverListener->onException starting..." + message); driver = castDriver(driver); try { @@ -207,8 +241,8 @@ public void onException(Throwable thr, WebDriver driver) { // 3. 99% those root exception means that we should prohibit screenshot generation for such use-case // 4. if 3rd one is true just update Screenshot.isCaptured() adding part of the exception to the list // handle cases which should't be captured - if (Screenshot.isCaptured(thr.getMessage())) { - captureScreenshot(thr.getMessage(), driver, null, true); + if (carinaDriver.isAlive() && Screenshot.isCaptured(message)) { + captureScreenshot(message, driver, null, true); } } catch (Exception e) { if (!e.getMessage().isEmpty() @@ -218,7 +252,7 @@ public void onException(Throwable thr, WebDriver driver) { LOGGER.error("Unrecognized exception detected in DriverListener->onException! " + e.getMessage(), e); } } catch (Throwable e) { - LOGGER.error("Take a look to the logs above for current thread and add exception into the exclusion for Screenshot.isCaptured(). " + LOGGER.error("Take a look to the logs above for current thread and exclude particular exception for Screenshot.isCaptured(). " + e.getMessage(), e); }