From 77d95d039cea5cd642167163344ec223d823890f Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Tue, 28 May 2019 11:41:05 +1000 Subject: [PATCH 1/2] Issue #3696 - unwrap ExecutionException in javax WS connectToServer Signed-off-by: Lachlan Roberts --- .../javax/client/JavaxWebSocketClientContainer.java | 5 +++++ .../javax/tests/client/misbehaving/MisbehavingClassTest.java | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java b/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java index ae5c8465411..e6166e6403d 100644 --- a/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java +++ b/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.net.URI; import java.util.Objects; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ExecutionException; import java.util.concurrent.Executor; import java.util.concurrent.Future; import java.util.concurrent.TimeUnit; @@ -148,6 +149,10 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple return sessionFuture.get(timeout+1000, TimeUnit.MILLISECONDS); return sessionFuture.get(); } + catch (ExecutionException e) + { + throw new IOException("Connection future not completed " + destURI, e.getCause()); + } catch (TimeoutException e) { throw new IOException("Connection future not completed " + destURI, e); diff --git a/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java b/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java index 5a577c2c884..25b47707bdc 100644 --- a/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java +++ b/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java @@ -26,6 +26,7 @@ import javax.websocket.ContainerProvider; import javax.websocket.WebSocketContainer; import org.eclipse.jetty.util.log.StacklessLogging; +import org.eclipse.jetty.websocket.core.CloseException; import org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession; import org.eclipse.jetty.websocket.javax.tests.CoreServer; import org.junit.jupiter.api.AfterAll; @@ -76,7 +77,7 @@ public class MisbehavingClassTest { // expecting IOException during onOpen Exception e = assertThrows(IOException.class, () -> container.connectToServer(socket, server.getWsUri()), "Should have failed .connectToServer()"); - assertThat(e.getCause(), instanceOf(ExecutionException.class)); + assertThat(e.getCause(), instanceOf(CloseException.class)); assertThat("Close should have occurred", socket.closeLatch.await(1, TimeUnit.SECONDS), is(true)); @@ -97,7 +98,7 @@ public class MisbehavingClassTest { // expecting IOException during onOpen Exception e = assertThrows(IOException.class, () -> container.connectToServer(socket, server.getWsUri()), "Should have failed .connectToServer()"); - assertThat(e.getCause(), instanceOf(ExecutionException.class)); + assertThat(e.getCause(), instanceOf(CloseException.class)); assertThat("Close should have occurred", socket.closeLatch.await(5, TimeUnit.SECONDS), is(true)); From 0cf91c91e194e937128055abddee2490c50daa32 Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Wed, 29 May 2019 19:21:00 +0200 Subject: [PATCH 2/2] Issue #3696 - Unwrap JavaxWebSocketClientContainer.connectToServer() exceptions. Improved exception unwrapping. Code cleanups. Signed-off-by: Simone Bordet --- .../client/JavaxWebSocketClientContainer.java | 25 +++++++----- .../misbehaving/MisbehavingClassTest.java | 40 ++++++------------- 2 files changed, 27 insertions(+), 38 deletions(-) diff --git a/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java b/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java index e6166e6403d..e051d723a28 100644 --- a/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java +++ b/jetty-websocket/javax-websocket-client/src/main/java/org/eclipse/jetty/websocket/javax/client/JavaxWebSocketClientContainer.java @@ -127,7 +127,7 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple JavaxClientUpgradeRequest upgradeRequest = new JavaxClientUpgradeRequest(this, getWebSocketCoreClient(), destURI, configuredEndpoint); EndpointConfig config = configuredEndpoint.getConfig(); - if (config != null && config instanceof ClientEndpointConfig) + if (config instanceof ClientEndpointConfig) { ClientEndpointConfig clientEndpointConfig = (ClientEndpointConfig)config; @@ -141,30 +141,35 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple upgradeRequest.setSubProtocols(clientEndpointConfig.getPreferredSubprotocols()); } + long timeout = coreClient.getHttpClient().getConnectTimeout(); try { Future sessionFuture = connect(upgradeRequest); - long timeout = coreClient.getHttpClient().getConnectTimeout(); if (timeout>0) return sessionFuture.get(timeout+1000, TimeUnit.MILLISECONDS); return sessionFuture.get(); } catch (ExecutionException e) { - throw new IOException("Connection future not completed " + destURI, e.getCause()); + var cause = e.getCause(); + if (cause instanceof RuntimeException) + throw (RuntimeException)cause; + if (cause instanceof IOException) + throw (IOException)cause; + throw new IOException(cause); } catch (TimeoutException e) { - throw new IOException("Connection future not completed " + destURI, e); + throw new IOException("Connection future timeout " + timeout + " ms for " + destURI, e); } - catch (Exception e) + catch (Throwable e) { throw new IOException("Unable to connect to " + destURI, e); } } @Override - public Session connectToServer(final Class endpointClass, final ClientEndpointConfig config, URI path) throws DeploymentException, IOException + public Session connectToServer(final Class endpointClass, final ClientEndpointConfig config, URI path) throws IOException { ClientEndpointConfig clientEndpointConfig = config; if (clientEndpointConfig == null) @@ -176,7 +181,7 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple } @Override - public Session connectToServer(final Class annotatedEndpointClass, final URI path) throws DeploymentException, IOException + public Session connectToServer(final Class annotatedEndpointClass, final URI path) throws IOException { ConfiguredEndpoint instance = newConfiguredEndpoint(annotatedEndpointClass, new EmptyClientEndpointConfig()); return connect(instance, path); @@ -217,11 +222,11 @@ public class JavaxWebSocketClientContainer extends JavaxWebSocketContainer imple { try { - return newConfiguredEndpoint(endpointClass.newInstance(), config); + return newConfiguredEndpoint(endpointClass.getConstructor().newInstance(), config); } - catch (DeploymentException | InstantiationException | IllegalAccessException e) + catch (Throwable e) { - throw new InvalidWebSocketException("Unable to instantiate websocket: " + endpointClass.getClass()); + throw new InvalidWebSocketException("Unable to instantiate websocket: " + endpointClass.getName()); } } diff --git a/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java b/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java index 25b47707bdc..acbc3d27145 100644 --- a/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java +++ b/jetty-websocket/javax-websocket-tests/src/test/java/org/eclipse/jetty/websocket/javax/tests/client/misbehaving/MisbehavingClassTest.java @@ -18,8 +18,6 @@ package org.eclipse.jetty.websocket.javax.tests.client.misbehaving; -import java.io.IOException; -import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import javax.websocket.ContainerProvider; @@ -29,8 +27,8 @@ import org.eclipse.jetty.util.log.StacklessLogging; import org.eclipse.jetty.websocket.core.CloseException; import org.eclipse.jetty.websocket.core.internal.WebSocketCoreSession; import org.eclipse.jetty.websocket.javax.tests.CoreServer; -import org.junit.jupiter.api.AfterAll; -import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; @@ -40,32 +38,21 @@ import static org.junit.jupiter.api.Assertions.assertThrows; public class MisbehavingClassTest { + private CoreServer server; - private static CoreServer server; - - @SuppressWarnings("Duplicates") - @BeforeAll - public static void startServer() throws Exception + @BeforeEach + public void startServer() throws Exception { server = new CoreServer(new CoreServer.EchoNegotiator()); - // Start Server server.start(); } - @AfterAll - public static void stopServer() + @AfterEach + public void stopServer() throws Exception { - try - { - server.stop(); - } - catch (Exception e) - { - e.printStackTrace(System.err); - } + server.stop(); } - @SuppressWarnings("Duplicates") @Test public void testEndpointRuntimeOnOpen() throws Exception { @@ -75,9 +62,8 @@ public class MisbehavingClassTest try (StacklessLogging ignored = new StacklessLogging(WebSocketCoreSession.class)) { - // expecting IOException during onOpen - Exception e = assertThrows(IOException.class, () -> container.connectToServer(socket, server.getWsUri()), "Should have failed .connectToServer()"); - assertThat(e.getCause(), instanceOf(CloseException.class)); + // Expecting CloseException during onOpen(). + assertThrows(CloseException.class, () -> container.connectToServer(socket, server.getWsUri()), "Should have failed .connectToServer()"); assertThat("Close should have occurred", socket.closeLatch.await(1, TimeUnit.SECONDS), is(true)); @@ -86,7 +72,6 @@ public class MisbehavingClassTest } } - @SuppressWarnings("Duplicates") @Test public void testAnnotatedRuntimeOnOpen() throws Exception { @@ -96,9 +81,8 @@ public class MisbehavingClassTest try (StacklessLogging ignored = new StacklessLogging(WebSocketCoreSession.class)) { - // expecting IOException during onOpen - Exception e = assertThrows(IOException.class, () -> container.connectToServer(socket, server.getWsUri()), "Should have failed .connectToServer()"); - assertThat(e.getCause(), instanceOf(CloseException.class)); + // Expecting CloseException during onOpen(). + assertThrows(CloseException.class, () -> container.connectToServer(socket, server.getWsUri()), "Should have failed .connectToServer()"); assertThat("Close should have occurred", socket.closeLatch.await(5, TimeUnit.SECONDS), is(true));