jihoonson closed pull request #6090: Fix missing exception handling as part of
`io.druid.java.util.http.client.netty.HttpClientPipelineFactory`
URL: https://github.com/apache/incubator-druid/pull/6090
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/java-util/src/main/java/io/druid/java/util/http/client/NettyHttpClient.java
b/java-util/src/main/java/io/druid/java/util/http/client/NettyHttpClient.java
index 03a5e3e47c1..7ca510ccfd5 100644
---
a/java-util/src/main/java/io/druid/java/util/http/client/NettyHttpClient.java
+++
b/java-util/src/main/java/io/druid/java/util/http/client/NettyHttpClient.java
@@ -280,18 +280,17 @@ public void exceptionCaught(ChannelHandlerContext
context, ExceptionEvent event)
if (response != null) {
handler.exceptionCaught(response, event.getCause());
}
- removeHandlers();
try {
- channel.close();
+ if (channel.isOpen()) {
+ channel.close();
+ }
}
catch (Exception e) {
- // ignore
+ log.warn(e, "Error while closing channel");
}
finally {
channelResourceContainer.returnResource();
}
-
- context.sendUpstream(event);
}
@Override
@@ -310,7 +309,6 @@ public void channelDisconnected(ChannelHandlerContext
context, ChannelStateEvent
log.warn("[%s] Channel disconnected before response complete",
requestDesc);
retVal.setException(new ChannelException("Channel
disconnected"));
}
- context.sendUpstream(event);
}
private void removeHandlers()
diff --git
a/java-util/src/main/java/io/druid/java/util/http/client/pool/ChannelResourceFactory.java
b/java-util/src/main/java/io/druid/java/util/http/client/pool/ChannelResourceFactory.java
index 0c8c39d931e..04124b2b914 100644
---
a/java-util/src/main/java/io/druid/java/util/http/client/pool/ChannelResourceFactory.java
+++
b/java-util/src/main/java/io/druid/java/util/http/client/pool/ChannelResourceFactory.java
@@ -27,8 +27,11 @@
import org.jboss.netty.channel.ChannelException;
import org.jboss.netty.channel.ChannelFuture;
import org.jboss.netty.channel.ChannelFutureListener;
+import org.jboss.netty.channel.ChannelHandlerContext;
import org.jboss.netty.channel.ChannelPipeline;
import org.jboss.netty.channel.Channels;
+import org.jboss.netty.channel.ExceptionEvent;
+import org.jboss.netty.channel.SimpleChannelUpstreamHandler;
import org.jboss.netty.handler.ssl.SslHandler;
import org.jboss.netty.util.Timer;
@@ -111,6 +114,25 @@ public ChannelFuture generate(final String hostname)
pipeline.addFirst("ssl", sslHandler);
final ChannelFuture handshakeFuture =
Channels.future(connectFuture.getChannel());
+ pipeline.addLast("connectionErrorHandler", new
SimpleChannelUpstreamHandler()
+ {
+ @Override
+ public void exceptionCaught(ChannelHandlerContext ctx, ExceptionEvent
e)
+ {
+ final Channel channel = ctx.getChannel();
+ if (channel == null) {
+ // For the case where this pipeline is not attached yet.
+ handshakeFuture.setFailure(new ChannelException(
+ StringUtils.format("Channel is null. The context name is
[%s]", ctx.getName())
+ ));
+ return;
+ }
+ handshakeFuture.setFailure(e.getCause());
+ if (channel.isOpen()) {
+ channel.close();
+ }
+ }
+ });
connectFuture.addListener(
new ChannelFutureListener()
{
diff --git
a/java-util/src/test/java/io/druid/java/util/http/client/JankyServersTest.java
b/java-util/src/test/java/io/druid/java/util/http/client/JankyServersTest.java
index bdc8a2acd85..0700e85c98b 100644
---
a/java-util/src/test/java/io/druid/java/util/http/client/JankyServersTest.java
+++
b/java-util/src/test/java/io/druid/java/util/http/client/JankyServersTest.java
@@ -31,7 +31,9 @@
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.BeforeClass;
+import org.junit.Rule;
import org.junit.Test;
+import org.junit.rules.ExpectedException;
import javax.net.ssl.SSLContext;
import java.io.IOException;
@@ -55,6 +57,9 @@
static ServerSocket echoServerSocket;
static ServerSocket closingServerSocket;
+ @Rule
+ public ExpectedException expectedException = ExpectedException.none();
+
@BeforeClass
public static void setUp() throws Exception
{
@@ -309,16 +314,10 @@ public void testHttpEchoServer() throws Throwable
new StatusResponseHandler(StandardCharsets.UTF_8)
);
- Throwable e = null;
- try {
- response.get();
- }
- catch (ExecutionException e1) {
- e = e1.getCause();
- }
+ expectedException.expect(ExecutionException.class);
+ expectedException.expectMessage("java.lang.IllegalArgumentException:
invalid version format: GET");
- Assert.assertTrue("IllegalArgumentException thrown by 'get'", e
instanceof IllegalArgumentException);
- Assert.assertTrue("Expected error message",
e.getMessage().matches(".*invalid version format:.*"));
+ response.get();
}
finally {
lifecycle.stop();
@@ -339,15 +338,10 @@ public void testHttpsEchoServer() throws Throwable
new StatusResponseHandler(StandardCharsets.UTF_8)
);
- Throwable e = null;
- try {
- response.get();
- }
- catch (ExecutionException e1) {
- e = e1.getCause();
- }
+ expectedException.expect(ExecutionException.class);
+
expectedException.expectMessage("org.jboss.netty.channel.ChannelException:
Faulty channel in resource pool");
- Assert.assertNotNull("ChannelException thrown by 'get'", e);
+ response.get();
}
finally {
lifecycle.stop();
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
