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

onResume and onDisconnect are not fired with WebSocket connection. #814

Closed
flowersinthesand opened this issue Jan 12, 2013 · 13 comments
Closed

Comments

@flowersinthesand
Copy link
Member

Full story: flowersinthesand/portal-java#21

onResume or onDisconnect are not fired with WebSocket connection. When

Simple question - Is it safe to close WebSocket connection using cancel method of AtmosphereResourceImpl? As far as I remember, resume method couldn't disconnect WebSocket so that the close event is not fired in the browser side, so I used cancel method instead. However I'm not sure.

@jfarcand
Copy link
Member

OK just tested with 1.0.x and it worked as expected using the sample/chat. Now trying with 1.1.0-SNAPSHOT (master)

jfarcand added a commit that referenced this issue Jan 14, 2013
@jfarcand
Copy link
Member

Added to the samples/chat support for onDisconnect events Still unable to reproduce. Maybe you means AtmosphereResource.cancel() isn't calling onDisconnect?

@jfarcand
Copy link
Member

Ok some explanation. The AtmosphereResourceImpl.cancel() will close the underlying WebSocket, but this is not a public interface. With resume, the AtmosphereResourceImpl will be removed from Broadcaster, but the underlying connection will not be closed. The recommended way is to call AtmosphereResponse.closeStreamOrWriter(), which will close the underlying websocket.

I think I need to work on a new API that close the websocket directly as well as the connection, independent of the transport.

jfarcand added a commit that referenced this issue Jan 14, 2013
@jfarcand
Copy link
Member

OK added both a test for onResume, onDisconnect and onClose here I cannot reproduce the issue with Jetty.

@flowersinthesand
Copy link
Member Author

Okay, I'll test it and get back to you

@flowersinthesand
Copy link
Member Author

About WebSocket and 'AtmosphereResponse.closeStreamOrWriter':

Both r.resume(); r.getResponse().closeStreamOrWriter(); and r.resume(); fire either onResume or onDisconnect event in the server side but, don't fire close event of the WebSocket in the client side. I think this is due to resume method. Only r.getResponse().closeStreamOrWriter(); does nothing even in the server side.

closeStreamOrWriter doesn't affect the WebSocket connection -

if (resource() != null && resource().transport() != AtmosphereResource.TRANSPORT.WEBSOCKET) {

@flowersinthesand
Copy link
Member Author

With 1.1.0-SNAPSHOT, problem still occurs. Here's the scenario, and I'll check about added unit testss.

Action

  • Use this AtmosphereHandler
  • Open chrome developer tools' console and type the client scripts
  • Close chrome if 'get out' message comes out
  • Check there was onDisconnect or onResume event.

Situation 1 - error
Client:

var ws = new WebSocket("ws://localhost:8080/test-with-atmosphere");
ws.onopen = function() {
    ws.send("nothing");
    console.log('get out.');
};

Log:

2013-01-15 22:18:49,124 DEBUG (org.atmosphere.websocket.DefaultWebSocketProcessor:127) - Atmosphere detected WebSocket: org.atmosphere.container.version.Jetty8WebSocket
2013-01-15 22:18:49,134 DEBUG (com.github.flowersinthesand.samples.TestAtmosphereHandler:44) - onPreSuspend AtmosphereResourceEventImpl{isCancelled=false,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode2364305,
 action=Action{timeout=-1, type=CREATED},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@80d028,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@e229be, com.github.flowersinthesand.samples.TestAtmosphereHandler$1@22b8ff]}}
2013-01-15 22:18:49,134 DEBUG (com.github.flowersinthesand.samples.TestAtmosphereHandler:49) - onSuspend AtmosphereResourceEventImpl{isCancelled=false,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode2364305,
 action=Action{timeout=-1, type=SUSPEND},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@80d028,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@e229be, com.github.flowersinthesand.samples.TestAtmosphereHandler$1@22b8ff]}}
2013-01-15 22:18:51,265 DEBUG (org.atmosphere.cpr.AsynchronousProcessor:529) - Cancelling the connection for request AtmosphereRequest{ contextPath= servletPath=/test-with-atmosphere pathInfo=null requestURI=/test-with-atmosphere requestURL=http://localhost:8080/test-with-atmosphere destroyable=false}
2013-01-15 22:19:09,141 DEBUG (com.github.flowersinthesand.samples.TestAtmosphereHandler:53) - Time to bid farewell
2013-01-15 22:19:09,141 DEBUG (org.atmosphere.cpr.AtmosphereResourceImpl:306) - Cannot resume an already resumed/cancelled request AtmosphereResourceImpl{
 hasCode2364305,
 action=Action{timeout=-1, type=RESUME},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@80d028,
 serializer=null,
 isInScope=false,
 listeners=[]}

No onResume and onDisconnect.

Situation 2
Client:

var ws = new WebSocket("ws://localhost:8080/test-with-atmosphere");
ws.onopen = function() {
    // ws.send("nothing");
    console.log('get out.');
};

Log:

2013-01-15 22:21:04,866 DEBUG (org.atmosphere.websocket.DefaultWebSocketProcessor:127) - Atmosphere detected WebSocket: org.atmosphere.container.version.Jetty8WebSocket
2013-01-15 22:21:04,876 DEBUG (com.github.flowersinthesand.samples.TestAtmosphereHandler:44) - onPreSuspend AtmosphereResourceEventImpl{isCancelled=false,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode13246582,
 action=Action{timeout=-1, type=CREATED},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@1961f4,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@6e119d, com.github.flowersinthesand.samples.TestAtmosphereHandler$1@469729]}}
2013-01-15 22:21:04,886 DEBUG (com.github.flowersinthesand.samples.TestAtmosphereHandler:49) - onSuspend AtmosphereResourceEventImpl{isCancelled=false,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode13246582,
 action=Action{timeout=-1, type=SUSPEND},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@1961f4,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@6e119d, com.github.flowersinthesand.samples.TestAtmosphereHandler$1@469729]}}
2013-01-15 22:21:05,977 DEBUG (org.atmosphere.cpr.AsynchronousProcessor:529) - Cancelling the connection for request AtmosphereRequest{ contextPath= servletPath=/test-with-atmosphere pathInfo=null requestURI=/test-with-atmosphere requestURL=http://localhost:8080/test-with-atmosphere destroyable=false}
2013-01-15 22:21:05,978 DEBUG (com.github.flowersinthesand.samples.TestAtmosphereHandler:76) - onDisconnect AtmosphereResourceEventImpl{isCancelled=true,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode13246582,
 action=Action{timeout=-1, type=CANCELLED},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@1961f4,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@6e119d, com.github.flowersinthesand.samples.TestAtmosphereHandler$1@469729]}}
2013-01-15 22:21:24,893 DEBUG (com.github.flowersinthesand.samples.TestAtmosphereHandler:53) - Time to bid farewell
2013-01-15 22:21:24,893 DEBUG (org.atmosphere.cpr.AtmosphereResourceImpl:306) - Cannot resume an already resumed/cancelled request AtmosphereResourceImpl{
 hasCode13246582,
 action=Action{timeout=-1, type=RESUME},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@1961f4,
 serializer=null,
 isInScope=false,
 listeners=[]}

There was onDisconnect in 2013-01-15 22:21:05,978

@jfarcand
Copy link
Member

OK, first, like you noted, there is no API for closing a WebSocket connection. The resume will not close the connection. So I have filled #817.

Now from your log I do see the onDisconnect called...but I may miss something, so let me try with your handler.

@jfarcand
Copy link
Member

OK, I installed your handler, ran the javascript, and got the following trace if I let the timer execute (so I don't close the client)

INFO] Starting scanner at interval of 1 seconds.
13:26:43.726 [qtp2006761955-63] DEBUG o.a.w.DefaultWebSocketProcessor - Atmosphere detected WebSocket: org.atmosphere.container.version.Jetty8WebSocket
13:26:43.739 [qtp2006761955-63] DEBUG o.a.s.pubsub.TestAtmosphereHandler - onPreSuspend AtmosphereResourceEventImpl{isCancelled=false,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode1229554982,
 action=Action{timeout=-1, type=CREATED},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@79c4a760,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@5dc8ce14, org.atmosphere.samples.pubsub.TestAtmosphereHandler$1@2a51b326]}}
13:26:43.740 [qtp2006761955-63] DEBUG o.a.s.pubsub.TestAtmosphereHandler - onSuspend AtmosphereResourceEventImpl{isCancelled=false,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode1229554982,
 action=Action{timeout=-1, type=SUSPEND},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@79c4a760,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@5dc8ce14, org.atmosphere.samples.pubsub.TestAtmosphereHandler$1@2a51b326]}}
13:27:03.742 [Timer-0] DEBUG o.a.s.pubsub.TestAtmosphereHandler - Time to bid farewell
13:27:03.743 [Timer-0] DEBUG o.a.s.pubsub.TestAtmosphereHandler - onResume AtmosphereResourceEventImpl{isCancelled=false,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode1229554982,
 action=Action{timeout=-1, type=RESUME},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@79c4a760,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@5dc8ce14, org.atmosphere.samples.pubsub.TestAtmosphereHandler$1@2a51b326]}}
java.lang.Exception: Stack trace
    at java.lang.Thread.dumpStack(Thread.java:1273)
    at org.atmosphere.samples.pubsub.TestAtmosphereHandler$1.onDisconnect(TestAtmosphereHandler.java:92)
    at org.atmosphere.cpr.AtmosphereResourceImpl.onDisconnect(AtmosphereResourceImpl.java:697)
    at org.atmosphere.cpr.AtmosphereResourceImpl.notifyListeners(AtmosphereResourceImpl.java:636)
    at org.atmosphere.cpr.AtmosphereResourceImpl.resume(AtmosphereResourceImpl.java:311)
    at org.atmosphere.samples.pubsub.TestAtmosphereHandler$1$1.run(TestAtmosphereHandler.java:70)
    at java.util.TimerThread.mainLoop(Timer.java:512)
    at java.util.TimerThread.run(Timer.java:462)
13:27:03.744 [Timer-0] DEBUG o.a.s.pubsub.TestAtmosphereHandler - onDisconnect AtmosphereResourceEventImpl{isCancelled=true,
 isResumedOnTimeout=false,
 throwable=null,
 message=null,
     resource=AtmosphereResourceImpl{
 hasCode1229554982,
 action=Action{timeout=-1, type=RESUME},
 broadcaster=org.atmosphere.cpr.DefaultBroadcaster,
 asyncSupport=org.atmosphere.container.JettyAsyncSupportWithWebSocket@79c4a760,
 serializer=null,
 isInScope=true,
 listeners=[org.atmosphere.interceptor.DefaultHeadersInterceptor$1@5dc8ce14, org.atmosphere.samples.pubsub.TestAtmosphereHandler$1@2a51b326]}}

So both onDisconnect and onResume worked.

@jfarcand
Copy link
Member

OK confirm the issue with closing the browser and ondisconnect not called. Looking at it now. Thanks!

@jfarcand
Copy link
Member

Ah! The issue is the onClose is called, but not the onDisconnected. Fixing.

@jfarcand
Copy link
Member

Oh! The issue is you must extends WebSocketEventListener, not the AtmosphereResourceEventListener for getting the WebSocket's special events. Changing your AtmosphereHandler to use it make it work. Sorry I should have spotted the issue much before. I will mark the issue for documentation for 1.1

@flowersinthesand
Copy link
Member Author

Thanks, I confirmed that it works with WebSocketEventListener.

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

No branches or pull requests

2 participants