Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async websocket onOpen, onError and onClose in 10.0.x #3290

Closed
gregw opened this issue Jan 23, 2019 · 3 comments
Closed

async websocket onOpen, onError and onClose in 10.0.x #3290

gregw opened this issue Jan 23, 2019 · 3 comments

Comments

@gregw
Copy link
Contributor

gregw commented Jan 23, 2019

No description provided.

gregw added a commit that referenced this issue Jan 23, 2019
Signed-off-by: Greg Wilkins <gregw@webtide.com>
@joakime
Copy link
Contributor

joakime commented Jan 24, 2019

Some test cases for you to work out ...

The onOpen throws an exception.

import javax.websocket.OnOpen;
import javax.websocket.Session;
import javax.websocket.server.ServerEndpoint;

@ServerEndpoint("fastfail")
public class FastFailSocket
{
    @OnOpen
    public void onOpen(Session session)
    {
        throw new RuntimeException("Intentional exception");
    }
}

The onOpen closes the connection.

import java.io.IOException;
import javax.websocket.CloseReason;
import javax.websocket.OnOpen;
import javax.websocket.Session;
import javax.websocket.server.ServerEndpoint;

@ServerEndpoint("fastclose")
public class FastCloseSocket
{
    @OnOpen
    public void onOpen(Session session)
    {
        try
        {
            session.close(new CloseReason(CloseReason.CloseCodes.TRY_AGAIN_LATER, "I'm not ready yet, shoo."));
        }
        catch (IOException e)
        {
            throw new RuntimeException("Ooops, unable to send close frame", e);
        }
    }
}

or a endpoint that just harshly disconnects.

import java.io.IOException;

import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
import org.eclipse.jetty.websocket.api.annotations.WebSocket;

@WebSocket
public class DropConnectionSocket
{
    @OnWebSocketConnect
    public void onConnect(Session session)
    {
        try
        {
            session.disconnect();
        }
        catch (IOException e)
        {
            throw new RuntimeException("Oops, unable to disconnect??", e);
        }
    }
}

Be sure to test the onClose and onError from both the server and client on these.

@joakime
Copy link
Contributor

joakime commented Jan 24, 2019

Jetty 9.4.x has a testcase that also ensures that the connections (and sessions) are cleaned up properly.

https://github.com/eclipse/jetty.project/blob/jetty-9.4.x/jetty-websocket/websocket-server/src/test/java/org/eclipse/jetty/websocket/server/ManyConnectionsCleanupTest.java

@joakime
Copy link
Contributor

joakime commented Jan 24, 2019

There is also the slow onOpen/onConnect method ...

import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;

import org.eclipse.jetty.websocket.api.RemoteEndpoint;
import org.eclipse.jetty.websocket.api.Session;
import org.eclipse.jetty.websocket.api.annotations.OnWebSocketConnect;
import org.eclipse.jetty.websocket.api.annotations.OnWebSocketMessage;
import org.eclipse.jetty.websocket.api.annotations.WebSocket;

@WebSocket
public class SlowOpenSocket
{
    private final AtomicBoolean stillInOnOpen = new AtomicBoolean(true);

    @OnWebSocketConnect
    public void onConnect(Session session)
    {
        try
        {
            RemoteEndpoint remote = session.getRemote();
            remote.sendString("hello");
            TimeUnit.SECONDS.sleep(2);
            remote.sendString("there");
            TimeUnit.SECONDS.sleep(2);
            remote.sendString("how's");
            TimeUnit.SECONDS.sleep(2);
            remote.sendString("it");
            TimeUnit.SECONDS.sleep(2);
            remote.sendString("going?");
            stillInOnOpen.compareAndSet(true, false);
        }
        catch (Exception e)
        {
            throw new RuntimeException("Ooops, unable to send opening messages", e);
        }
    }

    @OnWebSocketMessage
    public void onMessage(Session session, String msg)
    {
        if (stillInOnOpen.get())
        {
            throw new RuntimeException("Shouldn't have received messages yet");
        }
    }
}

gregw added a commit that referenced this issue Jan 26, 2019
Gave onOpen, onError and onClose callback signatures
Illegal to ask for demand prior to onOpen success
added tests for various onOpen scenarios

Signed-off-by: Greg Wilkins <gregw@webtide.com>
@gregw gregw changed the title Review and test websocket onOpen states in 10.0.x async websocket onOpen, onError and onClose in 10.0.x Jan 26, 2019
gregw added a commit that referenced this issue Jan 28, 2019
Changes after review:
 + removed Adaptor from FrameHandler, so all non test usages of
   FrameHandler now use async API.
 + Fixed sequencing of multiple async operation so callback is notified
   after completion (created more Callback.from utilities for this)
 + fixed import order

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Jan 28, 2019
Changes after review:
 + fixed import order

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Jan 28, 2019
Changes after review:
 + failure to send abnormal close closes connection prior to failing
   callback.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Jan 29, 2019
Fixed OSGi tests.

Signed-off-by: Greg Wilkins <gregw@webtide.com>
gregw added a commit that referenced this issue Jan 30, 2019
Issue #3290 async websocket onOpen, onError and onClose
@gregw gregw closed this as completed Feb 5, 2019
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 6, 2019
when the WSConnection reads EOF it now notifies the WSChannel
the channel instead of handling it locally

fixed FlusherFlusher failure issues

fixed issue with the WebSocketCloseTest expecting close reason

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 6, 2019
introduce channelState check in the catch in WSChannel sendFrame
to guard from multiple closes

WebSocketConnection fillAndParse will now try to read until EOF

removed state change in the isOutputOpen check in webSocketChannelState
to as we do the state change in the catch block in WSChannel

added and improved WebSocketCloseTest to test more cases

Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
lachlan-roberts added a commit to lachlan-roberts/jetty.project that referenced this issue Feb 7, 2019
Signed-off-by: Lachlan Roberts <lachlan@webtide.com>
gregw added a commit that referenced this issue Feb 17, 2019
…cket-close

Issue #3290 - FrameFlusher and WebSocket close fixes
sbordet added a commit that referenced this issue Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants