Issue #4747 - changes from review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/CloseStatus.java

@@ -55,7 +55,7 @@ public class CloseStatus
 
     private final int code;
     private final String reason;
-    private Throwable cause;
+    private final Throwable cause;
 
     /**
      * Creates a reason for closing a web socket connection with the no given status code.
@@ -211,11 +211,6 @@ public boolean isAbnormal()
         return !isOrdinary(code);
     }
 
-    public void initCause(Throwable cause)
-    {
-        this.cause = cause;
-    }
-
     public Throwable getCause()
     {
         return cause;

jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java

@@ -20,6 +20,7 @@
 
 import java.nio.channels.ClosedChannelException;
 
+import org.eclipse.jetty.util.Callback;
 import org.eclipse.jetty.websocket.core.CloseStatus;
 import org.eclipse.jetty.websocket.core.Frame;
 import org.eclipse.jetty.websocket.core.OpCode;
@@ -124,8 +125,19 @@ public boolean onClosed(CloseStatus closeStatus)
     }
 
     /**
-     * This should only be called directly before closing the connection when we are in {@link State#CLOSED} state.
-     * This ensures an abnormal close status and if we have no error in the CloseStatus we will set one.
+     * <p>
+     * If no error is set in the CloseStatus this will either, replace the current close status with
+     * a {@link CloseStatus#SERVER_ERROR} status if we had a NORMAL close code, or, it will set the cause
+     * of the CloseStatus if the previous cause was null, this allows onError to be notified after the connection is closed.
+     * </p>
+     * <p>
+     * This should only be called if there is an error directly before the call to
+     * {@link WebSocketCoreSession#closeConnection(CloseStatus, Callback)}.
+     * </p>
+     * <p>
+     * This could occur if the FrameHandler throws an exception in onFrame after receiving a close frame reply, in this
+     * case to notify onError we must set the cause in the closeStatus.
+     * </p>
      * @param t the error which occurred.
      */
     public void onError(Throwable t)
@@ -141,7 +153,7 @@ public void onError(Throwable t)
 
             // Otherwise set the error if it wasn't already set to notify onError as well as onClose.
             if (_closeStatus.getCause() == null)
-                _closeStatus.initCause(t);
+                _closeStatus = new CloseStatus(_closeStatus.getCode(), _closeStatus.getReason(), t);
         }
     }
 

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

@@ -1,5 +1,5 @@
 # Jetty Logging using jetty-slf4j-impl
-org.eclipse.jetty.LEVEL=INFO
+# org.eclipse.jetty.LEVEL=DEBUG
 # org.eclipse.jetty.io.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.core.LEVEL=DEBUG
 # org.eclipse.jetty.websocket.core.TestFrameHandler.LEVEL=DEBUG

jetty-websocket/websocket-javax-tests/src/test/resources/jetty-logging.properties

@@ -1,5 +1,5 @@
 # Jetty Logging using jetty-slf4j-impl
-org.eclipse.jetty.LEVEL=INFO
+# org.eclipse.jetty.LEVEL=DEBUG
 # org.eclipse.jetty.util.log.stderr.LONG=true
 # org.eclipse.jetty.server.AbstractConnector.LEVEL=DEBUG
 # org.eclipse.jetty.io.WriteFlusher.LEVEL=DEBUG