remove parentCtx from the httpserver Runnable (#48)

* remove parentCtx from the httpserver Runnable

* use t.Context instead of context.Background for tests

Author: robbyt

examples/http/main.go

@@ -81,27 +81,22 @@ func buildRoutes(logHandler slog.Handler) ([]httpserver.Route, error) {
 }
 
 // RunServer initializes and runs the HTTP server with supervisor
-// Returns the supervisor and a cleanup function
 func RunServer(
 	ctx context.Context,
 	logHandler slog.Handler,
 	routes []httpserver.Route,
-) (*supervisor.PIDZero, func(), error) {
+) (*supervisor.PIDZero, error) {
 	// Create a config callback function that will be used by the runner
 	configCallback := func() (*httpserver.Config, error) {
 		return httpserver.NewConfig(ListenOn, routes, httpserver.WithDrainTimeout(DrainTimeout))
 	}
 
-	// Create the HTTP server runner with a custom context
-	customCtx, customCancel := context.WithCancel(ctx)
-
+	// Create the HTTP server runner
 	runner, err := httpserver.NewRunner(
-		httpserver.WithContext(customCtx),
 		httpserver.WithConfigCallback(configCallback),
 		httpserver.WithLogHandler(logHandler))
 	if err != nil {
-		customCancel()
-		return nil, nil, fmt.Errorf("failed to create HTTP server runner: %w", err)
+		return nil, fmt.Errorf("failed to create HTTP server runner: %w", err)
 	}
 
 	// Create a PIDZero supervisor and add the runner
@@ -110,11 +105,10 @@ func RunServer(
 		supervisor.WithLogHandler(logHandler),
 		supervisor.WithRunnables(runner))
 	if err != nil {
-		customCancel()
-		return nil, nil, fmt.Errorf("failed to create supervisor: %w", err)
+		return nil, fmt.Errorf("failed to create supervisor: %w", err)
 	}
 
-	return sv, customCancel, nil
+	return sv, nil
 }
 
 func main() {
@@ -135,12 +129,11 @@ func main() {
 		os.Exit(1)
 	}
 
-	sv, cancel, err := RunServer(ctx, handler, routes)
+	sv, err := RunServer(ctx, handler, routes)
 	if err != nil {
 		slog.Error("Failed to setup server", "error", err)
 		os.Exit(1)
 	}
-	defer cancel()
 
 	// Start the supervisor - this will block until shutdown
 	slog.Info("Starting supervisor with HTTP server on " + ListenOn)

examples/http/main_test.go

@@ -30,10 +30,9 @@ func TestRunServer(t *testing.T) {
 	require.NoError(t, err, "Failed to build routes")
 	require.NotEmpty(t, routes, "Routes should not be empty")
 
-	sv, cleanup, err := RunServer(ctx, logHandler, routes)
+	sv, err := RunServer(ctx, logHandler, routes)
 	require.NoError(t, err, "RunServer should not return an error")
 	require.NotNil(t, sv, "Supervisor should not be nil")
-	require.NotNil(t, cleanup, "Cleanup function should not be nil")
 
 	// Start the server in a goroutine to avoid blocking the test
 	errCh := make(chan error, 1)
@@ -54,8 +53,8 @@ func TestRunServer(t *testing.T) {
 	assert.Equal(t, http.StatusOK, resp.StatusCode)
 	assert.Equal(t, "Status: OK\n", string(body))
 
-	// Clean up
-	cleanup()
+	// Stop the supervisor
+	sv.Shutdown()
 
 	// Check that Run() didn't return an error
 	select {
@@ -88,7 +87,6 @@ func TestRunServerInvalidPort(t *testing.T) {
 
 	// Create HTTP server runner with invalid port
 	runner, err := httpserver.NewRunner(
-		httpserver.WithContext(ctx),
 		httpserver.WithConfigCallback(configCallback),
 		httpserver.WithLogHandler(logHandler.WithGroup("httpserver")),
 	)

runnables/httpcluster/runner.go

@@ -63,7 +63,6 @@ func defaultRunnerFactory(
 	return httpserver.NewRunner(
 		httpserver.WithName(id),
 		httpserver.WithConfig(cfg),
-		httpserver.WithContext(ctx),
 		httpserver.WithLogHandler(handler),
 	)
 }

runnables/httpserver/config_test.go

@@ -298,12 +298,10 @@ func TestContextPropagation(t *testing.T) {
 		return NewConfig(listenPort, hConfig, WithDrainTimeout(2*time.Second))
 	}
 
-	// Create a new context that we'll cancel to trigger shutdown
-	ctx, cancel := context.WithCancel(context.Background())
-	defer cancel()
+	// Context for the server Run method
+	ctx := t.Context()
 
 	server, err := NewRunner(
-		WithContext(ctx),
 		WithConfigCallback(cfgCallback),
 	)
 	require.NoError(t, err)
@@ -313,7 +311,7 @@ func TestContextPropagation(t *testing.T) {
 
 	// Start the server in a goroutine
 	go func() {
-		err := server.Run(context.Background())
+		err := server.Run(ctx)
 		runComplete <- err
 	}()
 
@@ -346,7 +344,7 @@ func TestContextPropagation(t *testing.T) {
 	}
 
 	// Initiate server shutdown
-	cancel() // This should cancel the context passed to the server
+	server.Stop() // This should cancel the server's context
 
 	// Verify that the handler's context was canceled
 	select {

runnables/httpserver/helpers_test.go

@@ -1,7 +1,6 @@
 package httpserver
 
 import (
-	"context"
 	"fmt"
 	"net"
 	"net/http"
@@ -44,7 +43,7 @@ func createTestServer(
 		return NewConfig(listenPort, hConfig, WithDrainTimeout(drainTimeout))
 	}
 
-	server, err := NewRunner(WithContext(context.Background()), WithConfigCallback(cfgCallback))
+	server, err := NewRunner(WithConfigCallback(cfgCallback))
 	require.NoError(t, err)
 	require.NotNil(t, server)
 

runnables/httpserver/options.go

@@ -1,7 +1,6 @@
 package httpserver
 
 import (
-	"context"
 	"log/slog"
 )
 
@@ -21,16 +20,6 @@ func WithLogHandler(handler slog.Handler) Option {
 	}
 }
 
-// WithContext sets a custom context for the Runner instance.
-// This allows for more granular control over cancellation and timeouts.
-func WithContext(ctx context.Context) Option {
-	return func(r *Runner) {
-		if ctx != nil {
-			r.ctx, r.cancel = context.WithCancel(ctx)
-		}
-	}
-}
-
 // WithConfigCallback sets the function that will be called to load or reload configuration.
 // Either this option or WithConfig initializes the Runner instance by providing the
 // configuration for the HTTP server managed by the Runner.

runnables/httpserver/options_test.go

@@ -1,7 +1,6 @@
 package httpserver
 
 import (
-	"context"
 	"log/slog"
 	"net/http"
 	"strings"
@@ -13,47 +12,6 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
-// Define a custom type for context keys to avoid string collision
-type contextKey string
-
-// TestWithContext verifies the WithContext option works correctly
-func TestWithContext(t *testing.T) {
-	t.Parallel()
-	// Create a custom context with a value using the type-safe key
-	testKey := contextKey("test-key")
-	customCtx := context.WithValue(context.Background(), testKey, "test-value")
-	// Create a server with the custom context
-	handler := func(w http.ResponseWriter, r *http.Request) {}
-	route, err := NewRoute("v1", "/", handler)
-	require.NoError(t, err)
-	hConfig := Routes{*route}
-	cfgCallback := func() (*Config, error) {
-		return NewConfig(":0", hConfig, WithDrainTimeout(1*time.Second))
-	}
-	server, err := NewRunner(WithContext(context.Background()),
-		WithConfigCallback(cfgCallback),
-		WithContext(customCtx))
-	require.NoError(t, err)
-	// Verify the custom context was applied
-	actualValue := server.ctx.Value(testKey)
-	assert.Equal(t, "test-value", actualValue, "Context value should be preserved")
-	// Verify cancellation works through server.Stop()
-	done := make(chan struct{})
-	go func() {
-		<-server.ctx.Done()
-		close(done)
-	}()
-	// Call Stop to cancel the internal context
-	server.Stop()
-	// Wait for the server context to be canceled or timeout
-	select {
-	case <-done:
-		// Success, context was canceled
-	case <-time.After(1 * time.Second):
-		t.Fatal("Context cancellation not propagated")
-	}
-}
-
 func TestWithLogHandler(t *testing.T) {
 	t.Parallel()
 	// Create a custom logger with a buffer for testing output
@@ -69,7 +27,6 @@ func TestWithLogHandler(t *testing.T) {
 	}
 	// Create a server with the custom logger
 	server, err := NewRunner(
-		WithContext(context.Background()),
 		WithConfigCallback(cfgCallback),
 		WithLogHandler(customHandler),
 	)
@@ -95,7 +52,6 @@ func TestWithConfig(t *testing.T) {
 	require.NoError(t, err)
 	// Create a server with the static config
 	server, err := NewRunner(
-		WithContext(context.Background()),
 		WithConfig(staticConfig),
 	)
 	require.NoError(t, err)
@@ -143,7 +99,6 @@ func TestWithServerCreator(t *testing.T) {
 	}
 	// Create a server with the config that has a custom server creator
 	server, err := NewRunner(
-		WithContext(context.Background()),
 		WithConfigCallback(cfgCallback),
 	)
 	require.NoError(t, err)

runnables/httpserver/reload_mocked_test.go

@@ -1,7 +1,6 @@
 package httpserver
 
 import (
-	"context"
 	"errors"
 	"net/http"
 	"testing"
@@ -203,7 +202,6 @@ func TestReloadConfig_WithFullRunner(t *testing.T) {
 
 		// Create the Runner with the config callback
 		runner, err := NewRunner(
-			WithContext(context.Background()),
 			WithConfigCallback(configCallback),
 		)
 		require.NoError(t, err)