Skip to content

RuntimeException in WebSocketWorker should not stop server #207

Description

@sippykup

Here is the method I'm referring to, in WebSocketServer.java:

        public void run() {
            WebSocketImpl ws = null;
            try {
                while ( true ) {
                    ByteBuffer buf = null;
                    ws = iqueue.take();
                    buf = ws.inQueue.poll();
                    assert ( buf != null );
                    try {
                        ws.decode( buf );
                    } finally {
                        pushBuffer( buf );
                    }
                }
            } catch ( InterruptedException e ) {
            } catch ( RuntimeException e ) {
                handleFatal( ws, e );
            }
        }
    }

handleFatal() is what calls stop(). My argument is that you shouldn't treat this RuntimeException as a fatal error that you cannot recover from. Here's the stack trace that caused our production server to shut down today:

java.lang.NullPointerException
at org.java_websocket.WebSocketAdapter.getFlashPolicy(WebSocketAdapter.java:87)
at org.java_websocket.WebSocketImpl.decodeHandshake(WebSocketImpl.java:193)
at org.java_websocket.WebSocketImpl.decode(WebSocketImpl.java:161)
at org.java_websocket.server.WebSocketServer$WebSocketWorker.run(WebSocketServer.java:663)

That operation is just trying to decode a handshake from a single client. Anything unplanned that goes wrong while dealing with one client should not have the effect of shutting down the entire server. It should log an error, close the client connection, and move on.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions