Skip to content

[DE-120] Bugfix swallowing connection exceptions #420

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.

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
merged 3 commits into from
Dec 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

<groupId>com.arangodb</groupId>
<artifactId>arangodb-java-driver</artifactId>
<version>6.14.0</version>
<version>6.15.0-SNAPSHOT</version>
<inceptionYear>2016</inceptionYear>
<packaging>jar</packaging>

Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/arangodb/ArangoDBException.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ public ArangoDBException(final Throwable cause) {
this.responseCode = null;
}

public ArangoDBException(final String message, final Throwable cause) {
super(message, cause);
this.entity = null;
this.responseCode = null;
}

/**
* @return ArangoDB error message
*/
Expand Down
44 changes: 44 additions & 0 deletions src/main/java/com/arangodb/ArangoDBMultipleException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package com.arangodb;

import java.util.List;
import java.util.Objects;
import java.util.StringJoiner;

public class ArangoDBMultipleException extends RuntimeException {

private final List<Throwable> exceptions;

public ArangoDBMultipleException(List<Throwable> exceptions) {
super();
this.exceptions = exceptions;
}

public List<Throwable> getExceptions() {
return exceptions;
}

@Override
public boolean equals(Object o) {
if (this == o) return true;
if (o == null || getClass() != o.getClass()) return false;
ArangoDBMultipleException that = (ArangoDBMultipleException) o;
return Objects.equals(exceptions, that.exceptions);
}

@Override
public int hashCode() {
return Objects.hash(exceptions);
}

@Override
public String toString() {
StringJoiner joiner = new StringJoiner("\n\t", "ArangoDBMultipleException{\n\t", "\n}");
for (Throwable t : exceptions) {
StringJoiner tJoiner = new StringJoiner("\n\t\t", "\n\t\t", "");
for (StackTraceElement stackTraceElement : t.getStackTrace())
tJoiner.add("at " + stackTraceElement);
joiner.add(t + tJoiner.toString());
}
return joiner.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ protected CompletableFuture<Response> execute(final Request request, final VstCo
final String location = e.getLocation();
final HostDescription redirectHost = HostUtils.createFromLocation(location);
hostHandler.closeCurrentOnError();
hostHandler.fail();
hostHandler.fail(e);
execute(request, new HostHandle().setHost(redirectHost), attemptCount + 1)
.whenComplete((v, err) -> {
if (v != null) {
Expand Down
16 changes: 8 additions & 8 deletions src/main/java/com/arangodb/internal/http/HttpCommunication.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@

import java.io.Closeable;
import java.io.IOException;
import java.net.SocketException;

/**
* @author Mark Vollmary
Expand Down Expand Up @@ -71,11 +70,11 @@ public void close() throws IOException {
hostHandler.close();
}

public Response execute(final Request request, final HostHandle hostHandle) throws ArangoDBException, IOException {
public Response execute(final Request request, final HostHandle hostHandle) throws ArangoDBException {
return execute(request, hostHandle, 0);
}

private Response execute(final Request request, final HostHandle hostHandle, final int attemptCount) throws ArangoDBException, IOException {
private Response execute(final Request request, final HostHandle hostHandle, final int attemptCount) throws ArangoDBException {
final AccessType accessType = RequestUtils.determineAccessType(request);
Host host = hostHandler.get(hostHandle, accessType);
try {
Expand All @@ -86,19 +85,20 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
hostHandler.success();
hostHandler.confirm();
return response;
} catch (final SocketException se) {
hostHandler.fail();
} catch (final IOException e) {
hostHandler.fail(e);
if (hostHandle != null && hostHandle.getHost() != null) {
hostHandle.setHost(null);
}
final Host failedHost = host;
host = hostHandler.get(hostHandle, accessType);
if (host != null) {
LOGGER.warn(String.format("Could not connect to %s", failedHost.getDescription()), se);
LOGGER.warn(String.format("Could not connect to %s", failedHost.getDescription()), e);
LOGGER.warn(String.format("Could not connect to %s. Try connecting to %s",
failedHost.getDescription(), host.getDescription()));
} else {
throw se;
LOGGER.error(e.getMessage(), e);
throw new ArangoDBException(e);
}
}
}
Expand All @@ -107,7 +107,7 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
final String location = ((ArangoDBRedirectException) e).getLocation();
final HostDescription redirectHost = HostUtils.createFromLocation(location);
hostHandler.closeCurrentOnError();
hostHandler.fail();
hostHandler.fail(e);
return execute(request, new HostHandle().setHost(redirectHost), attemptCount + 1);
} else {
throw e;
Expand Down
6 changes: 1 addition & 5 deletions src/main/java/com/arangodb/internal/http/HttpProtocol.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,7 @@ public HttpProtocol(final HttpCommunication httpCommunitaction) {

@Override
public Response execute(final Request request, final HostHandle hostHandle) throws ArangoDBException {
try {
return httpCommunitaction.execute(request, hostHandle);
} catch (final IOException e) {
throw new ArangoDBException(e);
}
return httpCommunitaction.execute(request, hostHandle);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ public void success() {
}

@Override
public void fail() {
determineHostHandler().fail();
public void fail(Exception exception) {
determineHostHandler().fail(exception);
}

@Override
Expand Down
16 changes: 12 additions & 4 deletions src/main/java/com/arangodb/internal/net/FallbackHostHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
package com.arangodb.internal.net;

import com.arangodb.ArangoDBException;
import com.arangodb.ArangoDBMultipleException;

import java.util.ArrayList;
import java.util.List;

/**
Expand All @@ -33,12 +35,14 @@ public class FallbackHostHandler implements HostHandler {
private Host current;
private Host lastSuccess;
private int iterations;
private final List<Throwable> lastFailExceptions;
private boolean firstOpened;
private HostSet hosts;

public FallbackHostHandler(final HostResolver resolver) {
this.resolver = resolver;
iterations = 0;
lastFailExceptions = new ArrayList<>();
reset();
hosts = resolver.resolve(true, false);
current = lastSuccess = hosts.getHostsList().get(0);
firstOpened = true;
Expand All @@ -49,19 +53,21 @@ public Host get(final HostHandle hostHandle, AccessType accessType) {
if (current != lastSuccess || iterations < 3) {
return current;
} else {
ArangoDBException e = new ArangoDBException("Cannot contact any host!",
new ArangoDBMultipleException(new ArrayList<>(lastFailExceptions)));
reset();
throw new ArangoDBException("Cannot contact any host!");
throw e;
}
}

@Override
public void success() {
lastSuccess = current;
iterations = 0;
reset();
}

@Override
public void fail() {
public void fail(Exception exception) {
hosts = resolver.resolve(false, false);
final List<Host> hostList = hosts.getHostsList();
final int index = hostList.indexOf(current) + 1;
Expand All @@ -70,11 +76,13 @@ public void fail() {
if (!inBound) {
iterations++;
}
lastFailExceptions.add(exception);
}

@Override
public void reset() {
iterations = 0;
lastFailExceptions.clear();
}

@Override
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/com/arangodb/internal/net/HostHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public interface HostHandler {

void success();

void fail();
void fail(Exception exception);

void reset();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ public void success() {
}

@Override
public void fail() {
fallback.fail();
public void fail(Exception exception) {
fallback.fail(exception);
current = fallback.get(null, null);
}

Expand Down
18 changes: 14 additions & 4 deletions src/main/java/com/arangodb/internal/net/RoundRobinHostHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
package com.arangodb.internal.net;

import com.arangodb.ArangoDBException;
import com.arangodb.ArangoDBMultipleException;

import java.util.ArrayList;
import java.util.List;

/**
* @author Mark Vollmary
Expand All @@ -30,15 +34,17 @@ public class RoundRobinHostHandler implements HostHandler {
private final HostResolver resolver;
private int current;
private int fails;
private final List<Exception> lastFailExceptions;
private Host currentHost;
private HostSet hosts;

public RoundRobinHostHandler(final HostResolver resolver) {
super();
this.resolver = resolver;
lastFailExceptions = new ArrayList<>();
hosts = resolver.resolve(true, false);
current = 0;
fails = 0;
reset();
}

@Override
Expand All @@ -47,8 +53,10 @@ public Host get(final HostHandle hostHandle, AccessType accessType) {
final int size = hosts.getHostsList().size();

if (fails > size) {
ArangoDBException e = new ArangoDBException("Cannot contact any host!",
new ArangoDBMultipleException(new ArrayList<>(lastFailExceptions)));
reset();
throw new ArangoDBException("Cannot contact any host!");
throw e;
}

final int index = (current++) % size;
Expand All @@ -72,17 +80,19 @@ public Host get(final HostHandle hostHandle, AccessType accessType) {

@Override
public void success() {
fails = 0;
reset();
}

@Override
public void fail() {
public void fail(Exception exception) {
fails++;
lastFailExceptions.add(exception);
}

@Override
public void reset() {
fails = 0;
lastFailExceptions.clear();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,13 @@ protected synchronized C connect(final HostHandle hostHandle, final AccessType a
hostHandler.confirm();
if (!connection.isOpen()) {
// see https://github.com/arangodb/arangodb-java-driver/issues/384
hostHandler.fail();
hostHandler.fail(new IOException("The connection is closed."));
host = hostHandler.get(hostHandle, accessType);
continue;
}
return connection;
} catch (final IOException e) {
hostHandler.fail();
hostHandler.fail(e);
if (hostHandle != null && hostHandle.getHost() != null) {
hostHandle.setHost(null);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ protected Response execute(final Request request, final VstConnectionSync connec
final String location = e.getLocation();
final HostDescription redirectHost = HostUtils.createFromLocation(location);
hostHandler.closeCurrentOnError();
hostHandler.fail();
hostHandler.fail(e);
return execute(request, new HostHandle().setHost(redirectHost), attemptCount + 1);
}
}
Expand Down
8 changes: 5 additions & 3 deletions src/test/java/com/arangodb/ArangoSslTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.TrustManagerFactory;
import java.security.KeyStore;
import java.util.List;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.*;
import static org.junit.Assert.fail;

/**
Expand Down Expand Up @@ -84,7 +84,9 @@ public void connectWithoutValidSslContext() {
arangoDB.getVersion();
fail("this should fail");
} catch (final ArangoDBException ex) {
assertThat(ex.getCause() instanceof SSLHandshakeException, is(true));
assertThat(ex.getCause(), is(instanceOf(ArangoDBMultipleException.class)));
List<Throwable> exceptions = ((ArangoDBMultipleException) ex.getCause()).getExceptions();
exceptions.forEach(e -> assertThat(e, is(instanceOf(SSLHandshakeException.class))));
}
}

Expand Down
Loading