diff --git a/java/client/src/org/openqa/selenium/support/ui/FluentWait.java b/java/client/src/org/openqa/selenium/support/ui/FluentWait.java index 9668604a6ed65..f5fbdec3f001e 100644 --- a/java/client/src/org/openqa/selenium/support/ui/FluentWait.java +++ b/java/client/src/org/openqa/selenium/support/ui/FluentWait.java @@ -28,10 +28,11 @@ import java.time.Clock; import java.time.Duration; import java.time.Instant; -import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collection; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.function.Function; import java.util.function.Supplier; @@ -203,44 +204,70 @@ public FluentWait ignoring(Class firstType, */ @Override public V until(Function isTrue) { - Instant end = clock.instant().plus(timeout); - - Throwable lastException; - while (true) { - try { - V value = isTrue.apply(input); - if (value != null && (Boolean.class != value.getClass() || Boolean.TRUE.equals(value))) { - return value; + try { + return CompletableFuture.supplyAsync(checkConditionInLoop(isTrue)) + .get(deriveSafeTimeout(), TimeUnit.MILLISECONDS); + } catch (ExecutionException cause) { + if (cause.getCause() instanceof RuntimeException) { + throw (RuntimeException) cause.getCause(); + } else if (cause.getCause() instanceof Error) { + throw (Error) cause.getCause(); + } + + throw new RuntimeException(cause); + } catch (InterruptedException cause) { + throw new RuntimeException(cause); + } catch (java.util.concurrent.TimeoutException cause) { + throw new TimeoutException("Supplied function might have stalled", cause); + } + } + + private Supplier checkConditionInLoop(Function isTrue) { + return () -> { + Instant end = clock.instant().plus(timeout); + + Throwable lastException; + while (true) { + //noinspection ProhibitedExceptionCaught + try { + V value = isTrue.apply(input); + if (value != null && (Boolean.class != value.getClass() || Boolean.TRUE.equals(value))) { + return value; + } + + // Clear the last exception; if another retry or timeout exception would + // be caused by a false or null value, the last exception is not the + // cause of the timeout. + lastException = null; + } catch (Throwable e) { + lastException = propagateIfNotIgnored(e); } - // Clear the last exception; if another retry or timeout exception would - // be caused by a false or null value, the last exception is not the - // cause of the timeout. - lastException = null; - } catch (Throwable e) { - lastException = propagateIfNotIgnored(e); - } + // Check the timeout after evaluating the function to ensure conditions + // with a zero timeout can succeed. + if (end.isBefore(clock.instant())) { + String message = messageSupplier != null ? messageSupplier.get() : null; - // Check the timeout after evaluating the function to ensure conditions - // with a zero timeout can succeed. - if (end.isBefore(clock.instant())) { - String message = messageSupplier != null ? - messageSupplier.get() : null; - - String timeoutMessage = String.format( - "Expected condition failed: %s (tried for %d second(s) with %d milliseconds interval)", - message == null ? "waiting for " + isTrue : message, - timeout.getSeconds(), interval.toMillis()); - throw timeoutException(timeoutMessage, lastException); - } + String timeoutMessage = String.format( + "Expected condition failed: %s (tried for %d second(s) with %d milliseconds interval)", + message == null ? "waiting for " + isTrue : message, + timeout.getSeconds(), interval.toMillis()); + throw timeoutException(timeoutMessage, lastException); + } - try { - sleeper.sleep(interval); - } catch (InterruptedException e) { - Thread.currentThread().interrupt(); - throw new WebDriverException(e); + try { + sleeper.sleep(interval); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new WebDriverException(e); + } } - } + }; + } + + /** This timeout is somewhat arbitrary. */ + private long deriveSafeTimeout() { + return this.timeout.toMillis() + this.interval.toMillis(); } private Throwable propagateIfNotIgnored(Throwable e) { @@ -265,33 +292,4 @@ private Throwable propagateIfNotIgnored(Throwable e) { protected RuntimeException timeoutException(String message, Throwable lastException) { throw new TimeoutException(message, lastException); } - - /** - * Converts the {@code TimeUnit} to the equivalent {@code ChronoUnit}. - * - * This is a backport from Java 9, see https://bugs.openjdk.java.net/browse/JDK-8141452. - * - * @param timeUnit the TimeUnit to convert - * @return the converted equivalent ChronoUnit - */ - private ChronoUnit toChronoUnit(TimeUnit timeUnit) { - switch (timeUnit) { - case NANOSECONDS: - return ChronoUnit.NANOS; - case MICROSECONDS: - return ChronoUnit.MICROS; - case MILLISECONDS: - return ChronoUnit.MILLIS; - case SECONDS: - return ChronoUnit.SECONDS; - case MINUTES: - return ChronoUnit.MINUTES; - case HOURS: - return ChronoUnit.HOURS; - case DAYS: - return ChronoUnit.DAYS; - default: - throw new IllegalArgumentException("No ChronoUnit equivalent for " + timeUnit); - } - } } diff --git a/java/client/test/org/openqa/selenium/support/ui/FluentWaitTest.java b/java/client/test/org/openqa/selenium/support/ui/FluentWaitTest.java index 05255ef0601f6..3a969f9b9aa32 100644 --- a/java/client/test/org/openqa/selenium/support/ui/FluentWaitTest.java +++ b/java/client/test/org/openqa/selenium/support/ui/FluentWaitTest.java @@ -256,6 +256,25 @@ protected RuntimeException timeoutException(String message, Throwable lastExcept .satisfies(expected -> assertThat(sentinelException).isSameAs(expected)); } + @Test + public void timeoutWhenConditionMakesNoProgress() { + + when(mockClock.instant()).thenReturn(EPOCH, EPOCH.plusMillis(2500)); + when(mockCondition.apply(mockDriver)).then(invocation -> { + while (true) { + // it gets into an endless loop and makes no progress. + } + }); + + FluentWait wait = new FluentWait<>(mockDriver, mockClock, mockSleeper) + .withTimeout(Duration.ofSeconds(0)) + .pollingEvery(Duration.ofSeconds(2)); + + assertThatExceptionOfType(org.openqa.selenium.TimeoutException.class) + .isThrownBy(() -> wait.until(mockCondition)) + .satisfies(actual -> assertThat(actual.getMessage()).startsWith("Supplied function might have stalled")); + } + private static class TestException extends RuntimeException { }