From 9c295dfa7839e0aade5a8713d026ca2979914c33 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 9 Mar 2024 00:22:41 +0100 Subject: [PATCH 1/8] chore: Make server startup error messages more useful --- .../local/AppiumDriverLocalService.java | 108 ++++++++++++------ .../AppiumServerAvailabilityChecker.java | 86 ++++++++++++++ .../local/AppiumServerConnectionError.java | 61 ++++++++++ .../local/AppiumServerConnectionTimeout.java | 31 +++++ ...rverHasNotBeenStartedLocallyException.java | 14 +-- 5 files changed, 256 insertions(+), 44 deletions(-) create mode 100644 src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java create mode 100644 src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java create mode 100644 src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index 45f611eab..027c6218e 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -18,7 +18,6 @@ import com.google.common.annotations.VisibleForTesting; import lombok.SneakyThrows; -import org.openqa.selenium.net.UrlChecker; import org.openqa.selenium.os.ExternalProcess; import org.openqa.selenium.remote.service.DriverService; import org.slf4j.Logger; @@ -30,14 +29,12 @@ import java.io.File; import java.io.IOException; import java.io.OutputStream; -import java.net.MalformedURLException; import java.net.URL; import java.time.Duration; import java.util.ArrayList; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import java.util.function.BiConsumer; import java.util.function.Consumer; @@ -67,6 +64,7 @@ public final class AppiumDriverLocalService extends DriverService { private final Duration startupTimeout; private final ReentrantLock lock = new ReentrantLock(true); //uses "fair" thread ordering policy private final ListOutputStream stream = new ListOutputStream().add(System.out); + private final AppiumServerAvailabilityChecker availabilityChecker = new AppiumServerAvailabilityChecker(); private final URL url; private String basePath; @@ -131,36 +129,39 @@ public boolean isRunning() { } try { - ping(IS_RUNNING_PING_TIMEOUT); - return true; - } catch (UrlChecker.TimeoutException e) { + return ping(IS_RUNNING_PING_TIMEOUT); + } catch (AppiumServerConnectionTimeout | AppiumServerConnectionError e) { return false; - } catch (MalformedURLException e) { - throw new AppiumServerHasNotBeenStartedLocallyException(e.getMessage(), e); + } catch (InterruptedException e) { + throw new RuntimeException(e); } } finally { lock.unlock(); } + } + private boolean ping(Duration timeout) throws InterruptedException { + var baseURL = fixBroadcastAddresses(getUrl()); + var statusUrl = addSuffix(baseURL, "/status"); + return availabilityChecker.waitUntilAvailable(statusUrl, timeout); } - private void ping(Duration timeout) throws UrlChecker.TimeoutException, MalformedURLException { - URL baseURL = getUrl(); - String host = baseURL.getHost(); + private URL fixBroadcastAddresses(URL url) { + var host = url.getHost(); // The operating system will block direct access to the universal broadcast IP address if (host.equals(BROADCAST_IP4_ADDRESS)) { - baseURL = replaceHost(baseURL, BROADCAST_IP4_ADDRESS, "127.0.0.1"); - } else if (host.equals(BROADCAST_IP6_ADDRESS)) { - baseURL = replaceHost(baseURL, BROADCAST_IP6_ADDRESS, "::1"); + return replaceHost(url, BROADCAST_IP4_ADDRESS, "127.0.0.1"); + } + if (host.equals(BROADCAST_IP6_ADDRESS)) { + return replaceHost(url, BROADCAST_IP6_ADDRESS, "::1"); } - URL status = addSuffix(baseURL, "/status"); - new UrlChecker().waitUntilAvailable(timeout.toMillis(), TimeUnit.MILLISECONDS, status); + return url; } /** * Starts the defined appium server. * - * @throws AppiumServerHasNotBeenStartedLocallyException If an error occurs while spawning the child process. + * @throws AppiumServerHasNotBeenStartedLocallyException If an error occurs on Appium server startup. * @see #stop() */ @Override @@ -172,40 +173,73 @@ public void start() throws AppiumServerHasNotBeenStartedLocallyException { } try { - ExternalProcess.Builder processBuilder = ExternalProcess.builder() + var processBuilder = ExternalProcess.builder() .command(this.nodeJSExec.getCanonicalPath(), nodeJSArgs) .copyOutputTo(stream); nodeJSEnvironment.forEach(processBuilder::environment); process = processBuilder.start(); + } catch (IOException e) { + throw new AppiumServerHasNotBeenStartedLocallyException(e); + } + + var didPingSucceed = false; + try { ping(startupTimeout); - } catch (Exception e) { - final Optional output = ofNullable(process) - .map(ExternalProcess::getOutput) - .filter(o -> !isNullOrEmpty(o)); - destroyProcess(); - List errorLines = new ArrayList<>(); - errorLines.add("The local appium server has not been started"); - errorLines.add(String.format("Reason: %s", e.getMessage())); - if (e instanceof UrlChecker.TimeoutException) { - errorLines.add(String.format( - "Consider increasing the server startup timeout value (currently %sms)", - startupTimeout.toMillis() - )); - } - errorLines.add( - String.format("Node.js executable path: %s", nodeJSExec.getAbsolutePath()) - ); - errorLines.add(String.format("Arguments: %s", nodeJSArgs)); - output.ifPresent(o -> errorLines.add(String.format("Output: %s", o))); + didPingSucceed = true; + } catch (AppiumServerConnectionTimeout | AppiumServerConnectionError e) { + var errorLines = new ArrayList<>(generateDetailedErrorMessagePrefix(e)); + errorLines.addAll(retrieveServerDebugInfo()); throw new AppiumServerHasNotBeenStartedLocallyException( String.join("\n", errorLines), e ); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } finally { + if (!didPingSucceed) { + destroyProcess(); + } } } finally { lock.unlock(); } } + private List generateDetailedErrorMessagePrefix(RuntimeException e) { + var errorLines = new ArrayList(); + if (e instanceof AppiumServerConnectionTimeout) { + errorLines.add(String.format( + "Appium HTTP server is not listening at %s after %s ms timeout. " + + "Consider increasing the server startup timeout value and " + + "check the server log for possible error messages.", + getUrl(), ((AppiumServerConnectionTimeout) e).getTimeout().toMillis() + )); + } else if (e instanceof AppiumServerConnectionError) { + var statusCode = ((AppiumServerConnectionError) e).getResponseCode(); + var statusUrl = ((AppiumServerConnectionError) e).getStatusUrl(); + var payload = ((AppiumServerConnectionError) e).getPayload(); + errorLines.add(String.format( + "Appium HTTP server has started and is listening although we were " + + "unable to get a successful response from %s. Make sure both client " + + "and server use the same base path and check the server log for possible " + + "error messages.", statusUrl + )); + errorLines.add(String.format("HTTP status code: %s", statusCode)); + payload.ifPresent(p -> errorLines.add(String.format("Response payload: %s", p))); + } + return errorLines; + } + + private List retrieveServerDebugInfo() { + var result = new ArrayList(); + result.add(String.format("Node.js executable path: %s", nodeJSExec.getAbsolutePath())); + result.add(String.format("Arguments: %s", nodeJSArgs)); + ofNullable(process) + .map(ExternalProcess::getOutput) + .filter(o -> !isNullOrEmpty(o)) + .ifPresent(o -> result.add(String.format("Server process output: %s", o))); + return result; + } + /** * Stops this service is it is currently running. This method will attempt to block until the * server has been fully shutdown. diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java new file mode 100644 index 000000000..2795240d3 --- /dev/null +++ b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java @@ -0,0 +1,86 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.appium.java_client.service.local; + +import java.io.IOException; +import java.net.HttpURLConnection; +import java.net.URL; +import java.time.Duration; +import java.time.Instant; +import java.util.Optional; + +public class AppiumServerAvailabilityChecker { + private static final Duration CONNECT_TIMEOUT = Duration.ofMillis(500); + private static final Duration READ_TIMEOUT = Duration.ofSeconds(1); + private static final Duration MAX_POLL_INTERVAL = Duration.ofMillis(320); + private static final Duration MIN_POLL_INTERVAL = Duration.ofMillis(10); + + /** + * Verifies a possibility of establishing a connection + * to a running Appium server. + * + * @param serverStatusUrl The URL of /status endpoint. + * @param timeout Wait timeout. If the server responds with non-200 error + * code then we are not going to retry, but throw an exception + * immediately. + * @return true in case of success + * @throws InterruptedException If the API is interrupted + * @throws AppiumServerConnectionTimeout If it is not possible to successfully open + * an HTTP connection to the server /status endpoint. + * @throws AppiumServerConnectionError If an HTTP connection was opened successfully, + * but non-200 error code was received. + */ + public boolean waitUntilAvailable(URL serverStatusUrl, Duration timeout) throws InterruptedException { + var interval = MIN_POLL_INTERVAL; + var start = Instant.now(); + IOException lastError = null; + while (Duration.between(start, Instant.now()).compareTo(timeout) <= 0) { + HttpURLConnection connection = null; + try { + connection = connectToUrl(serverStatusUrl); + return checkResponse(connection); + } catch (IOException e) { + lastError = e; + } finally { + Optional.ofNullable(connection).ifPresent(HttpURLConnection::disconnect); + } + //noinspection BusyWait + Thread.sleep(interval.toMillis()); + interval = interval.compareTo(MAX_POLL_INTERVAL) >= 0 ? interval : interval.multipliedBy(2); + } + throw new AppiumServerConnectionTimeout(timeout, lastError); + } + + private HttpURLConnection connectToUrl(URL url) throws IOException { + var connection = (HttpURLConnection) url.openConnection(); + connection.setConnectTimeout((int) CONNECT_TIMEOUT.toMillis()); + connection.setReadTimeout((int) READ_TIMEOUT.toMillis()); + connection.connect(); + return connection; + } + + private boolean checkResponse(HttpURLConnection connection) throws IOException { + var responseCode = connection.getResponseCode(); + if (responseCode == HttpURLConnection.HTTP_OK) { + return true; + } + var is = responseCode < HttpURLConnection.HTTP_BAD_REQUEST + ? connection.getInputStream() + : connection.getErrorStream(); + throw new AppiumServerConnectionError(connection.getURL(), responseCode, is); + } +} diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java b/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java new file mode 100644 index 000000000..161fb88a2 --- /dev/null +++ b/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java @@ -0,0 +1,61 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.appium.java_client.service.local; + +import lombok.Getter; + +import java.io.BufferedReader; +import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.net.URL; +import java.util.Optional; + +@Getter +public class AppiumServerConnectionError extends RuntimeException { + private static final int MAX_PAYLOAD_LEN = 500; + + private final URL statusUrl; + private final int responseCode; + private final Optional payload; + + public AppiumServerConnectionError(URL statusUrl, int responseCode, InputStream body) { + super(AppiumServerConnectionError.class.getSimpleName()); + this.statusUrl = statusUrl; + this.responseCode = responseCode; + this.payload = readResponseStreamSafely(body); + } + + private static Optional readResponseStreamSafely(InputStream is) { + try (var br = new BufferedReader(new InputStreamReader(is))) { + var result = new StringBuilder(); + String currentLine; + var didCut = false; + while ((currentLine = br.readLine()) != null) { + result.append(currentLine).append("\n"); + if (result.toString().length() >= MAX_PAYLOAD_LEN) { + didCut = true; + break; + } + } + var s = didCut ? (result.toString().trim() + "…") : result.toString().trim(); + return s.trim().isEmpty() ? Optional.empty() : Optional.of(s); + } catch (IOException e) { + return Optional.empty(); + } + } +} diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java b/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java new file mode 100644 index 000000000..f71428fcf --- /dev/null +++ b/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java @@ -0,0 +1,31 @@ +/* + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * See the NOTICE file distributed with this work for additional + * information regarding copyright ownership. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.appium.java_client.service.local; + +import lombok.Getter; + +import java.time.Duration; + +@Getter +public class AppiumServerConnectionTimeout extends RuntimeException { + private final Duration timeout; + + public AppiumServerConnectionTimeout(Duration timeout, Throwable cause) { + super(cause); + this.timeout = timeout; + } +} diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerHasNotBeenStartedLocallyException.java b/src/main/java/io/appium/java_client/service/local/AppiumServerHasNotBeenStartedLocallyException.java index 664e6a602..9c0afb248 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumServerHasNotBeenStartedLocallyException.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumServerHasNotBeenStartedLocallyException.java @@ -16,16 +16,16 @@ package io.appium.java_client.service.local; - public class AppiumServerHasNotBeenStartedLocallyException extends RuntimeException { + public AppiumServerHasNotBeenStartedLocallyException(String message, Throwable cause) { + super(message, cause); + } - private static final long serialVersionUID = 1L; - - public AppiumServerHasNotBeenStartedLocallyException(String messege, Throwable t) { - super(messege, t); + public AppiumServerHasNotBeenStartedLocallyException(String message) { + super(message); } - public AppiumServerHasNotBeenStartedLocallyException(String messege) { - super(messege); + public AppiumServerHasNotBeenStartedLocallyException(Throwable cause) { + super(cause); } } From 260327535b458cccd17d2dea8a9fdb7c3e8147e1 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 9 Mar 2024 08:56:46 +0100 Subject: [PATCH 2/8] Tune --- .../local/AppiumDriverLocalService.java | 27 ++++---- .../AppiumServerAvailabilityChecker.java | 69 +++++++++++++++++-- .../local/AppiumServerConnectionError.java | 61 ---------------- .../local/AppiumServerConnectionTimeout.java | 31 --------- 4 files changed, 79 insertions(+), 109 deletions(-) delete mode 100644 src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java delete mode 100644 src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index 027c6218e..e5d0d33f1 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -130,7 +130,8 @@ public boolean isRunning() { try { return ping(IS_RUNNING_PING_TIMEOUT); - } catch (AppiumServerConnectionTimeout | AppiumServerConnectionError e) { + } catch (AppiumServerAvailabilityChecker.ConnectionTimeout | + AppiumServerAvailabilityChecker.ConnectionError e) { return false; } catch (InterruptedException e) { throw new RuntimeException(e); @@ -186,7 +187,8 @@ public void start() throws AppiumServerHasNotBeenStartedLocallyException { try { ping(startupTimeout); didPingSucceed = true; - } catch (AppiumServerConnectionTimeout | AppiumServerConnectionError e) { + } catch (AppiumServerAvailabilityChecker.ConnectionTimeout | + AppiumServerAvailabilityChecker.ConnectionError e) { var errorLines = new ArrayList<>(generateDetailedErrorMessagePrefix(e)); errorLines.addAll(retrieveServerDebugInfo()); throw new AppiumServerHasNotBeenStartedLocallyException( @@ -206,24 +208,25 @@ public void start() throws AppiumServerHasNotBeenStartedLocallyException { private List generateDetailedErrorMessagePrefix(RuntimeException e) { var errorLines = new ArrayList(); - if (e instanceof AppiumServerConnectionTimeout) { + if (e instanceof AppiumServerAvailabilityChecker.ConnectionTimeout) { errorLines.add(String.format( "Appium HTTP server is not listening at %s after %s ms timeout. " + "Consider increasing the server startup timeout value and " + "check the server log for possible error messages.", - getUrl(), ((AppiumServerConnectionTimeout) e).getTimeout().toMillis() + getUrl(), + ((AppiumServerAvailabilityChecker.ConnectionTimeout) e).getTimeout().toMillis() )); - } else if (e instanceof AppiumServerConnectionError) { - var statusCode = ((AppiumServerConnectionError) e).getResponseCode(); - var statusUrl = ((AppiumServerConnectionError) e).getStatusUrl(); - var payload = ((AppiumServerConnectionError) e).getPayload(); + } else if (e instanceof AppiumServerAvailabilityChecker.ConnectionError) { + var statusCode = ((AppiumServerAvailabilityChecker.ConnectionError) e).getResponseCode(); + var statusUrl = ((AppiumServerAvailabilityChecker.ConnectionError) e).getStatusUrl(); + var payload = ((AppiumServerAvailabilityChecker.ConnectionError) e).getPayload(); errorLines.add(String.format( "Appium HTTP server has started and is listening although we were " + - "unable to get a successful response from %s. Make sure both client " + - "and server use the same base path and check the server log for possible " + - "error messages.", statusUrl + "unable to get an OK response from %s. Make sure both the client " + + "and the server use the same base path '%s' and check the server log for possible " + + "error messages.", statusUrl, Optional.ofNullable(basePath).orElse("/") )); - errorLines.add(String.format("HTTP status code: %s", statusCode)); + errorLines.add(String.format("Response status code: %s", statusCode)); payload.ifPresent(p -> errorLines.add(String.format("Response payload: %s", p))); } return errorLines; diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java index 2795240d3..c99dab3a4 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java @@ -16,11 +16,18 @@ package io.appium.java_client.service.local; +import lombok.Getter; + +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStream; +import java.io.InputStreamReader; import java.net.HttpURLConnection; import java.net.URL; import java.time.Duration; import java.time.Instant; +import java.util.LinkedList; +import java.util.List; import java.util.Optional; public class AppiumServerAvailabilityChecker { @@ -39,9 +46,9 @@ public class AppiumServerAvailabilityChecker { * immediately. * @return true in case of success * @throws InterruptedException If the API is interrupted - * @throws AppiumServerConnectionTimeout If it is not possible to successfully open - * an HTTP connection to the server /status endpoint. - * @throws AppiumServerConnectionError If an HTTP connection was opened successfully, + * @throws ConnectionTimeout If it is not possible to successfully open + * an HTTP connection to the server's /status endpoint. + * @throws ConnectionError If an HTTP connection was opened successfully, * but non-200 error code was received. */ public boolean waitUntilAvailable(URL serverStatusUrl, Duration timeout) throws InterruptedException { @@ -62,7 +69,7 @@ public boolean waitUntilAvailable(URL serverStatusUrl, Duration timeout) throws Thread.sleep(interval.toMillis()); interval = interval.compareTo(MAX_POLL_INTERVAL) >= 0 ? interval : interval.multipliedBy(2); } - throw new AppiumServerConnectionTimeout(timeout, lastError); + throw new ConnectionTimeout(timeout, lastError); } private HttpURLConnection connectToUrl(URL url) throws IOException { @@ -81,6 +88,58 @@ private boolean checkResponse(HttpURLConnection connection) throws IOException { var is = responseCode < HttpURLConnection.HTTP_BAD_REQUEST ? connection.getInputStream() : connection.getErrorStream(); - throw new AppiumServerConnectionError(connection.getURL(), responseCode, is); + throw new ConnectionError(connection.getURL(), responseCode, is); + } + + @Getter + public static class ConnectionError extends RuntimeException { + private static final int MAX_PAYLOAD_LEN = 500; + + private final URL statusUrl; + private final int responseCode; + private final Optional payload; + + public ConnectionError(URL statusUrl, int responseCode, InputStream body) { + super(ConnectionError.class.getSimpleName()); + this.statusUrl = statusUrl; + this.responseCode = responseCode; + this.payload = readResponseStreamSafely(body); + } + + private static Optional readResponseStreamSafely(InputStream is) { + try (var br = new BufferedReader(new InputStreamReader(is))) { + var result = new LinkedList(); + String currentLine; + var payloadSize = 0L; + while ((currentLine = br.readLine()) != null) { + result.addFirst(currentLine); + payloadSize += currentLine.length(); + while (payloadSize > MAX_PAYLOAD_LEN && result.size() > 1) { + payloadSize -= result.removeLast().length(); + } + } + var s = abbreviate(result); + return s.isEmpty() ? Optional.empty() : Optional.of(s); + } catch (IOException e) { + return Optional.empty(); + } + } + + private static String abbreviate(List filo) { + var result = String.join("\n", filo).trim(); + return result.length() > MAX_PAYLOAD_LEN + ? result.substring(0, MAX_PAYLOAD_LEN) + "…" + : result; + } + } + + @Getter + public static class ConnectionTimeout extends RuntimeException { + private final Duration timeout; + + public ConnectionTimeout(Duration timeout, Throwable cause) { + super(ConnectionTimeout.class.getSimpleName(), cause); + this.timeout = timeout; + } } } diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java b/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java deleted file mode 100644 index 161fb88a2..000000000 --- a/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionError.java +++ /dev/null @@ -1,61 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * See the NOTICE file distributed with this work for additional - * information regarding copyright ownership. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.appium.java_client.service.local; - -import lombok.Getter; - -import java.io.BufferedReader; -import java.io.IOException; -import java.io.InputStream; -import java.io.InputStreamReader; -import java.net.URL; -import java.util.Optional; - -@Getter -public class AppiumServerConnectionError extends RuntimeException { - private static final int MAX_PAYLOAD_LEN = 500; - - private final URL statusUrl; - private final int responseCode; - private final Optional payload; - - public AppiumServerConnectionError(URL statusUrl, int responseCode, InputStream body) { - super(AppiumServerConnectionError.class.getSimpleName()); - this.statusUrl = statusUrl; - this.responseCode = responseCode; - this.payload = readResponseStreamSafely(body); - } - - private static Optional readResponseStreamSafely(InputStream is) { - try (var br = new BufferedReader(new InputStreamReader(is))) { - var result = new StringBuilder(); - String currentLine; - var didCut = false; - while ((currentLine = br.readLine()) != null) { - result.append(currentLine).append("\n"); - if (result.toString().length() >= MAX_PAYLOAD_LEN) { - didCut = true; - break; - } - } - var s = didCut ? (result.toString().trim() + "…") : result.toString().trim(); - return s.trim().isEmpty() ? Optional.empty() : Optional.of(s); - } catch (IOException e) { - return Optional.empty(); - } - } -} diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java b/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java deleted file mode 100644 index f71428fcf..000000000 --- a/src/main/java/io/appium/java_client/service/local/AppiumServerConnectionTimeout.java +++ /dev/null @@ -1,31 +0,0 @@ -/* - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * See the NOTICE file distributed with this work for additional - * information regarding copyright ownership. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package io.appium.java_client.service.local; - -import lombok.Getter; - -import java.time.Duration; - -@Getter -public class AppiumServerConnectionTimeout extends RuntimeException { - private final Duration timeout; - - public AppiumServerConnectionTimeout(Duration timeout, Throwable cause) { - super(cause); - this.timeout = timeout; - } -} From 8cc3e26fd6cbd37d71e874f16b6a6f46c92d4522 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 9 Mar 2024 09:02:48 +0100 Subject: [PATCH 3/8] Checkstyle --- .../local/AppiumDriverLocalService.java | 22 +++++++++---------- .../AppiumServerAvailabilityChecker.java | 17 ++++++++++++-- 2 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index e5d0d33f1..cf09ce7b3 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -130,8 +130,8 @@ public boolean isRunning() { try { return ping(IS_RUNNING_PING_TIMEOUT); - } catch (AppiumServerAvailabilityChecker.ConnectionTimeout | - AppiumServerAvailabilityChecker.ConnectionError e) { + } catch (AppiumServerAvailabilityChecker.ConnectionTimeout + | AppiumServerAvailabilityChecker.ConnectionError e) { return false; } catch (InterruptedException e) { throw new RuntimeException(e); @@ -187,8 +187,8 @@ public void start() throws AppiumServerHasNotBeenStartedLocallyException { try { ping(startupTimeout); didPingSucceed = true; - } catch (AppiumServerAvailabilityChecker.ConnectionTimeout | - AppiumServerAvailabilityChecker.ConnectionError e) { + } catch (AppiumServerAvailabilityChecker.ConnectionTimeout + | AppiumServerAvailabilityChecker.ConnectionError e) { var errorLines = new ArrayList<>(generateDetailedErrorMessagePrefix(e)); errorLines.addAll(retrieveServerDebugInfo()); throw new AppiumServerHasNotBeenStartedLocallyException( @@ -210,9 +210,9 @@ private List generateDetailedErrorMessagePrefix(RuntimeException e) { var errorLines = new ArrayList(); if (e instanceof AppiumServerAvailabilityChecker.ConnectionTimeout) { errorLines.add(String.format( - "Appium HTTP server is not listening at %s after %s ms timeout. " + - "Consider increasing the server startup timeout value and " + - "check the server log for possible error messages.", + "Appium HTTP server is not listening at %s after %s ms timeout. " + + "Consider increasing the server startup timeout value and " + + "check the server log for possible error messages.", getUrl(), ((AppiumServerAvailabilityChecker.ConnectionTimeout) e).getTimeout().toMillis() )); @@ -221,10 +221,10 @@ private List generateDetailedErrorMessagePrefix(RuntimeException e) { var statusUrl = ((AppiumServerAvailabilityChecker.ConnectionError) e).getStatusUrl(); var payload = ((AppiumServerAvailabilityChecker.ConnectionError) e).getPayload(); errorLines.add(String.format( - "Appium HTTP server has started and is listening although we were " + - "unable to get an OK response from %s. Make sure both the client " + - "and the server use the same base path '%s' and check the server log for possible " + - "error messages.", statusUrl, Optional.ofNullable(basePath).orElse("/") + "Appium HTTP server has started and is listening although we were " + + "unable to get an OK response from %s. Make sure both the client " + + "and the server use the same base path '%s' and check the server log for possible " + + "error messages.", statusUrl, Optional.ofNullable(basePath).orElse("/") )); errorLines.add(String.format("Response status code: %s", statusCode)); payload.ifPresent(p -> errorLines.add(String.format("Response payload: %s", p))); diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java index c99dab3a4..3b4aee0cc 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java @@ -47,9 +47,9 @@ public class AppiumServerAvailabilityChecker { * @return true in case of success * @throws InterruptedException If the API is interrupted * @throws ConnectionTimeout If it is not possible to successfully open - * an HTTP connection to the server's /status endpoint. + * an HTTP connection to the server's /status endpoint. * @throws ConnectionError If an HTTP connection was opened successfully, - * but non-200 error code was received. + * but non-200 error code was received. */ public boolean waitUntilAvailable(URL serverStatusUrl, Duration timeout) throws InterruptedException { var interval = MIN_POLL_INTERVAL; @@ -99,6 +99,13 @@ public static class ConnectionError extends RuntimeException { private final int responseCode; private final Optional payload; + /** + * Thrown on server connection errors. + * + * @param statusUrl Appium server status URL. + * @param responseCode The response code received from the URL above. + * @param body The response body stream received from the URL above. + */ public ConnectionError(URL statusUrl, int responseCode, InputStream body) { super(ConnectionError.class.getSimpleName()); this.statusUrl = statusUrl; @@ -137,6 +144,12 @@ private static String abbreviate(List filo) { public static class ConnectionTimeout extends RuntimeException { private final Duration timeout; + /** + * Thrown on server timeout errors. + * + * @param timeout Timeout value. + * @param cause Timeout cause. + */ public ConnectionTimeout(Duration timeout, Throwable cause) { super(ConnectionTimeout.class.getSimpleName(), cause); this.timeout = timeout; From 07651442d30f53b8bd66a7149518ac1b03c464ca Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 9 Mar 2024 09:04:48 +0100 Subject: [PATCH 4/8] Tune --- .../service/local/AppiumDriverLocalService.java | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index cf09ce7b3..f55f3c8d7 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -17,6 +17,7 @@ package io.appium.java_client.service.local; import com.google.common.annotations.VisibleForTesting; +import lombok.Getter; import lombok.SneakyThrows; import org.openqa.selenium.os.ExternalProcess; import org.openqa.selenium.remote.service.DriverService; @@ -66,6 +67,7 @@ public final class AppiumDriverLocalService extends DriverService { private final ListOutputStream stream = new ListOutputStream().add(System.out); private final AppiumServerAvailabilityChecker availabilityChecker = new AppiumServerAvailabilityChecker(); private final URL url; + @Getter private String basePath; private ExternalProcess process = null; @@ -95,10 +97,6 @@ public AppiumDriverLocalService withBasePath(String basePath) { return this; } - public String getBasePath() { - return this.basePath; - } - @SneakyThrows private static URL addSuffix(URL url, String suffix) { return url.toURI().resolve("." + (suffix.startsWith("/") ? suffix : "/" + suffix)).toURL(); @@ -212,8 +210,7 @@ private List generateDetailedErrorMessagePrefix(RuntimeException e) { errorLines.add(String.format( "Appium HTTP server is not listening at %s after %s ms timeout. " + "Consider increasing the server startup timeout value and " - + "check the server log for possible error messages.", - getUrl(), + + "check the server log for possible error messages.", getUrl(), ((AppiumServerAvailabilityChecker.ConnectionTimeout) e).getTimeout().toMillis() )); } else if (e instanceof AppiumServerAvailabilityChecker.ConnectionError) { From 5f4365da7c706042c38c03743bad832aef28b52d Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 9 Mar 2024 11:02:18 +0100 Subject: [PATCH 5/8] ellipsis --- .../service/local/AppiumServerAvailabilityChecker.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java index 3b4aee0cc..2876c3707 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumServerAvailabilityChecker.java @@ -93,7 +93,7 @@ private boolean checkResponse(HttpURLConnection connection) throws IOException { @Getter public static class ConnectionError extends RuntimeException { - private static final int MAX_PAYLOAD_LEN = 500; + private static final int MAX_PAYLOAD_LEN = 1024; private final URL statusUrl; private final int responseCode; @@ -135,7 +135,7 @@ private static Optional readResponseStreamSafely(InputStream is) { private static String abbreviate(List filo) { var result = String.join("\n", filo).trim(); return result.length() > MAX_PAYLOAD_LEN - ? result.substring(0, MAX_PAYLOAD_LEN) + "…" + ? "…" + result.substring(0, MAX_PAYLOAD_LEN) : result; } } From 17300c04b7884da40a60a64100d415abb6d596c4 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 9 Mar 2024 11:12:56 +0100 Subject: [PATCH 6/8] Wording --- .../java_client/service/local/AppiumDriverLocalService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index f55f3c8d7..69225b526 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -236,7 +236,7 @@ private List retrieveServerDebugInfo() { ofNullable(process) .map(ExternalProcess::getOutput) .filter(o -> !isNullOrEmpty(o)) - .ifPresent(o -> result.add(String.format("Server process output: %s", o))); + .ifPresent(o -> result.add(String.format("Server log: %s", o))); return result; } From 29428bb85b2fd45e66a2ef47a040634fe607b6b4 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 9 Mar 2024 11:14:07 +0100 Subject: [PATCH 7/8] message --- .../java_client/service/local/AppiumDriverLocalService.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index 69225b526..043030632 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -210,7 +210,7 @@ private List generateDetailedErrorMessagePrefix(RuntimeException e) { errorLines.add(String.format( "Appium HTTP server is not listening at %s after %s ms timeout. " + "Consider increasing the server startup timeout value and " - + "check the server log for possible error messages.", getUrl(), + + "check the server log for possible error messages occurrences.", getUrl(), ((AppiumServerAvailabilityChecker.ConnectionTimeout) e).getTimeout().toMillis() )); } else if (e instanceof AppiumServerAvailabilityChecker.ConnectionError) { @@ -221,7 +221,7 @@ private List generateDetailedErrorMessagePrefix(RuntimeException e) { "Appium HTTP server has started and is listening although we were " + "unable to get an OK response from %s. Make sure both the client " + "and the server use the same base path '%s' and check the server log for possible " - + "error messages.", statusUrl, Optional.ofNullable(basePath).orElse("/") + + "error messages occurrences.", statusUrl, Optional.ofNullable(basePath).orElse("/") )); errorLines.add(String.format("Response status code: %s", statusCode)); payload.ifPresent(p -> errorLines.add(String.format("Response payload: %s", p))); From 70eec7e531070dfd8c073f5d97fd267b110882a0 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sun, 10 Mar 2024 13:07:34 +0100 Subject: [PATCH 8/8] fix --- .../service/local/AppiumDriverLocalService.java | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java index 043030632..666f2ba06 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java @@ -214,9 +214,10 @@ private List generateDetailedErrorMessagePrefix(RuntimeException e) { ((AppiumServerAvailabilityChecker.ConnectionTimeout) e).getTimeout().toMillis() )); } else if (e instanceof AppiumServerAvailabilityChecker.ConnectionError) { - var statusCode = ((AppiumServerAvailabilityChecker.ConnectionError) e).getResponseCode(); - var statusUrl = ((AppiumServerAvailabilityChecker.ConnectionError) e).getStatusUrl(); - var payload = ((AppiumServerAvailabilityChecker.ConnectionError) e).getPayload(); + var connectionError = (AppiumServerAvailabilityChecker.ConnectionError) e; + var statusCode = connectionError.getResponseCode(); + var statusUrl = connectionError.getStatusUrl(); + var payload = connectionError.getPayload(); errorLines.add(String.format( "Appium HTTP server has started and is listening although we were " + "unable to get an OK response from %s. Make sure both the client "