Skip to content

Commit a42ba4d

Browse files
mykola-mokhnachmykola-mokhnach
authored
chore: Improve the error message on service startup (#1928)
1 parent 6af53a0 commit a42ba4d

File tree

3 files changed

+46
-32
lines changed

3 files changed

+46
-32
lines changed

‎src/main/java/io/appium/java_client/service/local/AppiumDriverLocalService.java‎

Lines changed: 39 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import java.net.MalformedURLException;
3636
import java.net.URL;
3737
import java.time.Duration;
38+
import java.util.ArrayList;
3839
import java.util.List;
3940
import java.util.Map;
4041
import java.util.Optional;
@@ -46,7 +47,8 @@
4647
import java.util.regex.Pattern;
4748

4849
import static com.google.common.base.Preconditions.checkNotNull;
49-
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP_ADDRESS;
50+
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP4_ADDRESS;
51+
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP6_ADDRESS;
5052
import static org.slf4j.event.Level.DEBUG;
5153
import static org.slf4j.event.Level.INFO;
5254

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

6164
private final File nodeJSExec;
6265
private final List<String> nodeJSArgs;
@@ -106,7 +109,7 @@ private static URL addSuffix(URL url, String suffix) {
106109
@SneakyThrows
107110
@SuppressWarnings("SameParameterValue")
108111
private static URL replaceHost(URL source, String oldHost, String newHost) {
109-
return new URL(source.toString().replace(oldHost, newHost));
112+
return new URL(source.toString().replaceFirst(oldHost, newHost));
110113
}
111114

112115
/**
@@ -128,7 +131,7 @@ public boolean isRunning() {
128131
}
129132

130133
try {
131-
ping(Duration.ofMillis(1500));
134+
ping(IS_RUNNING_PING_TIMEOUT);
132135
return true;
133136
} catch (UrlChecker.TimeoutException e) {
134137
return false;
@@ -142,8 +145,15 @@ public boolean isRunning() {
142145
}
143146

144147
private void ping(Duration timeout) throws UrlChecker.TimeoutException, MalformedURLException {
145-
// The operating system might block direct access to the universal broadcast IP address
146-
URL status = addSuffix(replaceHost(getUrl(), BROADCAST_IP_ADDRESS, "127.0.0.1"), "/status");
148+
URL url = getUrl();
149+
String host = url.getHost();
150+
// The operating system will block direct access to the universal broadcast IP address
151+
if (host.equals(BROADCAST_IP4_ADDRESS)) {
152+
url = replaceHost(url, BROADCAST_IP4_ADDRESS, "127.0.0.1");
153+
} else if (host.equals(BROADCAST_IP6_ADDRESS)) {
154+
url = replaceHost(url, BROADCAST_IP6_ADDRESS, "::1");
155+
}
156+
URL status = addSuffix(url, "/status");
147157
new UrlChecker().waitUntilAvailable(timeout.toMillis(), TimeUnit.MILLISECONDS, status);
148158
}
149159

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

163173
try {
164-
process = new CommandLine(this.nodeJSExec.getCanonicalPath(),
165-
nodeJSArgs.toArray(new String[]{}));
174+
process = new CommandLine(
175+
this.nodeJSExec.getCanonicalPath(),
176+
nodeJSArgs.toArray(new String[]{})
177+
);
166178
process.setEnvironmentVariables(nodeJSEnvironment);
167179
process.copyOutputTo(stream);
168180
process.executeAsync();
169181
ping(startupTimeout);
170-
} catch (Throwable e) {
182+
} catch (Exception e) {
183+
final Optional<String> output = Optional.ofNullable(process)
184+
.map(CommandLine::getStdOut)
185+
.filter((o) -> !StringUtils.isBlank(o));
171186
destroyProcess();
172-
String msgTxt = "The local appium server has not been started. "
173-
+ "The given Node.js executable: " + this.nodeJSExec.getAbsolutePath()
174-
+ " Arguments: " + nodeJSArgs.toString() + " " + "\n";
175-
if (process != null) {
176-
String processStream = process.getStdOut();
177-
if (!StringUtils.isBlank(processStream)) {
178-
msgTxt = msgTxt + "Process output: " + processStream + "\n";
179-
}
187+
List<String> errorLines = new ArrayList<>();
188+
errorLines.add("The local appium server has not been started");
189+
errorLines.add(String.format("Reason: %s", e.getMessage()));
190+
if (e instanceof UrlChecker.TimeoutException) {
191+
errorLines.add(String.format(
192+
"Consider increasing the server startup timeout value (currently %sms)",
193+
startupTimeout.toMillis()
194+
));
180195
}
181-
182-
throw new AppiumServerHasNotBeenStartedLocallyException(msgTxt, e);
196+
errorLines.add(
197+
String.format("Node.js executable path: %s", nodeJSExec.getAbsolutePath())
198+
);
199+
errorLines.add(String.format("Arguments: %s", nodeJSArgs));
200+
output.ifPresent((o) -> errorLines.add(String.format("Output: %s", o)));
201+
throw new AppiumServerHasNotBeenStartedLocallyException(
202+
StringUtils.joinWith("\n", errorLines), e
203+
);
183204
}
184205
} finally {
185206
lock.unlock();

‎src/main/java/io/appium/java_client/service/local/AppiumServiceBuilder.java‎

Lines changed: 4 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.apache.commons.io.IOUtils;
3030
import org.apache.commons.lang3.StringUtils;
3131
import org.apache.commons.lang3.SystemUtils;
32-
import org.apache.commons.validator.routines.InetAddressValidator;
3332
import org.openqa.selenium.Capabilities;
3433
import org.openqa.selenium.Platform;
3534
import org.openqa.selenium.os.ExecutableFinder;
@@ -72,13 +71,14 @@ public final class AppiumServiceBuilder
7271
*/
7372
private static final String NODE_PATH = "NODE_BINARY_PATH";
7473

75-
public static final String BROADCAST_IP_ADDRESS = "0.0.0.0";
74+
public static final String BROADCAST_IP4_ADDRESS = "0.0.0.0";
75+
public static final String BROADCAST_IP6_ADDRESS = "::";
7676
private static final Path APPIUM_PATH_SUFFIX = Paths.get("appium", "build", "lib", "main.js");
7777
public static final int DEFAULT_APPIUM_PORT = 4723;
7878
private final Map<String, String> serverArguments = new HashMap<>();
7979
private File appiumJS;
8080
private File node;
81-
private String ipAddress = BROADCAST_IP_ADDRESS;
81+
private String ipAddress = BROADCAST_IP4_ADDRESS;
8282
private Capabilities capabilities;
8383
private boolean autoQuoteCapabilitiesOnWindows = false;
8484
private static final Function<File, String> APPIUM_JS_NOT_EXIST_ERROR = (fullPath) -> String.format(
@@ -363,14 +363,7 @@ protected ImmutableList<String> createArgs() {
363363
argList.add(String.valueOf(getPort()));
364364

365365
if (StringUtils.isBlank(ipAddress)) {
366-
ipAddress = BROADCAST_IP_ADDRESS;
367-
} else {
368-
InetAddressValidator validator = InetAddressValidator.getInstance();
369-
if (!validator.isValid(ipAddress) && !validator.isValidInet4Address(ipAddress)
370-
&& !validator.isValidInet6Address(ipAddress)) {
371-
throw new IllegalArgumentException(
372-
"The invalid IP address " + ipAddress + " is defined");
373-
}
366+
ipAddress = BROADCAST_IP4_ADDRESS;
374367
}
375368
argList.add("--address");
376369
argList.add(ipAddress);

‎src/test/java/io/appium/java_client/service/local/ServerBuilderTest.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
import static io.appium.java_client.TestUtils.getLocalIp4Address;
2020
import static io.appium.java_client.service.local.AppiumDriverLocalService.buildDefaultService;
2121
import static io.appium.java_client.service.local.AppiumServiceBuilder.APPIUM_PATH;
22-
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP_ADDRESS;
22+
import static io.appium.java_client.service.local.AppiumServiceBuilder.BROADCAST_IP4_ADDRESS;
2323
import static io.appium.java_client.service.local.AppiumServiceBuilder.DEFAULT_APPIUM_PORT;
2424
import static io.appium.java_client.service.local.flags.GeneralServerFlag.BASEPATH;
2525
import static io.appium.java_client.service.local.flags.GeneralServerFlag.CALLBACK_ADDRESS;
@@ -323,7 +323,7 @@ void checkAbilityToStartServiceUsingValidBasePathWithMultiplePathParams() {
323323
service = new AppiumServiceBuilder().withArgument(BASEPATH, basePath).build();
324324
service.start();
325325
assertTrue(service.isRunning());
326-
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP_ADDRESS, DEFAULT_APPIUM_PORT);
326+
String baseUrl = String.format("http://%s:%d/", BROADCAST_IP4_ADDRESS, DEFAULT_APPIUM_PORT);
327327
assertEquals(baseUrl + basePath + "/", service.getUrl().toString());
328328
}
329329

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

0 commit comments

Comments
 (0)