399515 - Websocket-client connect issues should report to websocket onError handlers

jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/WebSocketAdapter.java

@@ -66,7 +66,7 @@ public class WebSocketAdapter implements WebSocketListener
     }
 
     @Override
-    public void onWebSocketException(WebSocketException error)
+    public void onWebSocketError(Throwable cause)
     {
         /* do nothing */
     }

jetty-websocket/websocket-api/src/main/java/org/eclipse/jetty/websocket/api/WebSocketListener.java

@@ -69,7 +69,7 @@ public interface WebSocketListener
      * @param error
      *            the error that occurred.
      */
-    void onWebSocketException(WebSocketException error);
+    void onWebSocketError(Throwable cause);
 
     /**
      * A WebSocket Text frame was received.

jetty-websocket/websocket-client/src/main/java/org/eclipse/jetty/websocket/client/io/ConnectPromise.java

@@ -46,6 +46,16 @@ public abstract class ConnectPromise extends FuturePromise<Session> implements R
         this.masker = client.getMasker();
     }
 
+    @Override
+    public void failed(Throwable cause)
+    {
+        // Notify websocket of failure to connect
+        driver.onError(cause);
+
+        // Notify promise/future of failure to connect
+        super.failed(cause);
+    }
+
     public WebSocketClient getClient()
     {
         return client;

jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/ClientConnectTest.java

@@ -18,6 +18,9 @@
 
 package org.eclipse.jetty.websocket.client;
 
+import static org.hamcrest.Matchers.*;
+
+import java.io.IOException;
 import java.net.ConnectException;
 import java.net.URI;
 import java.util.List;
@@ -27,7 +30,6 @@ import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
 
 import org.eclipse.jetty.toolchain.test.TestTracker;
-import org.eclipse.jetty.util.FutureCallback;
 import org.eclipse.jetty.websocket.api.Session;
 import org.eclipse.jetty.websocket.api.UpgradeException;
 import org.eclipse.jetty.websocket.client.blockhead.BlockheadServer;
@@ -36,6 +38,7 @@ import org.eclipse.jetty.websocket.common.AcceptHash;
 import org.junit.After;
 import org.junit.Assert;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
 
@@ -51,6 +54,26 @@ public class ClientConnectTest
     private BlockheadServer server;
     private WebSocketClient client;
 
+    @SuppressWarnings("unchecked")
+    private <E extends Throwable> E assertExpectedError(ExecutionException e, TrackingSocket wsocket, Class<E> errorClass) throws IOException
+    {
+        // Validate thrown cause
+        Throwable cause = e.getCause();
+        Assert.assertThat("ExecutionException.cause",cause,instanceOf(errorClass));
+
+        // Validate websocket captured cause
+        Assert.assertThat("Error Queue Length",wsocket.errorQueue.size(),greaterThanOrEqualTo(1));
+        Throwable capcause = wsocket.errorQueue.poll();
+        Assert.assertThat("Error Queue[0]",capcause,notNullValue());
+        Assert.assertThat("Error Queue[0]",capcause,instanceOf(errorClass));
+
+        // Validate that websocket didn't see an open event
+        wsocket.assertNotOpened();
+
+        // Return the captured cause
+        return (E)capcause;
+    }
+
     @Before
     public void startClient() throws Exception
     {
@@ -78,7 +101,7 @@ public class ClientConnectTest
         server.stop();
     }
 
-    @Test(expected = UpgradeException.class)
+    @Test
     public void testBadHandshake() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
@@ -99,12 +122,12 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected Path
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,UpgradeException.class);
         }
     }
 
-    @Test(expected = UpgradeException.class)
+    @Test
     public void testBadHandshake_GetOK() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
@@ -125,12 +148,12 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected Path
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,UpgradeException.class);
         }
     }
 
-    @Test(expected = UpgradeException.class)
+    @Test
     public void testBadHandshake_GetOK_WithSecWebSocketAccept() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
@@ -158,12 +181,12 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected Path
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,UpgradeException.class);
         }
     }
 
-    @Test(expected = UpgradeException.class)
+    @Test
     public void testBadHandshake_SwitchingProtocols_InvalidConnectionHeader() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
@@ -191,12 +214,12 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected Path
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,UpgradeException.class);
         }
     }
 
-    @Test(expected = UpgradeException.class)
+    @Test
     public void testBadHandshake_SwitchingProtocols_NoConnectionHeader() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
@@ -224,12 +247,12 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected Path
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,UpgradeException.class);
         }
     }
 
-    @Test(expected = UpgradeException.class)
+    @Test
     public void testBadUpgrade() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
@@ -250,12 +273,13 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected Path
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,UpgradeException.class);
         }
     }
 
     @Test
+    @Ignore("Opened bug 399525")
     public void testConnectionNotAccepted() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
@@ -273,6 +297,8 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
+            // FIXME: Connect Timeout Error?
+            assertExpectedError(e,wsocket,UpgradeException.class);
             // Possible Passing Path (active session wait timeout)
             wsocket.assertNotOpened();
         }
@@ -283,25 +309,32 @@ public class ClientConnectTest
         }
     }
 
-    @Test(expected = ConnectException.class)
+    @Test
     public void testConnectionRefused() throws Exception
     {
         TrackingSocket wsocket = new TrackingSocket();
 
-        // Intentionally bad port
+        // Intentionally bad port with nothing listening on it
         URI wsUri = new URI("ws://127.0.0.1:1");
-        Future<Session> future = client.connect(wsocket,wsUri);
 
-        // The attempt to get upgrade response future should throw error
         try
         {
+            Future<Session> future = client.connect(wsocket,wsUri);
+
+            // The attempt to get upgrade response future should throw error
             future.get(1000,TimeUnit.MILLISECONDS);
             Assert.fail("Expected ExecutionException -> ConnectException");
         }
+        catch (ConnectException e)
+        {
+            Throwable t = wsocket.errorQueue.remove();
+            Assert.assertThat("Error Queue[0]",t,instanceOf(ConnectException.class));
+            wsocket.assertNotOpened();
+        }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected path - java.net.ConnectException
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,ConnectException.class);
         }
     }
 
@@ -326,8 +359,8 @@ public class ClientConnectTest
         }
         catch (ExecutionException e)
         {
-            // Expected Path - throw underlying exception
+            // Expected path - java.net.ConnectException ?
-            FutureCallback.rethrow(e);
+            assertExpectedError(e,wsocket,ConnectException.class);
         }
     }
 }

jetty-websocket/websocket-client/src/test/java/org/eclipse/jetty/websocket/client/TrackingSocket.java

@@ -47,6 +47,7 @@ public class TrackingSocket extends WebSocketAdapter
     public CountDownLatch closeLatch = new CountDownLatch(1);
     public CountDownLatch dataLatch = new CountDownLatch(1);
     public BlockingQueue<String> messageQueue = new BlockingArrayQueue<String>();
+    public BlockingQueue<Throwable> errorQueue = new BlockingArrayQueue<>();
 
     public void assertClose(int expectedStatusCode, String expectedReason) throws InterruptedException
     {
@@ -146,6 +147,13 @@ public class TrackingSocket extends WebSocketAdapter
         openLatch.countDown();
     }
 
+    @Override
+    public void onWebSocketError(Throwable cause)
+    {
+        LOG.debug("onWebSocketError",cause);
+        Assert.assertThat("Error capture",errorQueue.offer(cause),is(true));
+    }
+
     @Override
     public void onWebSocketText(String message)
     {

jetty-websocket/websocket-client/src/test/resources/jetty-logging.properties

@@ -2,7 +2,7 @@ org.eclipse.jetty.util.log.class=org.eclipse.jetty.util.log.StdErrLog
 org.eclipse.jetty.LEVEL=WARN
 # org.eclipse.jetty.io.ChannelEndPoint.LEVEL=INFO
 # org.eclipse.jetty.websocket.LEVEL=WARN
-# org.eclipse.jetty.websocket.LEVEL=DEBUG
+org.eclipse.jetty.websocket.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.client.TrackingSocket.LEVEL=DEBUG
 # Hide the stacktraces
 org.eclipse.jetty.websocket.client.internal.io.UpgradeConnection.STACKS=false

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/AnnotatedEventDriver.java

@@ -23,7 +23,6 @@ import java.io.InputStream;
 import java.io.Reader;
 import java.nio.ByteBuffer;
 
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.WebSocketPolicy;
 import org.eclipse.jetty.websocket.api.annotations.WebSocket;
 import org.eclipse.jetty.websocket.api.extensions.Frame;
@@ -128,11 +127,11 @@ public class AnnotatedEventDriver extends EventDriver
     }
 
     @Override
-    public void onException(WebSocketException e)
+    public void onError(Throwable cause)
     {
-        if (events.onException != null)
+        if (events.onError != null)
         {
-            events.onException.call(websocket,session,e);
+            events.onError.call(websocket,session,cause);
         }
     }
 

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventDriver.java

@@ -78,7 +78,7 @@ public abstract class EventDriver implements IncomingFrames
             terminateConnection(close.getStatusCode(),close.getMessage());
         }
 
-        onException(e);
+        onError(e);
     }
 
     @Override
@@ -161,7 +161,7 @@ public abstract class EventDriver implements IncomingFrames
 
     public abstract void onConnect();
 
-    public abstract void onException(WebSocketException e);
+    public abstract void onError(Throwable t);
 
     public abstract void onFrame(Frame frame);
 

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventDriverFactory.java

@@ -28,7 +28,6 @@ import java.util.concurrent.ConcurrentHashMap;
 import org.eclipse.jetty.util.StringUtil;
 import org.eclipse.jetty.websocket.api.InvalidWebSocketException;
 import org.eclipse.jetty.websocket.api.Session;
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.WebSocketListener;
 import org.eclipse.jetty.websocket.api.WebSocketPolicy;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
@@ -84,8 +83,8 @@ public class EventDriverFactory
         validCloseParams.addParams(Session.class,int.class,String.class);
 
         validErrorParams = new ParamList();
-        validErrorParams.addParams(WebSocketException.class);
+        validErrorParams.addParams(Throwable.class);
-        validErrorParams.addParams(Session.class,WebSocketException.class);
+        validErrorParams.addParams(Session.class,Throwable.class);
 
         validTextParams = new ParamList();
         validTextParams.addParams(String.class);
@@ -326,8 +325,8 @@ public class EventDriverFactory
                 if (method.getAnnotation(OnWebSocketError.class) != null)
                 {
                     assertValidSignature(method,OnWebSocketError.class,validErrorParams);
-                    assertUnset(events.onException,OnWebSocketError.class,method);
+                    assertUnset(events.onError,OnWebSocketError.class,method);
-                    events.onException = new EventMethod(pojo,method);
+                    events.onError = new EventMethod(pojo,method);
                     continue;
                 }
 

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/EventMethods.java

@@ -30,7 +30,7 @@ public class EventMethods
     public EventMethod onClose = null;
     public EventMethod onBinary = null;
     public EventMethod onText = null;
-    public EventMethod onException = null;
+    public EventMethod onError = null;
     public EventMethod onFrame = null;
 
     public EventMethods(Class<?> pojoClass)
@@ -97,7 +97,7 @@ public class EventMethods
         builder.append(", onText=");
         builder.append(onText);
         builder.append(", onException=");
-        builder.append(onException);
+        builder.append(onError);
         builder.append(", onFrame=");
         builder.append(onFrame);
         builder.append("]");

jetty-websocket/websocket-common/src/main/java/org/eclipse/jetty/websocket/common/events/ListenerEventDriver.java

@@ -21,7 +21,6 @@ package org.eclipse.jetty.websocket.common.events;
 import java.io.IOException;
 import java.nio.ByteBuffer;
 
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.WebSocketListener;
 import org.eclipse.jetty.websocket.api.WebSocketPolicy;
 import org.eclipse.jetty.websocket.api.extensions.Frame;
@@ -91,9 +90,9 @@ public class ListenerEventDriver extends EventDriver
     }
 
     @Override
-    public void onException(WebSocketException e)
+    public void onError(Throwable cause)
     {
-        listener.onWebSocketException(e);
+        listener.onWebSocketError(cause);
     }
 
     @Override

jetty-websocket/websocket-common/src/test/java/examples/AnnotatedTextSocket.java

@@ -19,7 +19,6 @@
 package examples;
 
 import org.eclipse.jetty.websocket.api.Session;
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketClose;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
 import org.eclipse.jetty.websocket.api.annotations.OnWebSocketError;
@@ -45,9 +44,9 @@ public class AnnotatedTextSocket
     }
 
     @OnWebSocketError
-    public void onError(WebSocketException e)
+    public void onError(Throwable cause)
     {
-        capture.add("onError(%s: %s)",e.getClass().getSimpleName(),e.getMessage());
+        capture.add("onError(%s: %s)",cause.getClass().getSimpleName(),cause.getMessage());
     }
 
     @OnWebSocketMessage

jetty-websocket/websocket-common/src/test/java/examples/ListenerBasicSocket.java

@@ -19,7 +19,6 @@
 package examples;
 
 import org.eclipse.jetty.websocket.api.Session;
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.WebSocketListener;
 import org.eclipse.jetty.websocket.common.events.EventCapture;
 
@@ -46,9 +45,9 @@ public class ListenerBasicSocket implements WebSocketListener
     }
 
     @Override
-    public void onWebSocketException(WebSocketException error)
+    public void onWebSocketError(Throwable cause)
     {
-        capture.add("onWebSocketException((%s) %s)",error.getClass().getSimpleName(),error.getMessage());
+        capture.add("onWebSocketError((%s) %s)",cause.getClass().getSimpleName(),cause.getMessage());
     }
 
     @Override

jetty-websocket/websocket-common/src/test/java/examples/echo/ListenerEchoSocket.java

@@ -22,7 +22,6 @@ import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import org.eclipse.jetty.websocket.api.Session;
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.WebSocketListener;
 
 /**
@@ -52,9 +51,9 @@ public class ListenerEchoSocket implements WebSocketListener
     }
 
     @Override
-    public void onWebSocketException(WebSocketException error)
+    public void onWebSocketError(Throwable cause)
     {
-        LOG.log(Level.WARNING,"onWebSocketException",error);
+        LOG.log(Level.WARNING,"onWebSocketError",cause);
     }
 
     @Override

jetty-websocket/websocket-common/src/test/java/org/eclipse/jetty/websocket/common/events/EventDriverFactoryTest.java

@@ -169,7 +169,7 @@ public class EventDriverFactoryTest
         assertHasEventMethod(classId + ".onBinary",methods.onBinary);
         assertHasEventMethod(classId + ".onClose",methods.onClose);
         assertHasEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertNoEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
 
@@ -193,7 +193,7 @@ public class EventDriverFactoryTest
         assertHasEventMethod(classId + ".onBinary",methods.onBinary);
         assertHasEventMethod(classId + ".onClose",methods.onClose);
         assertHasEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertNoEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
 
@@ -217,7 +217,7 @@ public class EventDriverFactoryTest
         assertHasEventMethod(classId + ".onBinary",methods.onBinary);
         assertHasEventMethod(classId + ".onClose",methods.onClose);
         assertHasEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertHasEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
     }
@@ -238,7 +238,7 @@ public class EventDriverFactoryTest
         assertNoEventMethod(classId + ".onBinary",methods.onBinary);
         assertHasEventMethod(classId + ".onClose",methods.onClose);
         assertHasEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertHasEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
     }
@@ -259,7 +259,7 @@ public class EventDriverFactoryTest
         assertNoEventMethod(classId + ".onBinary",methods.onBinary);
         assertNoEventMethod(classId + ".onClose",methods.onClose);
         assertNoEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertHasEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
 
@@ -283,7 +283,7 @@ public class EventDriverFactoryTest
         assertNoEventMethod(classId + ".onBinary",methods.onBinary);
         assertNoEventMethod(classId + ".onClose",methods.onClose);
         assertNoEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertNoEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
     }
@@ -304,7 +304,7 @@ public class EventDriverFactoryTest
         assertNoEventMethod(classId + ".onBinary",methods.onBinary);
         assertNoEventMethod(classId + ".onClose",methods.onClose);
         assertNoEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertNoEventMethod(classId + ".onText",methods.onText);
         assertHasEventMethod(classId + ".onFrame",methods.onFrame);
     }
@@ -325,7 +325,7 @@ public class EventDriverFactoryTest
         assertNoEventMethod(classId + ".onBinary",methods.onBinary);
         assertHasEventMethod(classId + ".onClose",methods.onClose);
         assertHasEventMethod(classId + ".onConnect",methods.onConnect);
-        assertHasEventMethod(classId + ".onException",methods.onException);
+        assertHasEventMethod(classId + ".onException",methods.onError);
         assertHasEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
 
@@ -349,7 +349,7 @@ public class EventDriverFactoryTest
         assertNoEventMethod(classId + ".onBinary",methods.onBinary);
         assertHasEventMethod(classId + ".onClose",methods.onClose);
         assertHasEventMethod(classId + ".onConnect",methods.onConnect);
-        assertNoEventMethod(classId + ".onException",methods.onException);
+        assertNoEventMethod(classId + ".onException",methods.onError);
         assertHasEventMethod(classId + ".onText",methods.onText);
         assertNoEventMethod(classId + ".onFrame",methods.onFrame);
 

jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/examples/echo/LogSocket.java

@@ -19,7 +19,6 @@
 package org.eclipse.jetty.websocket.server.examples.echo;
 
 import org.eclipse.jetty.websocket.api.Session;
-import org.eclipse.jetty.websocket.api.WebSocketException;
 import org.eclipse.jetty.websocket.api.WebSocketListener;
 
 public class LogSocket implements WebSocketListener
@@ -59,12 +58,12 @@ public class LogSocket implements WebSocketListener
     }
 
     @Override
-    public void onWebSocketException(WebSocketException error)
+    public void onWebSocketError(Throwable cause)
     {
         if (verbose)
         {
-            System.err.printf("onWebSocketException((%s) %s)%n",error.getClass().getName(),error.getMessage());
+            System.err.printf("onWebSocketError((%s) %s)%n",cause.getClass().getName(),cause.getMessage());
-            error.printStackTrace(System.err);
+            cause.printStackTrace(System.err);
         }
     }