From c6e58e693b10f09621e1c7721b01389e6021d2e6 Mon Sep 17 00:00:00 2001
From: Lachlan Roberts <lachlan@webtide.com>
Date: Wed, 22 Apr 2020 18:10:49 +1000
Subject: [PATCH] Issue #4747 - changes from review

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
---
 .../jetty/websocket/core/CloseStatus.java      |  7 +------
 .../core/internal/WebSocketSessionState.java   | 18 +++++++++++++++---
 .../test/resources/jetty-logging.properties    |  2 +-
 .../test/resources/jetty-logging.properties    |  2 +-
 4 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/CloseStatus.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/CloseStatus.java
index 4025c632aa1..cdbed64f7a6 100644
--- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/CloseStatus.java
+++ b/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 class CloseStatus
         return !isOrdinary(code);
     }
 
-    public void initCause(Throwable cause)
-    {
-        this.cause = cause;
-    }
-
     public Throwable getCause()
     {
         return cause;
diff --git a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java
index 574341dcbda..6054ab8068f 100644
--- a/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java
+++ b/jetty-websocket/websocket-core/src/main/java/org/eclipse/jetty/websocket/core/internal/WebSocketSessionState.java
@@ -20,6 +20,7 @@ package org.eclipse.jetty.websocket.core.internal;
 
 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 class WebSocketSessionState
     }
 
     /**
-     * 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 class WebSocketSessionState
 
             // 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);
         }
     }
 
diff --git a/jetty-websocket/websocket-core/src/test/resources/jetty-logging.properties b/jetty-websocket/websocket-core/src/test/resources/jetty-logging.properties
index 2f149811ed4..c08968b68e5 100644
--- a/jetty-websocket/websocket-core/src/test/resources/jetty-logging.properties
+++ b/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
diff --git a/jetty-websocket/websocket-javax-tests/src/test/resources/jetty-logging.properties b/jetty-websocket/websocket-javax-tests/src/test/resources/jetty-logging.properties
index 83afb9061fe..7485b6d2a83 100644
--- a/jetty-websocket/websocket-javax-tests/src/test/resources/jetty-logging.properties
+++ b/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