Callback pattern for handling socket response

Question

I am in the process of learning go and am coming from a PHP, JS, and Nodejs background.

I created a package that is a client which connects to a socket server and processes the data received from the server. I am using a callback pattern and am wondering if this is frowned upon in the Go community. Here are the relevant code snippets:

// SocketClient allows handles the socket connection to a server
type SocketClient struct {
    Host string
    Path string
    conn *websocket.Conn
}

// SocketResponse function callback for data from socket
type SocketResponse func(res []byte, err error)

// Connect create the connection with the host
func (sc SocketClient) Connect(cb SocketResponse) {
    interrupt := make(chan os.Signal, 1)
    signal.Notify(interrupt, os.Interrupt)

    u := url.URL{Scheme: "wss", Host: sc.Host, Path: sc.Path}

    var err error
    sc.conn, _, err = websocket.DefaultDialer.Dial(u.String(), nil)

    if err != nil {
        cb(nil, err)
    }

    defer sc.conn.Close()

    done := make(chan struct{})

    // anonymous function call
    go func() {
        defer sc.conn.Close()
        defer close(done)
        for {
            _, message, err := sc.conn.ReadMessage()
            if err != nil {
                cb(nil, err)
            }
            cb(message, nil)
        }
    }()

    // ...
}

And then in another file I actually initialize SocketClient and call Connect...

sc.Connect(func(res []byte, err error) {
    // handle the response
})

Answer 1

I am using a callback pattern and am wondering if this is frowned upon in the Go community.

"callback pattern" shouldn't be frowned upon (they are used in "first-class functions" for instance: https://blog.golang.org/first-class-functions-in-go-and-new-go https://dave.cheney.net/2016/11/13/do-not-fear-first-class-functions)

However regarding your code, I don't think that it looks like idiomatic go code.

---

Don't just check errors, handle them gracefully

sc.conn, _, err = websocket.DefaultDialer.Dial(u.String(), nil)

if err != nil {
    cb(nil, err)
}

You pass the err to your callback, but you then continue inside the Connect function: if you couldn't dial, why do you continue?

---

Never start a goroutine without knowing how it will stop

go func() {
    defer sc.conn.Close()
    defer close(done)
    for {
        _, message, err := sc.conn.ReadMessage()
        if err != nil {
            cb(nil, err)
        }
        cb(message, nil)
    }
}()

The inner for loop will never stop (so will your goroutine).

---

One possibility would be to change it like this:

// SocketResponse function callback for data from socket
type SocketResponse func(res []byte)

// Connect create the connection with the host
func (sc SocketClient) Connect(cb SocketResponse) error {
    u := url.URL{Scheme: "wss", Host: sc.Host, Path: sc.Path}

    var err error
    sc.conn, _, err = websocket.DefaultDialer.Dial(u.String(), nil)

    if err != nil {
        return err
    }

    defer sc.conn.Close()
    for {
        _, message, err := sc.conn.ReadMessage()
        if err != nil {
            return err
        }
        cb(message)
    }
}

Some properties of this code :

• on dial error, it stops
• it reads all the messages until one error is encountered
• it blocks (but go sc.Connect() is easy to write)

Some possible evolutions:

• to continue even in case of errors, you could return a channel of errors (and send all errors encountered)
• also use a channel to report the received messages
• to control when the Connect method should stop, you could use a context or split your method in 3 parts: Connect, ReadMessage & Close