Issue #2175 cleanups after review

Improve ws error handling by splitting processError into handling for
errors from the network and errors from the application.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
This commit is contained in:
Greg Wilkins 2019-01-22 21:20:01 +11:00
parent 7fec51ad40
commit d598e0dc6f
2 changed files with 29 additions and 47 deletions

View File

@ -46,7 +46,6 @@ public class MisbehavingClassTest
@BeforeAll @BeforeAll
public static void startServer() throws Exception public static void startServer() throws Exception
{ {
System.err.println("START");
server = new CoreServer(new CoreServer.EchoNegotiator()); server = new CoreServer(new CoreServer.EchoNegotiator());
// Start Server // Start Server
server.start(); server.start();
@ -55,7 +54,6 @@ public class MisbehavingClassTest
@AfterAll @AfterAll
public static void stopServer() public static void stopServer()
{ {
System.err.println("STOP");
try try
{ {
server.stop(); server.stop();

View File

@ -287,16 +287,19 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
public void onClosed(Throwable cause) public void onClosed(Throwable cause)
{ {
onClosed(cause, new CloseStatus(CloseStatus.NO_CLOSE, cause == null?null:cause.toString()));
CloseStatus closeStatus = new CloseStatus(CloseStatus.NO_CLOSE, cause == null?null:cause.toString());
if (channelState.onClosed(closeStatus))
onClosed(cause, closeStatus);
} }
public void onClosed(Throwable cause, CloseStatus closeStatus) public void onClosed(Throwable cause, CloseStatus closeStatus)
{ {
if (channelState.onClosed(closeStatus)) connection.cancelDemand();
{
connection.cancelDemand();
// Forward Errors to Local WebSocket EndPoint // Forward Errors to Local WebSocket EndPoint
if (cause!=null)
{
try try
{ {
handler.onError(cause); handler.onError(cause);
@ -306,19 +309,22 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
cause.addSuppressed(e); cause.addSuppressed(e);
LOG.warn(cause); LOG.warn(cause);
} }
try
{
handler.onClosed(closeStatus);
}
catch (Throwable e)
{
LOG.warn(e);
}
} }
try
{
handler.onClosed(closeStatus);
}
catch (Throwable e)
{
LOG.warn(e);
}
if (connection.getEndPoint().isOpen())
connection.close();
} }
AbnormalCloseStatus closeStatusFor(Throwable cause) AbnormalCloseStatus abnormalCloseStatusFor(Throwable cause)
{ {
int code; int code;
if (cause instanceof ProtocolException) if (cause instanceof ProtocolException)
@ -339,37 +345,30 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
/** /**
* Process an Error that originated from the connection. * Process an Error that originated from the connection.
* For protocol causes, send and abnormal close frame
* otherwise just close the connection.
* *
* @param cause the cause * @param cause the cause
*/ */
public void processConnectionError(Throwable cause) public void processConnectionError(Throwable cause)
{ {
CloseStatus closeStatus = closeStatusFor(cause); CloseStatus closeStatus = abnormalCloseStatusFor(cause);
if (closeStatus.getCode() == CloseStatus.PROTOCOL) if (closeStatus.getCode() == CloseStatus.PROTOCOL)
close(closeStatus, Callback.NOOP, false); close(closeStatus, Callback.NOOP, false);
else else if (channelState.onClosed(closeStatus))
{ onClosed(cause, closeStatus);
try
{
onClosed(cause, closeStatus);
}
finally
{
connection.close();
}
}
} }
/** /**
* Process an Error that originated from the handler. * Process an Error that originated from the handler.
* Send an abnormal close frame to ensure connection is closed.
* *
* @param cause the cause * @param cause the cause
*/ */
public void processHandlerError(Throwable cause) public void processHandlerError(Throwable cause)
{ {
CloseStatus closeStatus = closeStatusFor(cause); close(abnormalCloseStatusFor(cause), Callback.NOOP, false);
close(closeStatus, Callback.NOOP, false);
} }
/** /**
@ -498,8 +497,6 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
if (LOG.isDebugEnabled()) if (LOG.isDebugEnabled())
LOG.debug("close({}, {}, {})", CloseStatus.getCloseStatus(frame), callback, batch); LOG.debug("close({}, {}, {})", CloseStatus.getCloseStatus(frame), callback, batch);
System.err.println(behavior + " Closing " + closeConnection);
if (closeConnection) if (closeConnection)
{ {
callback = new Callback.Nested(callback) callback = new Callback.Nested(callback)
@ -507,20 +504,7 @@ public class WebSocketChannel implements IncomingFrames, FrameHandler.CoreSessio
@Override @Override
public void completed() public void completed()
{ {
System.err.println(behavior + " completed " + closeConnection); onClosed(null, channelState.getCloseStatus());
try
{
handler.onClosed(channelState.getCloseStatus());
}
catch (Throwable e)
{
LOG.warn(e);
}
finally
{
System.err.println(behavior + " connection.close ");
connection.close();
}
} }
}; };
} }