5074 Fix the WebSocket exception handling strategy

The ConnectionBase exception handler was not set when `webSocket.exceptionHandler(...)` is called. This was leading `ConnectionBase` to consider the exception as uncaught and to print it, ignoring the exception handler set at the WebSocketImplBase level.

Moreover, this WebSocket handler was also called after the call to `ConnectionBase#handleException(...)`.

So I decided to stick on the `ConnectionBase` exception handler for the WebSocketImplBase.

The replacement of `Http1xConnectionBase conn` with `Http1xConnectionBase<?> conn` was needed otherwise the compiler complains.

Signed-off-by: Nils Renaud <[email protected]>

src/main/java/io/vertx/core/http/impl/Http1xClientConnection.java

@@ -1207,18 +1207,13 @@ protected void handleIdle(IdleStateEvent event) {
   }
 
   @Override
-  protected void handleException(Throwable e) {
+  public void handleException(Throwable e) {
     super.handleException(e);
-    WebSocketImpl ws;
     LinkedHashSet<Stream> allStreams = new LinkedHashSet<>();
     synchronized (this) {
-      ws = webSocket;
       allStreams.addAll(requests);
       allStreams.addAll(responses);
     }
-    if (ws != null) {
-      ws.handleException(e);
-    }
     for (Stream stream : allStreams) {
       stream.handleException(e);
     }

src/main/java/io/vertx/core/http/impl/Http1xConnectionBase.java

@@ -116,6 +116,11 @@ public Http1xConnectionBase exceptionHandler(Handler<Throwable> handler) {
     return (Http1xConnectionBase) super.exceptionHandler(handler);
   }
 
+  @Override
+  public void handleException(Throwable t) {
+    super.handleException(t);
+  }
+
   @Override
   public HttpConnection goAway(long errorCode, int lastStreamId, Buffer debugData) {
     throw new UnsupportedOperationException("HTTP/1.x connections don't support GOAWAY");

src/main/java/io/vertx/core/http/impl/Http1xServerConnection.java

@@ -520,13 +520,11 @@ protected void handleClosed() {
   }
 
   @Override
-  protected void handleException(Throwable t) {
+  public void handleException(Throwable t) {
     super.handleException(t);
     Http1xServerRequest responseInProgress;
     Http1xServerRequest requestInProgress;
-    ServerWebSocketImpl ws;
     synchronized (this) {
-      ws = this.webSocket;
       requestInProgress = this.requestInProgress;
       responseInProgress = this.responseInProgress;
       if (METRICS_ENABLED && metrics != null) {
@@ -539,9 +537,6 @@ protected void handleException(Throwable t) {
     if (responseInProgress != null && responseInProgress != requestInProgress) {
       responseInProgress.handleException(t);
     }
-    if (ws != null) {
-      ws.context.execute(v -> ws.handleException(t));
-    }
   }
 
   @Override

src/main/java/io/vertx/core/http/impl/WebSocketImplBase.java

@@ -73,7 +73,6 @@ public abstract class WebSocketImplBase<S extends WebSocket> implements WebSocke
   private FrameAggregator frameAggregator;
   private Handler<Buffer> pongHandler;
   private Handler<Void> drainHandler;
-  private Handler<Throwable> exceptionHandler;
   private Handler<Void> closeHandler;
   private Handler<Void> endHandler;
   protected final Http1xConnectionBase conn;
@@ -412,13 +411,12 @@ protected void handleClose(boolean graceful) {
     Handler<Throwable> exceptionHandler;
     synchronized (conn) {
       closeHandler = this.closeHandler;
-      exceptionHandler = this.exceptionHandler;
+      exceptionHandler = conn::handleException;
       binaryConsumer = this.binaryHandlerRegistration;
       textConsumer = this.textHandlerRegistration;
       this.binaryHandlerRegistration = null;
       this.textHandlerRegistration = null;
       this.closeHandler = null;
-      this.exceptionHandler = null;
     }
     if (binaryConsumer != null) {
       binaryConsumer.unregister();
@@ -651,14 +649,7 @@ void handleWriteQueueDrained(Void v) {
   }
 
   void handleException(Throwable t) {
-    Handler<Throwable> handler;
-    synchronized (conn) {
-      handler = this.exceptionHandler;
-      if (handler == null) {
-        return;
-      }
-    }
-    context.dispatch(t, handler);
+    conn.handleException(t);
   }
 
   void handleConnectionClosed() {
@@ -718,13 +709,8 @@ private Handler<Void> endHandler() {
 
   @Override
   public S exceptionHandler(Handler<Throwable> handler) {
-    synchronized (conn) {
-      if (handler != null) {
-        checkClosed();
-      }
-      this.exceptionHandler = handler;
-      return (S) this;
-    }
+    conn.exceptionHandler(handler);
+    return (S) this;
   }
 
   @Override

src/test/java/io/vertx/core/http/WebSocketTest.java

@@ -77,6 +77,7 @@ public class WebSocketTest extends VertxTestBase {
 
   private static final String TEST_REASON = "I'm moving away!";
   private static final short TEST_STATUS_CODE = (short)1001;
+  private static final short INVALID_STATUS_CODE = (short)1004;
 
   private WebSocketClient client;
   private HttpServer server;
@@ -3785,4 +3786,24 @@ public void testConnect() throws Exception {
       }));
     await();
   }
+
+  @Test
+  public void testServerWebSocketExceptionHandlerIsCalled() throws InterruptedException {
+    waitFor(2);
+    server = vertx.createHttpServer(new HttpServerOptions().setPort(DEFAULT_HTTP_PORT)
+                                                           .setHost(DEFAULT_HTTP_HOST))
+                  .exceptionHandler(t -> fail())
+                  .connectionHandler(connection -> connection.exceptionHandler(t -> fail()))
+                  .webSocketHandler(ws -> {
+                    ws.endHandler(v -> fail());
+                    ws.closeHandler(v -> complete());
+                    ws.exceptionHandler(t -> complete());
+                  });
+    awaitFuture(server.listen());
+    vertx.createWebSocketClient()
+         .connect(DEFAULT_HTTP_PORT, DEFAULT_HTTP_HOST, "/")
+         .onSuccess(ws -> ws.close(INVALID_STATUS_CODE));
+    await();
+  }
+
 }