How to register new websocket connections gofiber

Question

I'm trying to set up a simple websocket server that should serve the client some content at unknown intervals.

My code currently looks like this:

router.go

func SetupRoutes(app *fiber.App) error {

    app.Get("/whop/validate", handler.HandleWhopValidate)
    /*Other non-websocket routes*/

    /*...*/

    app.Get("/ws/monitor", websocket.New(wsHandler.HandleWsMonitor))

    app.Use(func(c *fiber.Ctx) error {
        c.SendStatus(404)
        return c.Next()
    })

    return nil
}

handler.go

package handlers

import (
    "fmt"
    "log"

    "github.com/gofiber/websocket/v2"
)

var register = make(chan *websocket.Conn)
var unregister = make(chan *websocket.Conn)

func HandleWsMonitor(c *websocket.Conn) {
    go SocketListener()
    defer func() {
        unregister <- c
        //may need to check whether connection is already closed before re-closing?
        c.Close()
    }()
    //sends conn into channel
    register <- c
    for {
        messageType, message, err := c.ReadMessage()
        if err != nil {
            if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) {
                log.Println("read error:", err)
            }
            return
        }
        if messageType == websocket.TextMessage {
            log.Println("got textmessage:", string(message))
        } else {
            log.Println("received message of type:", messageType)
        }
    }
}

func SocketListener() {
    for {
        select {
        case c := <-register:
            messageType, message, err := c.ReadMessage()
            if err != nil {
                log.Println(err)
                unregister <- c
                return
            }

            fmt.Printf("Got message of type: %d\nMessage:%s\n", messageType, string(message))
            fmt.Printf("Connection Params: %s\n", c.Params("id"))
            //append to list of co
        case c := <-unregister:
            //remove conection from list of clients
            c.Close()
            fmt.Printf("Closed connection\n")

        }

    }
}

The issue I'm having is that when I connect to the websocket, my select case for register is not hit (I'd want to register the client connection to a map using a uuid that is previously provided to the client).

client.go

package main

import (
    "flag"
    "log"
    "net/url"

    "github.com/fasthttp/websocket"
)

type Client struct {
    C *websocket.Conn
}

func main() {
    addr := flag.String("addr", "localhost:8080", "http service address")
    u := url.URL{
        Scheme:   "ws",
        Host:     *addr,
        Path:     "/ws/monitor",
        RawQuery: "id=12",
    }
    wsClient := &Client{}

    log.Printf("connecting to %s\n", u.String())
    // Connect to the WebSocket server
    conn, resp, err := websocket.DefaultDialer.Dial(u.String(), nil)
    if err != nil {
        log.Fatal("Dial:", err)
    }
    wsClient.C = conn
    if resp != nil {
        log.Println("Got response:", resp)
    }
    defer wsClient.closeConn()
}

func (client *Client) closeConn() {
    err := client.C.WriteMessage(
        websocket.CloseMessage,
        websocket.FormatCloseMessage(websocket.CloseNormalClosure, ""),
    )
    if err != nil {
        log.Println("Write close:", err)
        return
    }
    client.C.Close()
    log.Println("Connection closed")
}

Is there something I'm missing in handler.go or should i just be taking a different approach when connecting to the server with my client?

Answer 1

The select case for register did hit according to my test (the code I used is attached to the bottom of this answer).

But I found other issues in the code:

1. The unregister chan is unbuffered, and unregister <- c in SocketListener will be blocked. When the code reaches unregister <- c, there is a deadlock between it and case c := <-unregister.
2. It seems we only need one SocketListener goroutine for the whole server. If this is the case, it should be moved outside of HandleWsMonitor.
3. Both HandleWsMonitor and SocketListener read from the connection. What's the responsibility of SocketListener? It seems that it should not read from the connection.

Thinking it more, it seems that you can add connections to and delete connections from the map in HandleWsMonitor directly. SocketListener can be removed entirely. Simplicity should be a key goal in design. See the KISS principle.

package main

import (
    "log"

    "github.com/gofiber/fiber/v2"
    "github.com/gofiber/websocket/v2"
)

var (
    register   = make(chan *websocket.Conn)
    unregister = make(chan *websocket.Conn)
)

func main() {
    // Make it easy to find out which line prints the log.
    log.SetFlags(log.Lshortfile)
    app := fiber.New()

    app.Get("/ws/monitor", websocket.New(HandleWsMonitor))

    log.Fatal(app.Listen(":8080"))
}

func HandleWsMonitor(c *websocket.Conn) {
    // It seems the we only need one SocketListener goroutine for the whole server.
    // If this is the case, the next line should be moved outside of this func.
    go SocketListener()
    defer func() {
        unregister <- c
        c.Close()
    }()

    register <- c
    for {
        messageType, message, err := c.ReadMessage()
        if err != nil {
            if websocket.IsUnexpectedCloseError(err, websocket.CloseGoingAway, websocket.CloseAbnormalClosure) {
                log.Println("read error:", err)
            }
            return
        }
        if messageType == websocket.TextMessage {
            log.Println("got textmessage:", string(message))
        } else {
            log.Println("received message of type:", messageType)
        }
    }
}

func SocketListener() {
    for {
        select {
        case c := <-register:
            // This did appear in the log.
            log.Println("case c := <-register")
            messageType, message, err := c.ReadMessage()
            if err != nil {
                log.Println(err)
                // unregister is unbuffered, the sending will be blocked.
                unregister <- c
                // If we use only one SocketListener goroutine then it should
                // not return here.
                return
            }

            log.Printf("Got message of type: %d\nMessage:%s\n", messageType, string(message))
            log.Printf("Connection Params: %s\n", c.Params("id"))
        case c := <-unregister:
            c.Close()
            log.Println("Closed connection")

        }
    }
}