Skip to content

chore: Improve the error message on service startup #1928

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mykola-mokhnach merged 3 commits into from
May 29, 2023
Merged

chore: Improve the error message on service startup #1928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1ad95ba
chore: Improve the error message on service startup
mykola-mokhnach May 29, 2023
2f31d39
style
mykola-mokhnach May 29, 2023
22be23e
Tune filtering
mykola-mokhnach May 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 39 additions & 18 deletions src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
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;
Expand All @@ -46,7 +47,8 @@
import java.util.regex.Pattern;

import static com.google.common.base.Preconditions.checkNotNull;
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP_ADDRESS;
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP4_ADDRESS;
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP6_ADDRESS;
import static org.slf4j.event.Level.DEBUG;
import static org.slf4j.event.Level.INFO;

Expand All @@ -57,6 +59,7 @@ public final class AppiumDriverLocalService extends DriverService {
private static final Pattern LOGGER_CONTEXT_PATTERN = Pattern.compile("^(\\[debug\\] )?\\[(.+?)\\]");
private static final String APPIUM_SERVICE_SLF4J_LOGGER_PREFIX = "appium.service";
private static final Duration DESTROY_TIMEOUT = Duration.ofSeconds(60);
private static final Duration IS_RUNNING_PING_TIMEOUT = Duration.ofMillis(1500);

private final File nodeJSExec;
private final List<String> nodeJSArgs;
Expand Down Expand Up @@ -106,7 +109,7 @@ private static URL addSuffix(URL url, String suffix) {
@SneakyThrows
@SuppressWarnings("SameParameterValue")
private static URL replaceHost(URL source, String oldHost, String newHost) {
return new URL(source.toString().replace(oldHost, newHost));
return new URL(source.toString().replaceFirst(oldHost, newHost));
}

/**
Expand All @@ -128,7 +131,7 @@ public boolean isRunning() {
}

try {
ping(Duration.ofMillis(1500));
ping(IS_RUNNING_PING_TIMEOUT);
return true;
} catch (UrlChecker.TimeoutException e) {
return false;
Expand All @@ -142,8 +145,15 @@ public boolean isRunning() {
}

private void ping(Duration timeout) throws UrlChecker.TimeoutException, MalformedURLException {
// The operating system might block direct access to the universal broadcast IP address
URL status = addSuffix(replaceHost(getUrl(), BROADCAST_IP_ADDRESS, "127.0.0.1"), "/status");
URL url = getUrl();
String host = url.getHost();
// The operating system will block direct access to the universal broadcast IP address
if (host.equals(BROADCAST_IP4_ADDRESS)) {
url = replaceHost(url, BROADCAST_IP4_ADDRESS, "127.0.0.1");
} else if (host.equals(BROADCAST_IP6_ADDRESS)) {
url = replaceHost(url, BROADCAST_IP6_ADDRESS, "::1");
}
URL status = addSuffix(url, "/status");
new UrlChecker().waitUntilAvailable(timeout.toMillis(), TimeUnit.MILLISECONDS, status);
}

Expand All @@ -161,25 +171,36 @@ public void start() throws AppiumServerHasNotBeenStartedLocallyException {
}

try {
process = new CommandLine(this.nodeJSExec.getCanonicalPath(),
nodeJSArgs.toArray(new String[]{}));
process = new CommandLine(
this.nodeJSExec.getCanonicalPath(),
nodeJSArgs.toArray(new String[]{})
);
process.setEnvironmentVariables(nodeJSEnvironment);
process.copyOutputTo(stream);
process.executeAsync();
ping(startupTimeout);
} catch (Throwable e) {
} catch (Exception e) {
final Optional<String> output = Optional.ofNullable(process)
.map(CommandLine::getStdOut)
.filter((o) -> !StringUtils.isBlank(o));
destroyProcess();
String msgTxt = "The local appium server has not been started. "
+ "The given Node.js executable: " + this.nodeJSExec.getAbsolutePath()
+ " Arguments: " + nodeJSArgs.toString() + " " + "\n";
if (process != null) {
String processStream = process.getStdOut();
if (!StringUtils.isBlank(processStream)) {
msgTxt = msgTxt + "Process output: " + processStream + "\n";
}
List<String> 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()
));
}

throw new AppiumServerHasNotBeenStartedLocallyException(msgTxt, e);
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)));
throw new AppiumServerHasNotBeenStartedLocallyException(
StringUtils.joinWith("\n", errorLines), e
);
}
} finally {
lock.unlock();
Expand Down
15 changes: 4 additions & 11 deletions src/main/java/io/appium/java_client/service/local/AppiumServiceBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.commons.lang3.SystemUtils;
import org.apache.commons.validator.routines.InetAddressValidator;
import org.openqa.selenium.Capabilities;
import org.openqa.selenium.Platform;
import org.openqa.selenium.os.ExecutableFinder;
Expand Down Expand Up @@ -72,13 +71,14 @@ public final class AppiumServiceBuilder
*/
private static final String NODE_PATH = "NODE_BINARY_PATH";

public static final String BROADCAST_IP_ADDRESS = "0.0.0.0";
public static final String BROADCAST_IP4_ADDRESS = "0.0.0.0";
public static final String BROADCAST_IP6_ADDRESS = "::";
private static final Path APPIUM_PATH_SUFFIX = Paths.get("appium", "build", "lib", "main.js");
public static final int DEFAULT_APPIUM_PORT = 4723;
private final Map<String, String> serverArguments = new HashMap<>();
private File appiumJS;
private File node;
private String ipAddress = BROADCAST_IP_ADDRESS;
private String ipAddress = BROADCAST_IP4_ADDRESS;
private Capabilities capabilities;
private boolean autoQuoteCapabilitiesOnWindows = false;
private static final Function<File, String> APPIUM_JS_NOT_EXIST_ERROR = (fullPath) -> String.format(
Expand Down Expand Up @@ -363,14 +363,7 @@ protected ImmutableList<String> createArgs() {
argList.add(String.valueOf(getPort()));

if (StringUtils.isBlank(ipAddress)) {
ipAddress = BROADCAST_IP_ADDRESS;
} else {
InetAddressValidator validator = InetAddressValidator.getInstance();
if (!validator.isValid(ipAddress) && !validator.isValidInet4Address(ipAddress)
&& !validator.isValidInet6Address(ipAddress)) {
throw new IllegalArgumentException(
"The invalid IP address " + ipAddress + " is defined");
}
ipAddress = BROADCAST_IP4_ADDRESS;
}
argList.add("--address");
argList.add(ipAddress);
Expand Down
6 changes: 3 additions & 3 deletions src/test/java/io/appium/java_client/service/local/ServerBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import static io.appium.java_client.TestUtils.getLocalIp4Address;
import static io.appium.java_client.service.local.AppiumDriverLocalService.buildDefaultService;
import static io.appium.java_client.service.local.AppiumServiceBuilder.APPIUM_PATH;
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP_ADDRESS;
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP4_ADDRESS;
import static io.appium.java_client.service.local.AppiumServiceBuilder.DEFAULT_APPIUM_PORT;
import static io.appium.java_client.service.local.flags.GeneralServerFlag.BASEPATH;
import static io.appium.java_client.service.local.flags.GeneralServerFlag.CALLBACK_ADDRESS;
Expand Down Expand Up @@ -323,7 +323,7 @@ void checkAbilityToStartServiceUsingValidBasePathWithMultiplePathParams() {
service = new AppiumServiceBuilder().withArgument(BASEPATH, basePath).build();
service.start();
assertTrue(service.isRunning());
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP_ADDRESS, DEFAULT_APPIUM_PORT);
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP4_ADDRESS, DEFAULT_APPIUM_PORT);
assertEquals(baseUrl + basePath + "/", service.getUrl().toString());
}

Expand All @@ -333,7 +333,7 @@ void checkAbilityToStartServiceUsingValidBasePathWithSinglePathParams() {
service = new AppiumServiceBuilder().withArgument(BASEPATH, basePath).build();
service.start();
assertTrue(service.isRunning());
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP_ADDRESS, DEFAULT_APPIUM_PORT);
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP4_ADDRESS, DEFAULT_APPIUM_PORT);
assertEquals(baseUrl + basePath.substring(1), service.getUrl().toString());
}

Expand Down