Changed send to be synchronous. Wrote session inner gorountines.

transport/connection.go

@@ -45,15 +45,15 @@ type Connection struct {
 	logger *slog.Logger
 
 	incoming chan []byte
-	outgoing chan []byte
 	errors   chan error
 	done     chan struct{}
 
 	state ConnectionState
 
-	wg     sync.WaitGroup
-	closed bool
-	mu     sync.RWMutex
+	wg      sync.WaitGroup
+	closed  bool
+	mu      sync.RWMutex
+	writeMu sync.Mutex
 }
 
 func NewConnection(urlStr string, config *ConnectionConfig, logger *slog.Logger) (*Connection, error) {
@@ -77,7 +77,6 @@ func NewConnection(urlStr string, config *ConnectionConfig, logger *slog.Logger)
 		config:   config,
 		logger:   logger,
 		incoming: make(chan []byte, 100),
-		outgoing: make(chan []byte, 100),
 		errors:   make(chan error, 10),
 		state:    StateDisconnected,
 		done:     make(chan struct{}),
@@ -108,7 +107,6 @@ func NewConnectionFromSocket(
 		config:   config,
 		logger:   logger,
 		incoming: make(chan []byte, 100),
-		outgoing: make(chan []byte, 100),
 		errors:   make(chan error, 10),
 		state:    StateConnected,
 		done:     make(chan struct{}),
@@ -119,7 +117,6 @@ func NewConnectionFromSocket(
 	}
 
 	conn.startReader()
-	conn.startWriter()
 
 	return conn, nil
 }
@@ -166,7 +163,6 @@ func (c *Connection) Connect(ctx context.Context) error {
 	}
 
 	c.startReader()
-	c.startWriter()
 
 	return nil
 }
@@ -221,63 +217,32 @@ func (c *Connection) startReader() {
 
 }
 
-func (c *Connection) startWriter() {
-	c.wg.Add(1)
-	go func() {
-		defer c.wg.Done()
-
-		for {
-			select {
-			case <-c.done:
-				return
-			case data := <-c.outgoing:
-				if c.config.WriteTimeout > 0 {
-					if err := c.socket.SetWriteDeadline(time.Now().Add(c.config.WriteTimeout)); err != nil {
-						if c.logger != nil {
-							c.logger.Error("write deadline error", "error", err)
-						}
-						select {
-						case c.errors <- fmt.Errorf("failed to set write deadline: %w", err):
-						case <-c.done:
-						}
-						c.shutdown()
-						return
-					}
-				}
-
-				if err := c.socket.WriteMessage(websocket.TextMessage, data); err != nil {
-					if c.logger != nil {
-						c.logger.Error("write error", "error", err)
-					}
-					select {
-					case c.errors <- err:
-					case <-c.done:
-					}
-					c.shutdown()
-					return
-				}
-			}
-		}
-	}()
-
-}
-
 func (c *Connection) Send(data []byte) error {
-	c.mu.RLock()
-	defer c.mu.RUnlock()
+	c.writeMu.Lock()
+	defer c.writeMu.Unlock()
 
 	if c.closed {
-		return NewConnectionError("connection closed")
+		return ErrConnectionClosed
 	}
 
-	select {
-	case c.outgoing <- data:
-		return nil
-	case <-c.done:
-		return NewConnectionError("connection closing")
-	default:
-		return NewConnectionError("outgoing queue full")
+	if c.config.WriteTimeout > 0 {
+		if err := c.socket.SetWriteDeadline(time.Now().Add(c.config.WriteTimeout)); err != nil {
+			if c.logger != nil {
+				c.logger.Error("write deadline error", "error", err)
+			}
+			c.shutdown()
+			return fmt.Errorf("failed to set write deadline: %w", err)
+		}
 	}
+
+	if err := c.socket.WriteMessage(websocket.TextMessage, data); err != nil {
+		if c.logger != nil {
+			c.logger.Error("write error", "error", err)
+		}
+		return fmt.Errorf("%w: %w", ErrWriteFailed, err)
+	}
+
+	return nil
 }
 
 func (c *Connection) Incoming() <-chan []byte {
@@ -326,7 +291,6 @@ func (c *Connection) shutdown() {
 
 		c.wg.Wait()
 		close(c.incoming)
-		close(c.outgoing)
 		close(c.errors)
 	}()
 

transport/connection_goroutine_test.go

@@ -5,10 +5,7 @@ import (
 	"git.wisehodl.dev/jay/go-honeybee/honeybeetest"
 	"github.com/gorilla/websocket"
 	"github.com/stretchr/testify/assert"
-	"io"
-	"strings"
 	"testing"
-	"time"
 )
 
 func TestStartReader(t *testing.T) {
@@ -88,198 +85,3 @@ func TestStartReader(t *testing.T) {
 		}, honeybeetest.TestTimeout, honeybeetest.TestTick)
 	})
 }
-
-func TestStartWriter(t *testing.T) {
-	t.Run("data from outgoing triggers write", func(t *testing.T) {
-		conn, _, _, outgoingData := setupTestConnection(t, nil)
-		defer conn.Close()
-
-		testData := []byte("test message")
-		err := conn.Send(testData)
-		assert.NoError(t, err)
-
-		honeybeetest.ExpectWrite(t, outgoingData, websocket.TextMessage, testData)
-	})
-
-	t.Run("multiple messages processed sequentially", func(t *testing.T) {
-		conn, _, _, outgoingData := setupTestConnection(t, nil)
-		defer conn.Close()
-
-		messages := [][]byte{[]byte("first"), []byte("second"), []byte("third")}
-		for _, msg := range messages {
-			err := conn.Send(msg)
-			assert.NoError(t, err)
-		}
-
-		for _, expected := range messages {
-			honeybeetest.ExpectWrite(t, outgoingData, websocket.TextMessage, expected)
-		}
-	})
-
-	t.Run("write timeout disabled when zero", func(t *testing.T) {
-		if testing.Short() {
-			t.Skip("skipping test in short mode")
-		}
-
-		config := &ConnectionConfig{WriteTimeout: 0}
-
-		outgoingData := make(chan honeybeetest.MockOutgoingData, 10)
-		mockSocket := honeybeetest.NewMockSocket()
-
-		mockSocket.CloseFunc = func() error {
-			mockSocket.Once.Do(func() {
-				close(mockSocket.Closed)
-			})
-			return nil
-		}
-
-		deadlineCalled := make(chan struct{}, 1)
-		mockSocket.SetWriteDeadlineFunc = func(t time.Time) error {
-			deadlineCalled <- struct{}{}
-			return nil
-		}
-
-		mockSocket.WriteMessageFunc = func(msgType int, data []byte) error {
-			select {
-			case outgoingData <- honeybeetest.MockOutgoingData{
-				MsgType: msgType, Data: data}:
-			case <-mockSocket.Closed:
-				return io.EOF
-			}
-			return nil
-		}
-
-		conn, err := NewConnectionFromSocket(mockSocket, config, nil)
-		assert.NoError(t, err)
-		defer conn.Close()
-
-		err = conn.Send([]byte("test"))
-		assert.NoError(t, err)
-
-		assert.Never(t, func() bool {
-			select {
-			case <-deadlineCalled:
-				return true
-			default:
-				return false
-			}
-		}, honeybeetest.NegativeTestTimeout, honeybeetest.TestTick,
-			"SetWriteDeadline should not be called when timeout is zero")
-	})
-
-	t.Run("write timeout sets deadline when positive", func(t *testing.T) {
-		config := &ConnectionConfig{WriteTimeout: 30 * time.Millisecond}
-
-		outgoingData := make(chan honeybeetest.MockOutgoingData, 10)
-		mockSocket := honeybeetest.NewMockSocket()
-
-		mockSocket.CloseFunc = func() error {
-			mockSocket.Once.Do(func() {
-				close(mockSocket.Closed)
-			})
-			return nil
-		}
-
-		deadlineCalled := make(chan struct{}, 1)
-		mockSocket.SetWriteDeadlineFunc = func(t time.Time) error {
-			deadlineCalled <- struct{}{}
-			return nil
-		}
-
-		mockSocket.WriteMessageFunc = func(msgType int, data []byte) error {
-			select {
-			case outgoingData <- honeybeetest.MockOutgoingData{
-				MsgType: msgType, Data: data}:
-			case <-mockSocket.Closed:
-				return io.EOF
-			}
-			return nil
-		}
-
-		conn, err := NewConnectionFromSocket(mockSocket, config, nil)
-		assert.NoError(t, err)
-		defer conn.Close()
-
-		err = conn.Send([]byte("test"))
-		assert.NoError(t, err)
-
-		assert.Eventually(t, func() bool {
-			select {
-			case <-deadlineCalled:
-				return true
-			default:
-				return false
-			}
-		}, honeybeetest.TestTimeout, honeybeetest.TestTick,
-			"SetWriteDeadline should be called when timeout is positive")
-	})
-
-	t.Run("writer exits on deadline error", func(t *testing.T) {
-		config := &ConnectionConfig{WriteTimeout: 1 * time.Millisecond}
-
-		mockSocket := honeybeetest.NewMockSocket()
-
-		mockSocket.CloseFunc = func() error {
-			mockSocket.Once.Do(func() {
-				close(mockSocket.Closed)
-			})
-			return nil
-		}
-
-		mockSocket.SetWriteDeadlineFunc = func(t time.Time) error {
-			return fmt.Errorf("test error")
-		}
-
-		conn, err := NewConnectionFromSocket(mockSocket, config, nil)
-		assert.NoError(t, err)
-
-		err = conn.Send([]byte("test"))
-		assert.NoError(t, err)
-		defer conn.Close()
-
-		assert.Eventually(t, func() bool {
-			select {
-			case err := <-conn.Errors():
-				return err != nil &&
-					strings.Contains(err.Error(), "failed to set write deadline")
-			default:
-				return false
-			}
-		}, honeybeetest.TestTimeout, honeybeetest.TestTick)
-
-		assert.Eventually(t, func() bool {
-			return conn.State() == StateClosed
-		}, honeybeetest.TestTimeout, honeybeetest.TestTick)
-	})
-
-	t.Run("writer exits on socket write error", func(t *testing.T) {
-		mockSocket := honeybeetest.NewMockSocket()
-
-		writeErr := fmt.Errorf("write failed")
-		mockSocket.WriteMessageFunc = func(msgType int, data []byte) error {
-			return writeErr
-		}
-
-		conn, err := NewConnectionFromSocket(mockSocket, nil, nil)
-		assert.NoError(t, err)
-		defer conn.Close()
-
-		err = conn.Send([]byte("test"))
-		assert.NoError(t, err)
-
-		assert.Eventually(t, func() bool {
-			select {
-			case err := <-conn.Errors():
-				return err == writeErr
-			default:
-				return false
-			}
-		}, honeybeetest.TestTimeout, honeybeetest.TestTick)
-
-		assert.Eventually(t, func() bool {
-			return conn.State() == StateClosed
-		}, honeybeetest.TestTimeout, honeybeetest.TestTick)
-	})
-}
-
-// Helpers

transport/connection_send_test.go

@@ -2,110 +2,243 @@ package transport
 
 import (
 	"fmt"
+	"git.wisehodl.dev/jay/go-honeybee/honeybeetest"
+	"github.com/gorilla/websocket"
 	"github.com/stretchr/testify/assert"
+	"io"
 	"sync"
 	"testing"
+	"time"
 )
 
 func TestConnectionSend(t *testing.T) {
-	cases := []struct {
-		name        string
-		setup       func(*Connection)
-		data        []byte
-		wantErr     bool
-		wantErrText string
-	}{
-		{
-			name:  "send succeeds when open",
-			setup: func(c *Connection) {},
-			data:  []byte("test message"),
-		},
-		{
-			name: "send fails when closed",
-			setup: func(c *Connection) {
-				c.Close()
-			},
-			data:        []byte("test"),
-			wantErr:     true,
-			wantErrText: "connection closed",
-		},
-		{
-			name: "send fails when queue full",
-			setup: func(c *Connection) {
-				// Fill outgoing channel
-				for i := 0; i < 100; i++ {
-					c.outgoing <- []byte("filler")
-				}
-			},
-			data:        []byte("overflow"),
-			wantErr:     true,
-			wantErrText: "outgoing queue full",
-		},
-	}
+	t.Run("writes message to socket", func(t *testing.T) {
+		conn, _, _, outgoingData := setupTestConnection(t, nil)
+		defer conn.Close()
 
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			conn, err := NewConnection("ws://test", nil, nil)
+		testData := []byte("test message")
+		err := conn.Send(testData)
+		assert.NoError(t, err)
+
+		honeybeetest.ExpectWrite(t, outgoingData, websocket.TextMessage, testData)
+	})
+
+	t.Run("writes multiple message to socket", func(t *testing.T) {
+		conn, _, _, outgoingData := setupTestConnection(t, nil)
+		defer conn.Close()
+
+		messages := [][]byte{[]byte("first"), []byte("second"), []byte("third")}
+		for _, msg := range messages {
+			err := conn.Send(msg)
 			assert.NoError(t, err)
-
-			tc.setup(conn)
-
-			err = conn.Send(tc.data)
-
-			if tc.wantErr {
-				assert.Error(t, err)
-				if tc.wantErrText != "" {
-					assert.ErrorContains(t, err, tc.wantErrText)
-				}
-				return
-			}
-
-			assert.NoError(t, err)
-
-			select {
-			case sent := <-conn.outgoing:
-				assert.Equal(t, tc.data, sent)
-			default:
-				t.Fatal("data not sent to outgoing channel")
-			}
-		})
-	}
-}
-
-// Run with `go test -race` to ensure no race conditions occur
-func TestConnectionSendConcurrent(t *testing.T) {
-	conn, err := NewConnection("ws://test", nil, nil)
-	assert.NoError(t, err)
-
-	// continuously consume outgoing channel in background
-	done := make(chan struct{})
-	go func() {
-		for {
-			select {
-			case <-conn.outgoing:
-			case <-done:
-				return
-			}
 		}
-	}()
-	defer close(done)
 
-	// Send from multiple goroutines concurrently
-	const goroutines = 5
-	const messagesPerGoroutine = 10
-	var wg sync.WaitGroup
+		for _, expected := range messages {
+			honeybeetest.ExpectWrite(t, outgoingData, websocket.TextMessage, expected)
+		}
+	})
 
-	for i := 0; i < goroutines; i++ {
-		wg.Add(1)
-		go func(id int) {
-			defer wg.Done()
-			for j := 0; j < messagesPerGoroutine; j++ {
-				data := []byte(fmt.Sprintf("msg-%d-%d", id, j))
-				err := conn.Send(data)
-				assert.NoError(t, err)
+	t.Run("concurrent sends write messages to socket", func(t *testing.T) {
+		conn, _, _, outgoingData := setupTestConnection(t, nil)
+		defer conn.Close()
+
+		mu := sync.Mutex{}
+		messages := []string{}
+		done := make(chan struct{})
+
+		go func() {
+			for {
+				select {
+				case msg := <-outgoingData:
+					fmt.Printf("got message %s\n", string(msg.Data))
+					mu.Lock()
+					messages = append(messages, string(msg.Data))
+					mu.Unlock()
+				case <-done:
+					return
+				}
 			}
-		}(i)
-	}
+		}()
 
-	wg.Wait()
+		defer close(done)
+
+		var wg sync.WaitGroup
+		for i := 0; i < 5; i++ {
+			wg.Add(1)
+			go func(id int) {
+				defer wg.Done()
+				for j := 0; j < 10; j++ {
+					data := []byte(fmt.Sprintf("msg-%d-%d", id, j))
+					fmt.Printf("sending message %s\n", string(data))
+					for {
+						// send and retry until success
+						err := conn.Send(data)
+						if err != nil {
+							continue
+						} else {
+							break
+						}
+					}
+				}
+			}(i)
+		}
+
+		wg.Wait()
+
+		assert.Eventually(t, func() bool {
+			mu.Lock()
+			defer mu.Unlock()
+			return len(messages) == 50
+		}, honeybeetest.TestTimeout, honeybeetest.TestTick,
+			"should have received 50 messages")
+
+	})
+
+	t.Run("send fails when connection is closed", func(t *testing.T) {
+		conn, _, _, _ := setupTestConnection(t, nil)
+		conn.Close()
+
+		testData := []byte("test message")
+		err := conn.Send(testData)
+		assert.ErrorIs(t, err, ErrConnectionClosed)
+	})
+
+	t.Run("write timeout disabled when zero", func(t *testing.T) {
+		config := &ConnectionConfig{WriteTimeout: 0}
+
+		outgoingData := make(chan honeybeetest.MockOutgoingData, 10)
+		mockSocket := honeybeetest.NewMockSocket()
+
+		mockSocket.CloseFunc = func() error {
+			mockSocket.Once.Do(func() {
+				close(mockSocket.Closed)
+			})
+			return nil
+		}
+
+		deadlineCalled := make(chan struct{}, 1)
+		mockSocket.SetWriteDeadlineFunc = func(t time.Time) error {
+			deadlineCalled <- struct{}{}
+			return nil
+		}
+
+		mockSocket.WriteMessageFunc = func(msgType int, data []byte) error {
+			select {
+			case outgoingData <- honeybeetest.MockOutgoingData{
+				MsgType: msgType, Data: data}:
+			case <-mockSocket.Closed:
+				return io.EOF
+			}
+			return nil
+		}
+
+		conn, err := NewConnectionFromSocket(mockSocket, config, nil)
+		assert.NoError(t, err)
+		defer conn.Close()
+
+		err = conn.Send([]byte("test"))
+		assert.NoError(t, err)
+
+		assert.Never(t, func() bool {
+			select {
+			case <-deadlineCalled:
+				return true
+			default:
+				return false
+			}
+		}, honeybeetest.NegativeTestTimeout, honeybeetest.TestTick,
+			"SetWriteDeadline should not be called when timeout is zero")
+	})
+
+	t.Run("write timeout sets deadline when positive", func(t *testing.T) {
+		config := &ConnectionConfig{WriteTimeout: 30 * time.Millisecond}
+
+		outgoingData := make(chan honeybeetest.MockOutgoingData, 10)
+		mockSocket := honeybeetest.NewMockSocket()
+
+		mockSocket.CloseFunc = func() error {
+			mockSocket.Once.Do(func() {
+				close(mockSocket.Closed)
+			})
+			return nil
+		}
+
+		deadlineCalled := make(chan struct{}, 1)
+		mockSocket.SetWriteDeadlineFunc = func(t time.Time) error {
+			deadlineCalled <- struct{}{}
+			return nil
+		}
+
+		mockSocket.WriteMessageFunc = func(msgType int, data []byte) error {
+			select {
+			case outgoingData <- honeybeetest.MockOutgoingData{
+				MsgType: msgType, Data: data}:
+			case <-mockSocket.Closed:
+				return io.EOF
+			}
+			return nil
+		}
+
+		conn, err := NewConnectionFromSocket(mockSocket, config, nil)
+		assert.NoError(t, err)
+		defer conn.Close()
+
+		err = conn.Send([]byte("test"))
+		assert.NoError(t, err)
+
+		assert.Eventually(t, func() bool {
+			select {
+			case <-deadlineCalled:
+				return true
+			default:
+				return false
+			}
+		}, honeybeetest.TestTimeout, honeybeetest.TestTick,
+			"SetWriteDeadline should be called when timeout is positive")
+	})
+
+	t.Run("send fails on deadline error", func(t *testing.T) {
+		config := &ConnectionConfig{WriteTimeout: 1 * time.Millisecond}
+
+		mockSocket := honeybeetest.NewMockSocket()
+
+		mockSocket.CloseFunc = func() error {
+			mockSocket.Once.Do(func() {
+				close(mockSocket.Closed)
+			})
+			return nil
+		}
+
+		mockSocket.SetWriteDeadlineFunc = func(t time.Time) error {
+			return fmt.Errorf("test error")
+		}
+
+		conn, err := NewConnectionFromSocket(mockSocket, config, nil)
+		assert.NoError(t, err)
+		defer conn.Close()
+
+		err = conn.Send([]byte("test"))
+		assert.ErrorContains(t, err, "failed to set write deadline: test error")
+
+		assert.Eventually(t, func() bool {
+			return conn.State() == StateClosed
+		}, honeybeetest.TestTimeout, honeybeetest.TestTick)
+	})
+
+	t.Run("send fails on socket write error", func(t *testing.T) {
+		mockSocket := honeybeetest.NewMockSocket()
+
+		writeErr := fmt.Errorf("test error")
+		mockSocket.WriteMessageFunc = func(msgType int, data []byte) error {
+			return writeErr
+		}
+
+		conn, err := NewConnectionFromSocket(mockSocket, nil, nil)
+		assert.NoError(t, err)
+		defer conn.Close()
+
+		err = conn.Send([]byte("test"))
+		assert.ErrorIs(t, err, ErrWriteFailed)
+		assert.ErrorContains(t, err, "test error")
+	})
 }

transport/connection_test.go

@@ -112,7 +112,6 @@ func TestNewConnection(t *testing.T) {
 			assert.Nil(t, conn.socket)
 			assert.NotNil(t, conn.config)
 			assert.NotNil(t, conn.incoming)
-			assert.NotNil(t, conn.outgoing)
 			assert.NotNil(t, conn.errors)
 			assert.NotNil(t, conn.done)
 			assert.Equal(t, StateDisconnected, conn.state)
@@ -213,7 +212,6 @@ func TestNewConnectionFromSocket(t *testing.T) {
 			assert.Equal(t, tc.socket, conn.socket)
 			assert.NotNil(t, conn.config)
 			assert.NotNil(t, conn.incoming)
-			assert.NotNil(t, conn.outgoing)
 			assert.NotNil(t, conn.errors)
 			assert.NotNil(t, conn.done)
 			assert.Equal(t, StateConnected, conn.state)

transport/errors.go

@@ -13,6 +13,10 @@ var (
 	InvalidRetryInitialDelay = errors.New("initial delay must be positive")
 	InvalidRetryMaxDelay     = errors.New("max delay must be positive")
 	InvalidRetryJitterFactor = errors.New("jitter factor must be between 0.0 and 1.0")
+
+	// Connection Errors
+	ErrConnectionClosed = errors.New("connection closed")
+	ErrWriteFailed      = errors.New("write failed")
 )
 
 func NewConfigError(text string) error {

transport/logging_test.go

@@ -413,7 +413,7 @@ func TestWriterLogging(t *testing.T) {
 		assert.NoError(t, err)
 
 		err = conn.Send([]byte("test"))
-		assert.NoError(t, err)
+		assert.ErrorContains(t, err, "failed to set write deadline: deadline error")
 
 		assert.Eventually(t, func() bool {
 			return findLogRecord(
@@ -443,7 +443,7 @@ func TestWriterLogging(t *testing.T) {
 		assert.NoError(t, err)
 
 		err = conn.Send([]byte("test"))
-		assert.NoError(t, err)
+		assert.ErrorContains(t, err, "write error")
 
 		assert.Eventually(t, func() bool {
 			return findLogRecord(

transport/socket_test.go

@@ -199,9 +199,9 @@ func TestAcquireSocketContextCancellation(t *testing.T) {
 				done <- err
 			}()
 
-			// wait for first dial to complete, then cancel during sleep
+			// wait for first two dials to complete, then cancel during sleep
 			assert.Eventually(t, func() bool {
-				return dialCount.Load() >= 1
+				return dialCount.Load() > 1
 			}, honeybeetest.TestTimeout, honeybeetest.TestTick)
 			cancel()