fixe added onClose

[nkzawa]

Thanks for your PR! Am I correct you confirmed this PR fixes NullPointerException https://github.com/socketio/socket.io-client-java/issues/343 ?

[shaacker]

In my change, I did the call to close() intentionally, because it checks the ready state and ALSO calls onClose() which updates the internal state properly.

public Transport close() {
    EventThread.exec(new Runnable() {
        @Override
        public void run() {
            if (Transport.this.readyState == ReadyState.OPENING || Transport.this.readyState == ReadyState.OPEN) {
                Transport.this.doClose();
                Transport.this.onClose();
            }
        }
    });
    return this;
}

How does this change improve the situation? I imagine it would leave the internal readyState still at ReadyState.OPEN which would then mess with reconnect attempts.

@nkzawa Please correct me if I'm wrong.

[nkzawa]

@chendrak I expect the onClose listener of websocket would be called in this case anyway. (https://github.com/socketio/engine.io-client-java/blob/master/src/main/java/io/socket/engineio/client/transports/WebSocket.java#L136) If it's true, we wouldn't need to change the ready state here IMO.

[shaacker]

@nkzawa Ah, perfect. In that case the close() might be the better option.

[eckovation]

@nkzawa @chendrak I just checked the the library by compiling it into a real app, and i can confirm that, this fix, fixes the issue.

I tried, regenerating it through following steps, Put the debugger on Line#345 (Socket.java) and before it executes that line, disconnect the internet. And after that it did not crash on Line#346 (Socket.java) as it used to, when we close the transport using close()

So it seems, doClose() is indeed working.

Can you guys please share your thoughts on this?

[eckovation]

@nkzawa if its all good, can you merge it?

[nkzawa]

@eckovation thank you!