Atmosphere / wasync

WebSockets with fallback transports client library for Node.js, Android and Java
http://async-io.org
161 stars 47 forks source link

StreamTransport triggers Event.REOPENED after each message when protocol is enabled #173

Closed adrianhj closed 1 year ago

adrianhj commented 1 year ago

As part of our lifecycle management for a third party integration requiring the use of wAsync for event subscriptions we are noticing that the REOPENED event is being triggered after every message received over the socket after it has been opened.

i.e. once connected we observe an event stream like:

TRANSPORT
STATUS
OPEN
MESSAGE
REOPENED
MESSAGE
REOPENED
...

Our expectation from the documentation/code comments was that OPEN/REOPENED events are only received once per active connection ahead of application MESSAGE events starting to indicate that the connection is now open (or re-opened) after the initial handshake has occured.

In terms of building the request/socket, we are not doing anything exotic from what I can see, ClientFactory.getDefault().newClient(AtmosphereClient.class); + socket.on(...) to subscribe to the various events.

Reviewing the code for StreamTransport it seems that triggerOpen() is continuously invoked via https://github.com/Atmosphere/wasync/blob/master/wasync/src/main/java/org/atmosphere/wasync/transport/StreamTransport.java#L167 and should possibly be gated to only event if the Socket status is not already OPEN as a start. It seemed like a good first contribution for someone in my team so have asked them to prepare a PR for review/discussion.

Separately I'm not sure if unlockFuture should be a bit smarter to only run its wrapped logic if the future is not already done, however looking at the reconnection logic I'm not sure this ever gets reset after the initial connection.

jfarcand commented 1 year ago

Fixed by https://github.com/Atmosphere/wasync/pull/174 . @adrianhj Do you need an official release?

Auroralias commented 1 year ago

I've opened #175 to add some coverage to my changes.

If you're happy to merge that, could we have an official release please?

jfarcand commented 1 year ago

3.0.2 has been released. Thanks!