feat: config hot-reload - fix merge conflicts

Author: sammcj

proxy/helpers_test.go

@@ -63,7 +63,7 @@ func getTestSimpleResponderConfigPort(expectedMessage string, port int) ModelCon
 
 	// Create a process configuration
 	return ModelConfig{
-		Cmd:           fmt.Sprintf("%s --port %d --silent --respond %s", binaryPath, port, expectedMessage), // Re-added --silent
+		Cmd:           fmt.Sprintf("%s --port %d --respond %s", binaryPath, port, expectedMessage),
 		Proxy:         fmt.Sprintf("http://127.0.0.1:%d", port),
 		CheckEndpoint: "/health",
 	}

proxy/logMonitor_test.go

@@ -21,40 +21,27 @@ func TestLogMonitor(t *testing.T) {
 	client2Messages := make([]byte, 0)
 
 	var wg sync.WaitGroup
-	wg.Add(2) // One for each client (goroutine)
+	wg.Add(1)
 
-	// Write messages first
-	if _, err := logMonitor.Write([]byte("1")); err != nil {
-		t.Fatalf("Failed to write log message: %v", err)
-	}
-	if _, err := logMonitor.Write([]byte("2")); err != nil {
-		t.Fatalf("Failed to write log message: %v", err)
-	}
-	if _, err := logMonitor.Write([]byte("3")); err != nil {
-		t.Fatalf("Failed to write log message: %v", err)
-	}
-
-	// Start goroutines to collect messages
 	go func() {
 		defer wg.Done()
-		messageCount := 0
-		for messageCount < 3 {
-			data := <-client1
-			client1Messages = append(client1Messages, data...)
-			messageCount++
+		for {
+			select {
+			case data := <-client1:
+				client1Messages = append(client1Messages, data...)
+			case data := <-client2:
+				client2Messages = append(client2Messages, data...)
+			default:
+				return
+			}
 		}
 	}()
 
-	go func() {
-		defer wg.Done()
-		messageCount := 0
-		for messageCount < 3 {
-			data := <-client2
-			client2Messages = append(client2Messages, data...)
-			messageCount++
-		}
-	}()
-	// Wait for both goroutines to finish
+	logMonitor.Write([]byte("1"))
+	logMonitor.Write([]byte("2"))
+	logMonitor.Write([]byte("3"))
+
+	// Wait for the goroutine to finish
 	wg.Wait()
 
 	// Check the buffer

proxy/process.go

@@ -119,10 +119,8 @@ func isValidTransition(from, to ProcessState) bool {
 		return to == StateStopping
 	case StateStopping:
 		return to == StateStopped || to == StateShutdown
-	case StateFailed:
-		return to == StateStopped // Allow resetting a failed process
-	case StateShutdown:
-		return false // No transitions from shutdown
+	case StateFailed, StateShutdown:
+		return false // No transitions allowed from these states
 	}
 	return false
 }
@@ -187,7 +185,7 @@ func (p *Process) start() error {
 		return fmt.Errorf("start() failed: %v", err)
 	}
 
-	// Capture the exit error for later signaling
+	// Capture the exit error for later signalling
 	go func() {
 		exitErr := p.cmd.Wait()
 		p.proxyLogger.Debugf("<%s> cmd.Wait() returned error: %v", p.ID, exitErr)
@@ -414,20 +412,7 @@ func (p *Process) ProxyRequest(w http.ResponseWriter, r *http.Request) {
 
 	// prevent new requests from being made while stopping or irrecoverable
 	currentState := p.CurrentState()
-
-	if currentState == StateFailed {
-		p.proxyLogger.Infof("<%s> Attempting to recover from failed state.", p.ID)
-		// Attempt to reset the state to stopped so it can be restarted
-		if _, err := p.swapState(StateFailed, StateStopped); err != nil {
-			p.proxyLogger.Errorf("<%s> Failed to reset state from Failed to Stopped: %v. Current state: %s", p.ID, err, p.CurrentState())
-			http.Error(w, fmt.Sprintf("Process failed and could not be reset, state is %s", p.CurrentState()), http.StatusServiceUnavailable)
-			return
-		}
-		p.proxyLogger.Infof("<%s> Process state reset to Stopped from Failed.", p.ID)
-		currentState = p.CurrentState() // Re-fetch current state, should now be StateStopped
-	}
-
-	if currentState == StateShutdown || currentState == StateStopping {
+	if currentState == StateFailed || currentState == StateShutdown || currentState == StateStopping {
 		http.Error(w, fmt.Sprintf("Process can not ProxyRequest, state is %s", currentState), http.StatusServiceUnavailable)
 		return
 	}

proxy/process_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"strings"
 	"sync"
 	"testing"
 	"time"
@@ -18,10 +19,10 @@ var (
 
 func init() {
 	// flip to help with debugging tests
-	if false { // Reverted to false
+	if false {
 		debugLogger.SetLogLevel(LevelDebug)
 	} else {
-		debugLogger.SetLogLevel(LevelError) // This will now be active
+		debugLogger.SetLogLevel(LevelError)
 	}
 }
 
@@ -38,13 +39,14 @@ func TestProcess_AutomaticallyStartsUpstream(t *testing.T) {
 	req := httptest.NewRequest("GET", "/test", nil)
 	w := httptest.NewRecorder()
 
-	// process is automatically started
-	assert.Equal(t, StateStopped, process.CurrentState())
 	process.ProxyRequest(w, req)
-	assert.Equal(t, StateReady, process.CurrentState())
 
-	assert.Equal(t, http.StatusOK, w.Code, "Expected status code %d, got %d", http.StatusOK, w.Code)
-	assert.Contains(t, w.Body.String(), expectedMessage)
+	if w.Code != http.StatusOK {
+		t.Errorf("Expected status code %d, got %d", http.StatusOK, w.Code)
+	}
+	if !strings.Contains(w.Body.String(), expectedMessage) {
+		t.Errorf("Expected body to contain '%s', got '%s'", expectedMessage, w.Body.String())
+	}
 
 	// Stop the process
 	process.Stop()
@@ -59,7 +61,6 @@ func TestProcess_AutomaticallyStartsUpstream(t *testing.T) {
 	req = httptest.NewRequest("GET", "/", nil)
 	w = httptest.NewRecorder()
 
-	// Proxy the request
 	process.ProxyRequest(w, req)
 
 	// should have automatically started the process again
@@ -109,16 +110,13 @@ func TestProcess_BrokenModelConfig(t *testing.T) {
 	req := httptest.NewRequest("GET", "/", nil)
 	w := httptest.NewRecorder()
 	process.ProxyRequest(w, req)
-	assert.Equal(t, http.StatusBadGateway, w.Code) // First attempt fails to start
-	assert.Contains(t, w.Body.String(), "unable to start process: start() failed: exec: \"nonexistent-command\": executable file not found in $PATH")
-	assert.Equal(t, StateFailed, process.CurrentState()) // Should be in failed state after first attempt
+	assert.Equal(t, http.StatusBadGateway, w.Code)
+	assert.Contains(t, w.Body.String(), "unable to start process")
 
-	// Second request should also attempt to start and fail similarly due to recovery logic
 	w = httptest.NewRecorder()
 	process.ProxyRequest(w, req)
-	assert.Equal(t, http.StatusBadGateway, w.Code) // Second attempt also fails to start
-	assert.Contains(t, w.Body.String(), "unable to start process: start() failed: exec: \"nonexistent-command\": executable file not found in $PATH")
-	assert.Equal(t, StateFailed, process.CurrentState()) // Should end up in failed state again
+	assert.Equal(t, http.StatusServiceUnavailable, w.Code)
+	assert.Contains(t, w.Body.String(), "Process can not ProxyRequest, state is failed")
 }
 
 func TestProcess_UnloadAfterTTL(t *testing.T) {
@@ -269,7 +267,7 @@ func TestProcess_SwapState(t *testing.T) {
 		{"Ready to Starting", StateReady, StateReady, StateStarting, ErrInvalidStateTransition, StateReady},
 		{"Ready to Failed", StateReady, StateReady, StateFailed, ErrInvalidStateTransition, StateReady},
 		{"Stopping to Ready", StateStopping, StateStopping, StateReady, ErrInvalidStateTransition, StateStopping},
-		{"Failed to Stopped", StateFailed, StateFailed, StateStopped, nil, StateStopped},
+		{"Failed to Stopped", StateFailed, StateFailed, StateStopped, ErrInvalidStateTransition, StateFailed},
 		{"Failed to Starting", StateFailed, StateFailed, StateStarting, ErrInvalidStateTransition, StateFailed},
 		{"Shutdown to Stopped", StateShutdown, StateShutdown, StateStopped, ErrInvalidStateTransition, StateShutdown},
 		{"Shutdown to Starting", StateShutdown, StateShutdown, StateStarting, ErrInvalidStateTransition, StateShutdown},