square / okhttp

Square’s meticulous HTTP client for the JVM, Android, and GraalVM.
https://square.github.io/okhttp/
Apache License 2.0
45.86k stars 9.16k forks source link

WebSocket not properly closed #6510

Closed ln-12 closed 3 years ago

ln-12 commented 3 years ago

I am using this multiplatform approach to use websockets on Android and iOS. On Android, okhttp is used. The problem is that I only get to the state CLOSING, but not CLOSED when I close the connection on the server side. To demonstrate it, I use this python script as server:

# WS server example

import asyncio
import websockets

async def hello(websocket, path):
    while True:
        try:
            msg = await websocket.recv()
        except websockets.ConnectionClosed:
            print(f"Terminated")
            break

        print(msg)

        await websocket.send(msg)
        await websocket.close()

start_server = websockets.serve(hello, "192.168.188.24", 8765)

asyncio.get_event_loop().run_until_complete(start_server)
asyncio.get_event_loop().run_forever()

For the client my implementation looks the following (note: technically it directly maps to okhttp methods/callback):

//Common
class AppSocket(url: String) {
    private val ws = PlatformSocket(url)

    private val socketListener: PlatformSocketListener = object : PlatformSocketListener {
        override fun onOpen() {
            Log.error { "OPEN" }
        }

        override fun onFailure(t: Throwable) {
            Log.error {"FAILURE" }
            t.printStackTrace()
        }

        override fun onMessage(msg: String) {
            Log.error {"MESSAGE: $msg" }
        }

        override fun onClosing(code: Int, reason: String) {
            Log.error { "CLOSING" }
        }

        override fun onClosed(code: Int, reason: String) {
            Log.error {"CLOSED" }
        }
    }

    fun test() {
        ws.openSocket(socketListener)
        ws.sendMessage(msg)
    }
}

The log output simply is:

E/Log: OPEN
E/Log: MESSAGE: TEST
E/Log: CLOSING

On iOS, I reach the state CLOSED. Using the Android debugger, I found out that toClose (see here is null. Is this intended behaviour? How would I achieve getting the callback for CLOSED?

swankjesse commented 3 years ago

You need to call close() when you're done with the web socket. https://square.github.io/okhttp/4.x/okhttp/okhttp3/-web-socket/-factory/new-web-socket/

ln-12 commented 3 years ago

Thanks for the link. I see that I need to close it, if I (the client) end the connection. But I can't see why it should be intended to leave the websocket in CLOSING state when the server closes the connection.

So would you suggest to do something like the following? That seems wrong for me. In what use case would I need a websocket anymore that is in CLOSING state?

override fun onClosing(code: Int, reason: String) {
    Log.error { "CLOSING" }

    ws.closeSocket(code, reason)
}
swankjesse commented 3 years ago

You're not strictly required to terminate immediately upon receipt of a close frame; you can finish up first. Once you've done that you gotta close though.

   If an endpoint receives a Close frame and did not previously send a
   Close frame, the endpoint MUST send a Close frame in response.  (When
   sending a Close frame in response, the endpoint typically echos the
   status code it received.)  It SHOULD do so as soon as practical.  An
   endpoint MAY delay sending a Close frame until its current message is
   sent (for instance, if the majority of a fragmented message is
   already sent, an endpoint MAY send the remaining fragments before
   sending a Close frame).  However, there is no guarantee that the
   endpoint that has already sent a Close frame will continue to process
   data.

https://tools.ietf.org/html/rfc6455#section-5.5.1

ln-12 commented 3 years ago

Thank you very much for the explanation.