From 86c06c4fd3cbe7a79c71ae01b4efc0d4f389bb6a Mon Sep 17 00:00:00 2001 From: Yury Semikhatsky Date: Fri, 5 Mar 2021 13:18:24 -0800 Subject: [PATCH] fix(api): throw TimeoutError on timeout (#323) --- .../java/com/microsoft/playwright/TimeoutError.java | 11 ++++++++--- .../com/microsoft/playwright/impl/Connection.java | 9 ++++++--- .../com/microsoft/playwright/impl/WaitableResult.java | 4 ++++ .../microsoft/playwright/impl/WaitableTimeout.java | 4 ++-- .../TestElementHandleWaitForElementState.java | 2 +- .../com/microsoft/playwright/TestFrameNavigate.java | 2 +- .../microsoft/playwright/TestPageSetInputFiles.java | 6 +++--- .../playwright/TestPageWaitForNavigation.java | 2 +- .../com/microsoft/playwright/TestWaitForFunction.java | 6 +++--- .../com/microsoft/playwright/tools/ApiGenerator.java | 4 ++++ 10 files changed, 33 insertions(+), 17 deletions(-) diff --git a/playwright/src/main/java/com/microsoft/playwright/TimeoutError.java b/playwright/src/main/java/com/microsoft/playwright/TimeoutError.java index e8333c65..85cfe19f 100644 --- a/playwright/src/main/java/com/microsoft/playwright/TimeoutError.java +++ b/playwright/src/main/java/com/microsoft/playwright/TimeoutError.java @@ -16,12 +16,17 @@ package com.microsoft.playwright; -import java.util.*; - /** * TimeoutError is emitted whenever certain operations are terminated due to timeout, e.g. {@link Page#waitForSelector * Page.waitForSelector()} or {@link BrowserType#launch BrowserType.launch()}. */ -public interface TimeoutError { +public class TimeoutError extends PlaywrightException { + public TimeoutError(String message) { + super(message); + } + + public TimeoutError(String message, Throwable exception) { + super(message, exception); + } } diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/Connection.java b/playwright/src/main/java/com/microsoft/playwright/impl/Connection.java index ea8bb793..a1d1175b 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/Connection.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/Connection.java @@ -20,6 +20,7 @@ import com.google.gson.JsonArray; import com.google.gson.JsonElement; import com.google.gson.JsonObject; import com.microsoft.playwright.PlaywrightException; +import com.microsoft.playwright.TimeoutError; import java.io.File; import java.io.IOException; @@ -200,10 +201,12 @@ public class Connection { if (message.error == null) { callback.complete(message.result); } else { - if (message.error.error != null) { - callback.completeExceptionally(new DriverException(message.error.error)); - } else { + if (message.error.error == null) { callback.completeExceptionally(new PlaywrightException(message.error.toString())); + } else if ("TimeoutError".equals(message.error.error.name)) { + callback.completeExceptionally(new TimeoutError(message.error.error.toString())); + } else { + callback.completeExceptionally(new DriverException(message.error.error)); } } return; diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/WaitableResult.java b/playwright/src/main/java/com/microsoft/playwright/impl/WaitableResult.java index baf9f6b5..8ff7fc9a 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/WaitableResult.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/WaitableResult.java @@ -17,6 +17,7 @@ package com.microsoft.playwright.impl; import com.microsoft.playwright.PlaywrightException; +import com.microsoft.playwright.TimeoutError; class WaitableResult implements Waitable { private T result; @@ -47,6 +48,9 @@ class WaitableResult implements Waitable { @Override public T get() { if (exception != null) { + if (exception instanceof TimeoutError) { + throw new TimeoutError(exception.getMessage(), exception); + } throw new PlaywrightException(exception.getMessage(), exception); } return result; diff --git a/playwright/src/main/java/com/microsoft/playwright/impl/WaitableTimeout.java b/playwright/src/main/java/com/microsoft/playwright/impl/WaitableTimeout.java index eab0441e..1c16fa3d 100644 --- a/playwright/src/main/java/com/microsoft/playwright/impl/WaitableTimeout.java +++ b/playwright/src/main/java/com/microsoft/playwright/impl/WaitableTimeout.java @@ -16,7 +16,7 @@ package com.microsoft.playwright.impl; -import com.microsoft.playwright.PlaywrightException; +import com.microsoft.playwright.TimeoutError; class WaitableTimeout implements Waitable { private final long deadline; @@ -38,7 +38,7 @@ class WaitableTimeout implements Waitable { if (timeoutStr.endsWith(".0")) { timeoutStr = timeoutStr.substring(0, timeoutStr.length() - 2); } - throw new PlaywrightException("Timeout " + timeoutStr + "ms exceeded"); + throw new TimeoutError("Timeout " + timeoutStr + "ms exceeded"); } @Override diff --git a/playwright/src/test/java/com/microsoft/playwright/TestElementHandleWaitForElementState.java b/playwright/src/test/java/com/microsoft/playwright/TestElementHandleWaitForElementState.java index d63f86cf..7b4d42b4 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestElementHandleWaitForElementState.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestElementHandleWaitForElementState.java @@ -54,7 +54,7 @@ public class TestElementHandleWaitForElementState extends TestBase { try { div.waitForElementState(VISIBLE, new ElementHandle.WaitForElementStateOptions().withTimeout(1000)); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 1000ms exceeded")); } } diff --git a/playwright/src/test/java/com/microsoft/playwright/TestFrameNavigate.java b/playwright/src/test/java/com/microsoft/playwright/TestFrameNavigate.java index bc2a99e8..3bb2f97b 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestFrameNavigate.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestFrameNavigate.java @@ -45,7 +45,7 @@ public class TestFrameNavigate extends TestBase { String url = server.PREFIX + "/frames/child-redirect.html"; try { page.navigate(url, new Page.NavigateOptions().withTimeout(5000).withWaitUntil(NETWORKIDLE)); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 5000ms exceeded.")); assertTrue(e.getMessage().contains("navigating to \"" + url +"\", waiting until \"networkidle\"")); } diff --git a/playwright/src/test/java/com/microsoft/playwright/TestPageSetInputFiles.java b/playwright/src/test/java/com/microsoft/playwright/TestPageSetInputFiles.java index b97ea091..cedf5256 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestPageSetInputFiles.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestPageSetInputFiles.java @@ -129,7 +129,7 @@ public class TestPageSetInputFiles extends TestBase { try { page.waitForFileChooser(new Page.WaitForFileChooserOptions().withTimeout(1), () -> {}); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 1ms exceeded")); } } @@ -140,7 +140,7 @@ public class TestPageSetInputFiles extends TestBase { try { page.waitForFileChooser(() -> {}); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 1ms exceeded")); } } @@ -151,7 +151,7 @@ public class TestPageSetInputFiles extends TestBase { try { page.waitForFileChooser(new Page.WaitForFileChooserOptions().withTimeout(1), () -> {}); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 1ms exceeded")); } } diff --git a/playwright/src/test/java/com/microsoft/playwright/TestPageWaitForNavigation.java b/playwright/src/test/java/com/microsoft/playwright/TestPageWaitForNavigation.java index 07717dac..06f0bdd6 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestPageWaitForNavigation.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestPageWaitForNavigation.java @@ -45,7 +45,7 @@ public class TestPageWaitForNavigation extends TestBase { new Page.WaitForNavigationOptions().withUrl("**/frame.html").withTimeout(5000), () -> page.navigate(server.EMPTY_PAGE)); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 5000ms exceeded")); // assertTrue(e.getMessage().contains("waiting for navigation to '**/frame.html' until 'load'")); // assertTrue(e.getMessage().contains("navigated to '${server.EMPTY_PAGE}'")); diff --git a/playwright/src/test/java/com/microsoft/playwright/TestWaitForFunction.java b/playwright/src/test/java/com/microsoft/playwright/TestWaitForFunction.java index a5941daa..53776c03 100644 --- a/playwright/src/test/java/com/microsoft/playwright/TestWaitForFunction.java +++ b/playwright/src/test/java/com/microsoft/playwright/TestWaitForFunction.java @@ -80,7 +80,7 @@ public class TestWaitForFunction extends TestBase { " console.log(window['counter']);\n" + "}", null, new Page.WaitForFunctionOptions().withPollingInterval(1).withTimeout(1000)); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 1000ms exceeded")); } @@ -180,7 +180,7 @@ public class TestWaitForFunction extends TestBase { try { page.waitForFunction("false", null, new Page.WaitForFunctionOptions().withTimeout(10)); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 10ms exceeded")); } } @@ -191,7 +191,7 @@ public class TestWaitForFunction extends TestBase { try { page.waitForFunction("false"); fail("did not throw"); - } catch (PlaywrightException e) { + } catch (TimeoutError e) { assertTrue(e.getMessage().contains("Timeout 1ms exceeded")); } } diff --git a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java index 344a5ef8..9ed075ad 100644 --- a/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java +++ b/tools/api-generator/src/main/java/com/microsoft/playwright/tools/ApiGenerator.java @@ -1038,6 +1038,10 @@ public class ApiGenerator { Map topLevelTypes = new HashMap<>(); for (JsonElement entry: api) { String name = entry.getAsJsonObject().get("name").getAsString(); + // We write this one manually. + if ("TimeoutError".equals(name)) { + continue; + } List lines = new ArrayList<>(); new Interface(entry.getAsJsonObject(), topLevelTypes).writeTo(lines, ""); String text = String.join("\n", lines);