Ws: WS reports invalid error code

[herkyl]

Description

I am using Gorilla Websockets to send a close frame to a connected client (the client being ws). The close frame is a standard 2 byte integer code + message.

I am setting the code to be 1008, however ws reports it as code: 1006. I know that Gorilla sends out the correct message because I have debugged what the first two bytes of the message are and the same code works correctly in Chrome, showing the correct 1008 code.

Reproducible in:

version: 3.0.0
Node.js version(s): 6.9.2
OS version(s): OSX 10.12.3

Steps to reproduce:

1. I am using this Go code to close the connect with an event:

cm := websocket.FormatCloseMessage(websocket.ClosePolicyViolation, msg)
if err := c.conn.WriteMessage(websocket.CloseMessage, cm); err != nil {
    log.Error(err)
}
err = c.conn.Close()

1. Listen to the close event of the ws connection and check the code.

Expected result:

I expect the close code to be 1008

Actual result:

The code is 1006

[lpinca]

Do you know if the close handshake is respected and the connection is closed cleanly? This example works as expected.

const WebSocket = require('ws');

const wss = new WebSocket.Server({ port: 3000 }, () => {
  const ws = new WebSocket('ws://localhost:3000');

  ws.on('close', (code, reason) => {
    console.log(code, reason);
  });
});

wss.on('connection', (ws) => ws.close(1008));

I wonder if the socket is closed with an error: https://nodejs.org/dist/latest/docs/api/net.html#net_event_close_1. In that case the close code is set to 1006 even if the close frame was received by the client.

[lpinca]

I tried with this code:

package main

import (
    "log"
    "net/http"

    "github.com/gorilla/websocket"
)

var upgrader = websocket.Upgrader{}

func echo(w http.ResponseWriter, r *http.Request) {
    c, err := upgrader.Upgrade(w, r, nil)
    if err != nil {
        log.Print(err)
        return
    }
    cm := websocket.FormatCloseMessage(websocket.ClosePolicyViolation, "")
    err = c.WriteMessage(websocket.CloseMessage, cm)
    if err != nil {
        log.Print(err)
    }
    err = c.Close()
    if err != nil {
        log.Print(err)
    }
}

func main() {
    log.SetFlags(0)
    http.HandleFunc("/echo", echo)
    log.Fatal(http.ListenAndServe("localhost:3000", nil))
}

The close frame is correctly received by the client but an error is emitted on the socket.

Error: read ECONNRESET
    at exports._errnoException (util.js:1026:11)
    at TCP.onread (net.js:607:25)

so the close code is set to 1006. This seems correct to me.

To get the expected result in the issue description we can update this if condition as follows:

if (!this._closeCode && error) this._closeCode = 1006;

but I'm not sure if it makes sense.

[herkyl]

Thanks for getting back to me.
I don't fully understand where the error is emitted. I'm testing it right now and onerror is not emitting anything, if that is what you meant.
Also how does wss.on('connection', (ws) => ws.close(1008)); work differently than the Go code?

[lpinca]

ws.close() works a bit differently. It implements the close handshake. The connection is not immediately closed after sending the data. It waits for the other peer to send back a close frame.

This is how it works in ws:

            +----------+  +-----------+  +----------+
     - - - -|ws.close()|->|close frame|->|ws.close()|- - - -
    |       +----------+  +-----------+  +----------+       |
            +----------+  +-----------+        |
    |       |ws.close()|<-|close frame|<-------+            |
            +----------+  +-----------+        |
 CLOSING          |           +---+            |         CLOSING
                  |        +--|fin|<-----------+
    |             |        |  +---+                         |
                  |        |  +---+     +-------------+
    |             +--------+->|fin|---->|ws.finalize()| - - +
                           |  +---+     +-------------+
    |      +-------------+ |
     - - - |ws.finalize()|<+
           +-------------+

In the go code the connection is closed without reading data that has been sent to it and I think this is why an ECONNRESET error is emitted.

[herkyl]

Interesting. I found that Gorilla often uses a sleep before closing the connection https://github.com/gorilla/websocket/blob/5ade364dff1f03abefdae800ed19074bf707b08f/examples/command/main.go#L77 but it seems kind of dirty.
Maybe I could instead wait for the close frame from the socket myself. Could you please show me the part in ws that handles this, I will try to duplicate it in Go.

[lpinca]

1. It starts in the close method.
2. When a peer receives a close frame it calls the close method.
3. And finally is handled by the event listeners on the socket.

[lpinca]

Hmm I think my explanation was similar to this

so if something isn't clear just ask.

[garyburd]

I found that Gorilla often uses a sleep before closing the connection https://github.com/gorilla/websocket/blob/5ade364dff1f03abefdae800ed19074bf707b08f/examples/command/main.go#L77 but it seems kind of dirty.

The sleep and close calls in the Gorilla example ensures that the connection is closed in the case where the client does not reply to the close message. If you want to rely on the client replying to the close message, then delete the call to time.Sleep() and ws.Close(). Do not delete the call to time.Sleep() without also deleting the call to ws.Close().

[herkyl]

Thanks for all the help!
Since I misunderstood how closing works in the first place I designed an erroneous flow for handling the closing on my part. You are correct, the sleep is simply there to ensure that if the handshake doesn't finish the connection still gets closed.
I will close the issue.