From 7eb54fa9aada3969dbc06998c0ee5ead1f95b3d4 Mon Sep 17 00:00:00 2001 From: Lachlan Roberts Date: Fri, 8 Nov 2024 11:24:51 +1100 Subject: [PATCH] PR #12441 - changes from review Signed-off-by: Lachlan Roberts --- .../core/server/internal/CreatorNegotiator.java | 6 ++---- .../websocket/server/ServerWebSocketContainer.java | 5 ++--- .../server/internal/JakartaWebSocketCreator.java | 2 +- .../server/JettyWebSocketServerContainer.java | 6 ++---- .../server/internal/JakartaWebSocketCreator.java | 2 +- .../server/JettyWebSocketServerContainer.java | 7 +++---- .../ee9/websocket/server/JettyWebSocketServlet.java | 11 +---------- 7 files changed, 12 insertions(+), 27 deletions(-) diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java index 77e7e91b7b7..207fe198b15 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-server/src/main/java/org/eclipse/jetty/websocket/core/server/internal/CreatorNegotiator.java @@ -15,9 +15,7 @@ package org.eclipse.jetty.websocket.core.server.internal; import java.util.concurrent.atomic.AtomicReference; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Context; -import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.websocket.core.FrameHandler; import org.eclipse.jetty.websocket.core.server.FrameHandlerFactory; @@ -61,7 +59,7 @@ public class CreatorNegotiator extends WebSocketNegotiator.AbstractNegotiator } catch (Throwable t) { - Response.writeError(request, response, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, t.getMessage()); + callback.failed(t); return null; } @@ -70,7 +68,7 @@ public class CreatorNegotiator extends WebSocketNegotiator.AbstractNegotiator FrameHandler frameHandler = factory.newFrameHandler(websocketPojo, request, response); if (frameHandler == null) - Response.writeError(request, response, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, "No WebSocket FrameHandler was created"); + callback.failed(new IllegalStateException("No WebSocket FrameHandler was created")); return frameHandler; } diff --git a/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java b/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java index 1ba9e5c7f58..9d50cd242f5 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java +++ b/jetty-core/jetty-websocket/jetty-websocket-jetty-server/src/main/java/org/eclipse/jetty/websocket/server/ServerWebSocketContainer.java @@ -20,7 +20,6 @@ import java.util.List; import java.util.concurrent.Executor; import java.util.function.Consumer; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.server.Context; import org.eclipse.jetty.server.Handler; @@ -365,8 +364,8 @@ public class ServerWebSocketContainer extends ContainerLifeCycle implements WebS catch (Throwable x) { if (LOG.isDebugEnabled()) - LOG.debug("Could not create WebSocket endpoint"); - Response.writeError(rq, rs, cb, HttpStatus.INTERNAL_SERVER_ERROR_500, "Could not create WebSocket endpoint"); + LOG.debug("Could not create WebSocket endpoint", x); + cb.failed(x); return null; } }; diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java index 400ccf30683..994ec93a916 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee10/websocket/jakarta/server/internal/JakartaWebSocketCreator.java @@ -168,7 +168,7 @@ public class JakartaWebSocketCreator implements WebSocketCreator { if (LOG.isDebugEnabled()) LOG.debug("Unable to create websocket: {}", config.getEndpointClass().getName(), x); - Response.writeError(request, response, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, "Unable to create WebSocket"); + callback.failed(x); return null; } } diff --git a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java index 844d567c32c..9c896bebc30 100644 --- a/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-ee10/jetty-ee10-websocket/jetty-ee10-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee10/websocket/server/JettyWebSocketServerContainer.java @@ -32,9 +32,7 @@ import org.eclipse.jetty.ee10.websocket.server.internal.DelegatedServerUpgradeRe import org.eclipse.jetty.ee10.websocket.server.internal.DelegatedServerUpgradeResponse; import org.eclipse.jetty.ee10.websocket.server.internal.JettyServerFrameHandlerFactory; import org.eclipse.jetty.ee10.websocket.servlet.WebSocketUpgradeFilter; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.pathmap.PathSpec; -import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.component.ContainerLifeCycle; import org.eclipse.jetty.util.component.Dumpable; @@ -162,7 +160,7 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements { if (LOG.isDebugEnabled()) LOG.debug("Could not create WebSocket endpoint", t); - Response.writeError(req, resp, cb, HttpStatus.INTERNAL_SERVER_ERROR_500, "Could not create WebSocket endpoint"); + cb.failed(t); return null; } }; @@ -215,7 +213,7 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements { if (LOG.isDebugEnabled()) LOG.debug("Could not create WebSocket endpoint", t); - Response.writeError(req, resp, cb, HttpStatus.INTERNAL_SERVER_ERROR_500, "Could not create WebSocket endpoint"); + cb.failed(t); return null; } }; diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/internal/JakartaWebSocketCreator.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/internal/JakartaWebSocketCreator.java index e83d4fde22b..1bde01dcbc9 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/internal/JakartaWebSocketCreator.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jakarta-server/src/main/java/org/eclipse/jetty/ee9/websocket/jakarta/server/internal/JakartaWebSocketCreator.java @@ -168,7 +168,7 @@ public class JakartaWebSocketCreator implements WebSocketCreator { if (LOG.isDebugEnabled()) LOG.debug("Unable to create WebSocket: {}", config.getEndpointClass().getName(), x); - Response.writeError(request, response, callback, HttpStatus.INTERNAL_SERVER_ERROR_500, "Unable to create WebSocket"); + callback.failed(x); return null; } } diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java index f198114bb9e..245fc785564 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServerContainer.java @@ -37,7 +37,6 @@ import org.eclipse.jetty.ee9.websocket.server.internal.DelegatedServerUpgradeReq import org.eclipse.jetty.ee9.websocket.server.internal.DelegatedServerUpgradeResponse; import org.eclipse.jetty.ee9.websocket.server.internal.JettyServerFrameHandlerFactory; import org.eclipse.jetty.ee9.websocket.servlet.WebSocketUpgradeFilter; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.http.pathmap.PathSpec; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; @@ -164,7 +163,7 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements { if (LOG.isDebugEnabled()) LOG.debug("Could not create WebSocket endpoint", t); - Response.writeError(req, resp, cb, HttpStatus.INTERNAL_SERVER_ERROR_500, "Could not create WebSocket endpoint"); + cb.failed(t); return null; } }; @@ -210,14 +209,14 @@ public class JettyWebSocketServerContainer extends ContainerLifeCycle implements { Object webSocket = creator.createWebSocket(new DelegatedServerUpgradeRequest(req), new DelegatedServerUpgradeResponse(resp)); if (webSocket == null) - Response.writeError(req, resp, cb, HttpStatus.INTERNAL_SERVER_ERROR_500, "Could not create WebSocket endpoint"); + cb.succeeded(); return webSocket; } catch (Throwable t) { if (LOG.isDebugEnabled()) LOG.debug("Could not create WebSocket endpoint", t); - Response.writeError(req, resp, cb, HttpStatus.INTERNAL_SERVER_ERROR_500, "Could not create WebSocket endpoint"); + cb.failed(t); return null; } }; diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServlet.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServlet.java index 165bdbb0b31..87d7c47a409 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServlet.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-server/src/main/java/org/eclipse/jetty/ee9/websocket/server/JettyWebSocketServlet.java @@ -30,7 +30,6 @@ import org.eclipse.jetty.ee9.websocket.server.internal.DelegatedServerUpgradeReq import org.eclipse.jetty.ee9.websocket.server.internal.DelegatedServerUpgradeResponse; import org.eclipse.jetty.ee9.websocket.server.internal.JettyServerFrameHandlerFactory; import org.eclipse.jetty.ee9.websocket.servlet.WebSocketUpgradeFilter; -import org.eclipse.jetty.http.HttpStatus; import org.eclipse.jetty.server.Request; import org.eclipse.jetty.server.Response; import org.eclipse.jetty.util.Callback; @@ -320,15 +319,7 @@ public abstract class JettyWebSocketServlet extends HttpServlet } catch (Throwable t) { - try - { - response.sendError(HttpStatus.INTERNAL_SERVER_ERROR_500, "Could not create WebSocket endpoint"); - callback.succeeded(); - } - catch (Throwable x) - { - callback.failed(x); - } + callback.failed(t); return null; } }