Skip to content

Commit 6142b59

Browse files
rashtaorashtao
authored
[DE-120] Bugfix swallowing connection exceptions (#420)
* keep track of previous exceptions in HostHandler * fixed IOException handling in HTTP protocol * SSL test
1 parent de182c5 commit 6142b59

15 files changed

+121
-44
lines changed

‎pom.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
<groupId>com.arangodb</groupId>
77
<artifactId>arangodb-java-driver</artifactId>
8-
<version>6.14.0</version>
8+
<version>6.15.0-SNAPSHOT</version>
99
<inceptionYear>2016</inceptionYear>
1010
<packaging>jar</packaging>
1111

‎src/main/java/com/arangodb/ArangoDBException.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ public ArangoDBException(final Throwable cause) {
5656
this.responseCode = null;
5757
}
5858

59+
public ArangoDBException(final String message, final Throwable cause) {
60+
super(message, cause);
61+
this.entity = null;
62+
this.responseCode = null;
63+
}
64+
5965
/**
6066
* @return ArangoDB error message
6167
*/

‎src/main/java/com/arangodb/ArangoDBMultipleException.java

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
package com.arangodb;
2+
3+
import java.util.List;
4+
import java.util.Objects;
5+
import java.util.StringJoiner;
6+
7+
public class ArangoDBMultipleException extends RuntimeException {
8+
9+
private final List<Throwable> exceptions;
10+
11+
public ArangoDBMultipleException(List<Throwable> exceptions) {
12+
super();
13+
this.exceptions = exceptions;
14+
}
15+
16+
public List<Throwable> getExceptions() {
17+
return exceptions;
18+
}
19+
20+
@Override
21+
public boolean equals(Object o) {
22+
if (this == o) return true;
23+
if (o == null || getClass() != o.getClass()) return false;
24+
ArangoDBMultipleException that = (ArangoDBMultipleException) o;
25+
return Objects.equals(exceptions, that.exceptions);
26+
}
27+
28+
@Override
29+
public int hashCode() {
30+
return Objects.hash(exceptions);
31+
}
32+
33+
@Override
34+
public String toString() {
35+
StringJoiner joiner = new StringJoiner("\n\t", "ArangoDBMultipleException{\n\t", "\n}");
36+
for (Throwable t : exceptions) {
37+
StringJoiner tJoiner = new StringJoiner("\n\t\t", "\n\t\t", "");
38+
for (StackTraceElement stackTraceElement : t.getStackTrace())
39+
tJoiner.add("at " + stackTraceElement);
40+
joiner.add(t + tJoiner.toString());
41+
}
42+
return joiner.toString();
43+
}
44+
}

‎src/main/java/com/arangodb/async/internal/velocystream/VstCommunicationAsync.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ protected CompletableFuture<Response> execute(final Request request, final VstCo
8585
final String location = e.getLocation();
8686
final HostDescription redirectHost = HostUtils.createFromLocation(location);
8787
hostHandler.closeCurrentOnError();
88-
hostHandler.fail();
88+
hostHandler.fail(e);
8989
execute(request, new HostHandle().setHost(redirectHost), attemptCount + 1)
9090
.whenComplete((v, err) -> {
9191
if (v != null) {

‎src/main/java/com/arangodb/internal/http/HttpCommunication.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232

3333
import java.io.Closeable;
3434
import java.io.IOException;
35-
import java.net.SocketException;
3635

3736
/**
3837
* @author Mark Vollmary
@@ -71,11 +70,11 @@ public void close() throws IOException {
7170
hostHandler.close();
7271
}
7372

74-
public Response execute(final Request request, final HostHandle hostHandle) throws ArangoDBException, IOException {
73+
public Response execute(final Request request, final HostHandle hostHandle) throws ArangoDBException {
7574
return execute(request, hostHandle, 0);
7675
}
7776

78-
private Response execute(final Request request, final HostHandle hostHandle, final int attemptCount) throws ArangoDBException, IOException {
77+
private Response execute(final Request request, final HostHandle hostHandle, final int attemptCount) throws ArangoDBException {
7978
final AccessType accessType = RequestUtils.determineAccessType(request);
8079
Host host = hostHandler.get(hostHandle, accessType);
8180
try {
@@ -86,19 +85,20 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
8685
hostHandler.success();
8786
hostHandler.confirm();
8887
return response;
89-
} catch (final SocketException se) {
90-
hostHandler.fail();
88+
} catch (final IOException e) {
89+
hostHandler.fail(e);
9190
if (hostHandle != null && hostHandle.getHost() != null) {
9291
hostHandle.setHost(null);
9392
}
9493
final Host failedHost = host;
9594
host = hostHandler.get(hostHandle, accessType);
9695
if (host != null) {
97-
LOGGER.warn(String.format("Could not connect to %s", failedHost.getDescription()), se);
96+
LOGGER.warn(String.format("Could not connect to %s", failedHost.getDescription()), e);
9897
LOGGER.warn(String.format("Could not connect to %s. Try connecting to %s",
9998
failedHost.getDescription(), host.getDescription()));
10099
} else {
101-
throw se;
100+
LOGGER.error(e.getMessage(), e);
101+
throw new ArangoDBException(e);
102102
}
103103
}
104104
}
@@ -107,7 +107,7 @@ private Response execute(final Request request, final HostHandle hostHandle, fin
107107
final String location = ((ArangoDBRedirectException) e).getLocation();
108108
final HostDescription redirectHost = HostUtils.createFromLocation(location);
109109
hostHandler.closeCurrentOnError();
110-
hostHandler.fail();
110+
hostHandler.fail(e);
111111
return execute(request, new HostHandle().setHost(redirectHost), attemptCount + 1);
112112
} else {
113113
throw e;

‎src/main/java/com/arangodb/internal/http/HttpProtocol.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,7 @@ public HttpProtocol(final HttpCommunication httpCommunitaction) {
4242

4343
@Override
4444
public Response execute(final Request request, final HostHandle hostHandle) throws ArangoDBException {
45-
try {
46-
return httpCommunitaction.execute(request, hostHandle);
47-
} catch (final IOException e) {
48-
throw new ArangoDBException(e);
49-
}
45+
return httpCommunitaction.execute(request, hostHandle);
5046
}
5147

5248
@Override

‎src/main/java/com/arangodb/internal/net/DirtyReadHostHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ public void success() {
5656
}
5757

5858
@Override
59-
public void fail() {
60-
determineHostHandler().fail();
59+
public void fail(Exception exception) {
60+
determineHostHandler().fail(exception);
6161
}
6262

6363
@Override

‎src/main/java/com/arangodb/internal/net/FallbackHostHandler.java

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
package com.arangodb.internal.net;
2222

2323
import com.arangodb.ArangoDBException;
24+
import com.arangodb.ArangoDBMultipleException;
2425

26+
import java.util.ArrayList;
2527
import java.util.List;
2628

2729
/**
@@ -33,12 +35,14 @@ public class FallbackHostHandler implements HostHandler {
3335
private Host current;
3436
private Host lastSuccess;
3537
private int iterations;
38+
private final List<Throwable> lastFailExceptions;
3639
private boolean firstOpened;
3740
private HostSet hosts;
3841

3942
public FallbackHostHandler(final HostResolver resolver) {
4043
this.resolver = resolver;
41-
iterations = 0;
44+
lastFailExceptions = new ArrayList<>();
45+
reset();
4246
hosts = resolver.resolve(true, false);
4347
current = lastSuccess = hosts.getHostsList().get(0);
4448
firstOpened = true;
@@ -49,19 +53,21 @@ public Host get(final HostHandle hostHandle, AccessType accessType) {
4953
if (current != lastSuccess || iterations < 3) {
5054
return current;
5155
} else {
56+
ArangoDBException e = new ArangoDBException("Cannot contact any host!",
57+
new ArangoDBMultipleException(new ArrayList<>(lastFailExceptions)));
5258
reset();
53-
throw new ArangoDBException("Cannot contact any host!");
59+
throw e;
5460
}
5561
}
5662

5763
@Override
5864
public void success() {
5965
lastSuccess = current;
60-
iterations = 0;
66+
reset();
6167
}
6268

6369
@Override
64-
public void fail() {
70+
public void fail(Exception exception) {
6571
hosts = resolver.resolve(false, false);
6672
final List<Host> hostList = hosts.getHostsList();
6773
final int index = hostList.indexOf(current) + 1;
@@ -70,11 +76,13 @@ public void fail() {
7076
if (!inBound) {
7177
iterations++;
7278
}
79+
lastFailExceptions.add(exception);
7380
}
7481

7582
@Override
7683
public void reset() {
7784
iterations = 0;
85+
lastFailExceptions.clear();
7886
}
7987

8088
@Override

‎src/main/java/com/arangodb/internal/net/HostHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public interface HostHandler {
3131

3232
void success();
3333

34-
void fail();
34+
void fail(Exception exception);
3535

3636
void reset();
3737

‎src/main/java/com/arangodb/internal/net/RandomHostHandler.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ public void success() {
5454
}
5555

5656
@Override
57-
public void fail() {
58-
fallback.fail();
57+
public void fail(Exception exception) {
58+
fallback.fail(exception);
5959
current = fallback.get(null, null);
6060
}
6161

‎src/main/java/com/arangodb/internal/net/RoundRobinHostHandler.java

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,10 @@
2121
package com.arangodb.internal.net;
2222

2323
import com.arangodb.ArangoDBException;
24+
import com.arangodb.ArangoDBMultipleException;
25+
26+
import java.util.ArrayList;
27+
import java.util.List;
2428

2529
/**
2630
* @author Mark Vollmary
@@ -30,15 +34,17 @@ public class RoundRobinHostHandler implements HostHandler {
3034
private final HostResolver resolver;
3135
private int current;
3236
private int fails;
37+
private final List<Exception> lastFailExceptions;
3338
private Host currentHost;
3439
private HostSet hosts;
3540

3641
public RoundRobinHostHandler(final HostResolver resolver) {
3742
super();
3843
this.resolver = resolver;
44+
lastFailExceptions = new ArrayList<>();
3945
hosts = resolver.resolve(true, false);
4046
current = 0;
41-
fails = 0;
47+
reset();
4248
}
4349

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

4955
if (fails > size) {
56+
ArangoDBException e = new ArangoDBException("Cannot contact any host!",
57+
new ArangoDBMultipleException(new ArrayList<>(lastFailExceptions)));
5058
reset();
51-
throw new ArangoDBException("Cannot contact any host!");
59+
throw e;
5260
}
5361

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

7381
@Override
7482
public void success() {
75-
fails = 0;
83+
reset();
7684
}
7785

7886
@Override
79-
public void fail() {
87+
public void fail(Exception exception) {
8088
fails++;
89+
lastFailExceptions.add(exception);
8190
}
8291

8392
@Override
8493
public void reset() {
8594
fails = 0;
95+
lastFailExceptions.clear();
8696
}
8797

8898
@Override

‎src/main/java/com/arangodb/internal/velocystream/VstCommunication.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -95,13 +95,13 @@ protected synchronized C connect(final HostHandle hostHandle, final AccessType a
9595
hostHandler.confirm();
9696
if (!connection.isOpen()) {
9797
// see https://github.com/arangodb/arangodb-java-driver/issues/384
98-
hostHandler.fail();
98+
hostHandler.fail(new IOException("The connection is closed."));
9999
host = hostHandler.get(hostHandle, accessType);
100100
continue;
101101
}
102102
return connection;
103103
} catch (final IOException e) {
104-
hostHandler.fail();
104+
hostHandler.fail(e);
105105
if (hostHandle != null && hostHandle.getHost() != null) {
106106
hostHandle.setHost(null);
107107
}

‎src/main/java/com/arangodb/internal/velocystream/VstCommunicationSync.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ protected Response execute(final Request request, final VstConnectionSync connec
143143
final String location = e.getLocation();
144144
final HostDescription redirectHost = HostUtils.createFromLocation(location);
145145
hostHandler.closeCurrentOnError();
146-
hostHandler.fail();
146+
hostHandler.fail(e);
147147
return execute(request, new HostHandle().setHost(redirectHost), attemptCount + 1);
148148
}
149149
}

‎src/test/java/com/arangodb/ArangoSslTest.java

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,10 @@
2929
import javax.net.ssl.SSLHandshakeException;
3030
import javax.net.ssl.TrustManagerFactory;
3131
import java.security.KeyStore;
32+
import java.util.List;
3233

3334
import static org.hamcrest.MatcherAssert.assertThat;
34-
import static org.hamcrest.Matchers.is;
35-
import static org.hamcrest.Matchers.notNullValue;
35+
import static org.hamcrest.Matchers.*;
3636
import static org.junit.Assert.fail;
3737

3838
/**
@@ -84,7 +84,9 @@ public void connectWithoutValidSslContext() {
8484
arangoDB.getVersion();
8585
fail("this should fail");
8686
} catch (final ArangoDBException ex) {
87-
assertThat(ex.getCause() instanceof SSLHandshakeException, is(true));
87+
assertThat(ex.getCause(), is(instanceOf(ArangoDBMultipleException.class)));
88+
List<Throwable> exceptions = ((ArangoDBMultipleException) ex.getCause()).getExceptions();
89+
exceptions.forEach(e -> assertThat(e, is(instanceOf(SSLHandshakeException.class))));
8890
}
8991
}
9092

0 commit comments

Comments
 (0)