From 24667342211bd429eafaafcc0f34e40885e589fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Sat, 7 Sep 2024 17:26:14 +0200 Subject: [PATCH] Fix glitches in WaitFor: NRE, monotonic clock, exception after success Fixed NullReferenceException when exceptionThrown variable in BrowserWrapper was null In general, if the condition is successful WaitFor will now suceed, even if the deadline has already passed. Switched to Stopwatch to avoid non-monotonic clock goblins --- .../Riganti.Selenium.Core/BrowserWrapper.cs | 52 ++++++++----------- .../Riganti.Selenium.Core/ElementWrapper.cs | 29 +++++------ .../Riganti.Selenium.Core/WaitForExecutor.cs | 29 ++++++----- 3 files changed, 49 insertions(+), 61 deletions(-) diff --git a/src/Core/Riganti.Selenium.Core/BrowserWrapper.cs b/src/Core/Riganti.Selenium.Core/BrowserWrapper.cs index 8dee6ab7..abff0c0b 100644 --- a/src/Core/Riganti.Selenium.Core/BrowserWrapper.cs +++ b/src/Core/Riganti.Selenium.Core/BrowserWrapper.cs @@ -589,34 +589,28 @@ public IBrowserWrapper WaitFor(Func condition, int timeout, string failure { throw new NullReferenceException("Condition cannot be null."); } - var now = DateTime.UtcNow; + var stopwatch = Stopwatch.StartNew(); - bool isConditionMet = false; - Exception ex = null; - do + while (true) { try { - isConditionMet = condition(); + if (condition()) + return this; } - catch (StaleElementReferenceException) + catch (StaleElementReferenceException) when (ignoreCertainException) { - if (!ignoreCertainException) - throw; } - catch (InvalidElementStateException) + catch (InvalidElementStateException) when (ignoreCertainException) { - if (!ignoreCertainException) - throw; } - if (DateTime.UtcNow.Subtract(now).TotalMilliseconds > timeout) + if (stopwatch.ElapsedMilliseconds > timeout) { - throw new WaitBlockException(failureMessage); + throw new WaitBlockException(failureMessage ?? "Condition returned false after timeout expired."); } Wait(checkInterval); - } while (!isConditionMet); - return this; + } } /// @@ -654,32 +648,28 @@ public IBrowserWrapper WaitFor(Action action, int timeout, int checkInterval = 3 { throw new ArgumentNullException(nameof(action)); } - var now = DateTime.UtcNow; + var stopwatch = Stopwatch.StartNew(); - Exception exceptionThrown = null; - do + while (true) { try { action(); - exceptionThrown = null; + return this; } - catch (Exception ex) + catch (Exception exceptionThrown) { - exceptionThrown = ex; - } - - if (DateTime.UtcNow.Subtract(now).TotalMilliseconds > timeout) - { - if (failureMessage != null) + if (stopwatch.ElapsedMilliseconds > timeout) { - throw new WaitBlockException(failureMessage, exceptionThrown); + if (failureMessage != null) + { + throw new WaitBlockException(failureMessage, exceptionThrown); + } + throw; } - throw exceptionThrown; } Wait(checkInterval); - } while (exceptionThrown != null); - return this; + } } public IElementWrapper WaitFor(Func selector, WaitForOptions options = null) @@ -788,4 +778,4 @@ protected IBrowserWrapper EvaluateBrowserCheck(IValidator condition, int maxTim { throw new NullReferenceException("Condition cannot be null."); } - var now = DateTime.UtcNow; - bool isConditionMet = false; - do + var stopwatch = Stopwatch.StartNew(); + + while (true) { try { - isConditionMet = condition(this); + if (condition(this)) + return this; } - catch (StaleElementReferenceException) + catch (StaleElementReferenceException) when (ignoreCertainException) { - if (!ignoreCertainException) - throw; } - catch (InvalidElementStateException) + catch (InvalidElementStateException) when (ignoreCertainException) { - if (!ignoreCertainException) - throw; } - if (DateTime.UtcNow.Subtract(now).TotalMilliseconds > maxTimeout) + if (stopwatch.ElapsedMilliseconds > maxTimeout) { - throw new WaitBlockException(failureMessage); + throw new WaitBlockException(failureMessage ?? "Condition returned false after timeout expired."); } - Wait(checkInterval); - } while (!isConditionMet); - return this; + Wait(checkInterval); + } } public IElementWrapper WaitFor(Action checkExpression, int maxTimeout, string failureMessage, int checkInterval = 30) @@ -730,4 +727,4 @@ public bool HasCssClass(string cssClass) return attr.Split(' ').Any(s => string.Equals(cssClass, s, StringComparison.OrdinalIgnoreCase)); } } -} \ No newline at end of file +} diff --git a/src/Core/Riganti.Selenium.Core/WaitForExecutor.cs b/src/Core/Riganti.Selenium.Core/WaitForExecutor.cs index 061dac5a..389d2035 100644 --- a/src/Core/Riganti.Selenium.Core/WaitForExecutor.cs +++ b/src/Core/Riganti.Selenium.Core/WaitForExecutor.cs @@ -1,6 +1,7 @@ using System; using Riganti.Selenium.Core.Abstractions.Exceptions; using System.Threading; +using System.Diagnostics; namespace Riganti.Selenium.Core { @@ -12,26 +13,25 @@ public void WaitFor(Action condition, int timeout, string failureMessage, int ch { throw new NullReferenceException("Condition cannot be null."); } - var now = DateTime.UtcNow; + var stopwatch = Stopwatch.StartNew(); - bool isConditionMet = false; - do + while (true) { try { condition(); - isConditionMet = true; + return; } catch (TestExceptionBase ex) { - if (DateTime.UtcNow.Subtract(now).TotalMilliseconds > timeout) + if (stopwatch.ElapsedMilliseconds > timeout) { if (throwOriginal) throw; else throw new WaitBlockException(failureMessage ?? ex.Message, ex); } } Thread.Sleep(checkInterval); - } while (!isConditionMet); + } } public void WaitFor(Func condition, WaitForOptions options = null) { @@ -40,29 +40,30 @@ public void WaitFor(Func condition, WaitForOptions options = null) { throw new NullReferenceException("Condition cannot be null."); } - var now = DateTime.UtcNow; + var stopwatch = Stopwatch.StartNew(); - bool isConditionMet = false; - do + while (true) { try { - isConditionMet = condition(); + if (condition()) + return; } catch (TestExceptionBase ex) { - if (DateTime.UtcNow.Subtract(now).TotalMilliseconds > options.Timeout) + if (stopwatch.ElapsedMilliseconds > options.Timeout) { if (options.ThrowOriginalException) throw; else throw new WaitBlockException(options.FailureMessage ?? ex.Message, ex); } } - if (DateTime.UtcNow.Subtract(now).TotalMilliseconds > options.Timeout) + if (stopwatch.ElapsedMilliseconds > options.Timeout) { - throw new WaitBlockException(options.FailureMessage); + throw new WaitBlockException(options.FailureMessage ?? "Condition returned false after timeout expired."); } + Thread.Sleep(options.CheckInterval); - } while (!isConditionMet); + } } ///