fixup! add log message when removing stale client

- add a constant for timeout duration in seconds
- set write deadline for client connections
- remove client from active connections on error
- log additional information when broadcasting fails

🤖

cmd/server/main.go

@@ -54,6 +54,7 @@ func Start() {
 	r := http.NewServeMux()
 	r.HandleFunc("/", websocket.HandleConnection)
 	go websocket.SendPermanentBroadCastMessage()
+	go websocket.RemoveStaleClients()
 
 	helper.Logger.Info("GoTomato started", "version", metadata.GoTomatoVersion)
 	helper.Logger.Info("Websocket listening", "address", listen)

internal/websocket/broadcast.go

@@ -12,6 +12,7 @@ import (
 // Sends continous messages to all connected WebSocket clients
 func SendPermanentBroadCastMessage() {
 	tick := time.NewTicker(time.Second)
+
 	for {
 		// Marshal the message into JSON format
 		jsonMessage, err := json.Marshal(shared.State)
@@ -19,14 +20,19 @@ func SendPermanentBroadCastMessage() {
 			helper.Logger.Error("Error marshalling message:", "msg", err)
 			return
 		}
+
 		// Iterate over all connected clients and broadcast the message
+		mu.Lock()
 		for _, client := range Clients {
-			err := client.SendMessage(websocket.TextMessage, jsonMessage)
+			// Send message to client
+			client.Conn.SetWriteDeadline(time.Now().Add(SEND_TIMEOUT * time.Second))
+			err := client.Conn.WriteMessage(websocket.TextMessage, jsonMessage)
 			if err != nil {
-				helper.Logger.Error("Error broadcasting to client:", "msg", err)
-				// The client is responsible for closing itself on error
+				helper.Logger.Error("Error broadcasting to client:", "msg", err, "host", client.RealIP, "clients", len(Clients))
 			}
 		}
+		mu.Unlock()
+
 		<-tick.C
 	}
 }

internal/websocket/client_commands.go

@@ -21,7 +21,10 @@ func handleClientCommands(c models.WebsocketClient) {
 
 		_, message, err := ws.ReadMessage()
 		if err != nil {
+			// remove client on error/disconnect
+			mu.Lock()
 			delete(Clients, ws.LocalAddr())
+			mu.Unlock()
 			helper.Logger.Info("Client disconnected:", "msg", err, "host", c.RealIP, "clients", len(Clients))
 			break
 		}

internal/websocket/handle_connection.go

@@ -2,18 +2,13 @@ package websocket
 
 import (
 	"github.com/gorilla/websocket"
-	"net"
 	"net/http"
-	"sync"
+	"time"
 
 	"git.smsvc.net/pomodoro/GoTomato/internal/helper"
 	"git.smsvc.net/pomodoro/GoTomato/pkg/models"
 )
 
-// Clients is a map of connected WebSocket clients, where each client is represented by the WebsocketClient struct
-var Clients = make(map[net.Addr]*models.WebsocketClient)
-var mu sync.Mutex // Mutex to protect access to the Clients map
-
 // Upgrade HTTP requests to WebSocket connections
 var upgrader = websocket.Upgrader{
 	CheckOrigin: func(r *http.Request) bool { return true },
@@ -31,9 +26,15 @@ func HandleConnection(w http.ResponseWriter, r *http.Request) {
 
 	// Register the new client
 	client := models.WebsocketClient{
-		Conn:   ws,
-		RealIP: r.RemoteAddr,
+		Conn:     ws,
+		LastPong: time.Now(),
+		RealIP:   r.RemoteAddr,
 	}
+	client.Conn.SetPongHandler(func(s string) error {
+		client.LastPong = time.Now()
+		return nil
+	})
+
 	mu.Lock()
 	Clients[ws.LocalAddr()] = &client
 	mu.Unlock()

internal/websocket/staleClients.go

@@ -0,0 +1,42 @@
+package websocket
+
+import (
+	"time"
+
+	"git.smsvc.net/pomodoro/GoTomato/internal/helper"
+	"git.smsvc.net/pomodoro/GoTomato/pkg/models"
+	"github.com/gorilla/websocket"
+)
+
+// Check and remove stale clients
+func RemoveStaleClients() {
+	ticker := time.NewTicker(STALE_CHECK_INTERVALL * time.Second)
+	defer ticker.Stop()
+
+	for range ticker.C {
+		mu.Lock()
+		for _, client := range Clients {
+			if !sendPing(client) || isStale(client) {
+				client.Conn.Close()
+				delete(Clients, client.Conn.LocalAddr())
+				helper.Logger.Info("Removed stale client", "host", client.RealIP)
+			}
+		}
+		mu.Unlock()
+	}
+}
+
+func sendPing(client *models.WebsocketClient) bool {
+	client.Conn.SetWriteDeadline(time.Now().Add(SEND_TIMEOUT * time.Second))
+	err := client.Conn.WriteMessage(websocket.PingMessage, nil)
+
+	if err != nil {
+		return false
+	}
+
+	return true
+}
+
+func isStale(client *models.WebsocketClient) bool {
+	return time.Since(client.LastPong) > (STALE_CLIENT_TIMEOUT * time.Second)
+}

internal/websocket/vars.go

@@ -0,0 +1,18 @@
+package websocket
+
+import (
+	"net"
+	"sync"
+
+	"git.smsvc.net/pomodoro/GoTomato/pkg/models"
+)
+
+const SEND_TIMEOUT = 10
+const STALE_CLIENT_TIMEOUT = 90
+const STALE_CHECK_INTERVALL = 30
+
+// Clients is a map of connected WebSocket clients, where each client is represented by the WebsocketClient struct
+var Clients = make(map[net.Addr]*models.WebsocketClient)
+
+// Mutex to protect access to the Clients map
+var mu sync.Mutex

pkg/models/client.go

@@ -1,9 +1,9 @@
 package models
 
 import (
-	"github.com/gorilla/websocket"
+	"time"
 
-	"git.smsvc.net/pomodoro/GoTomato/internal/helper"
+	"github.com/gorilla/websocket"
 )
 
 // Represents a command from the client (start/stop)
@@ -15,17 +15,7 @@ type ClientCommand struct {
 
 // Represents a single client
 type WebsocketClient struct {
-	Conn   *websocket.Conn
-	RealIP string
-}
-
-// Sends a message to the websocket.
-// Automatically locks and unlocks the client mutex, to ensure that only one goroutine can write at a time.
-func (c *WebsocketClient) SendMessage(messageType int, data []byte) error {
-	err := c.Conn.WriteMessage(messageType, data)
-	if err != nil {
-		helper.Logger.Error("Error writing to WebSocket:", "msg", err)
-		c.Conn.Close() // Close the connection on error
-	}
-	return err
+	Conn     *websocket.Conn
+	LastPong time.Time
+	RealIP   string
 }