coder / websocket

Minimal and idiomatic WebSocket library for Go
ISC License
3.98k stars 298 forks source link

failed: Close received after close #448

Open chjiyun opened 7 months ago

chjiyun commented 7 months ago

Hello, I recently encountered a problem. Can someone help me take a look?, I used the example and successfully ran it, but I sent a ping to the server and received an error below:

WebSocket connection to 'ws://127.0.0.1:8888/subscribe' failed: Close received after close
nhooyr commented 6 months ago

Can you show your client code?

chjiyun commented 6 months ago

@nhooyr go version 1.22.2

conn.addEventListener('open', ev => {
    console.info('websocket connected')
    setTimeout(() => {
      conn.send('ping')
    }, 3000)
  })
nhooyr commented 6 months ago

Unrelated to the library. Something's wrong with your client code calling close after the connection has been closed. See https://github.com/mqttjs/MQTT.js/issues/1361

fogrye commented 6 months ago

I would reopen this as after last update I have received same error with native WebSocket in Chrome, while safari is ok with that.

nhooyr commented 6 months ago

Ok possible we need to add a guard in c.writeClose to not write a close frame twice. I think that we're writing a close frame, we get a close frame back and then the code doesn't keep track that it already wrote a close frame and so writes a close frame in response to the handshake close frame.

fogrye commented 6 months ago

Yes, I saw that it's waiting for closing but CloseNow did not check if it was closed already. I believe that's what's happening.

dgpc commented 2 weeks ago

Is https://github.com/coder/websocket/pull/476 the chosen approach here, and if so anything we can do to get it merged? My application is experiencing this same issue.

I also noticed another project had decided to fork the library in order to pick up this solution: https://github.com/flowey-org/websocket/commit/b1784e66892beb81fbfa91d896d35bc605f6b5f6.

mafredri commented 2 weeks ago

@dgpc yes, that's the chosen approach. The only blocker is that I've been meaning to do some additional testing but haven't gotten around to it. If users like yourself are able to provide feedback whether or not it works as expected/fixes the issue, that would be helpful but not a requirement. I'll try to get it merged this or early next week.