feat: add lifecycle management for network ownership in engine options and improve test coverage

Author: ziflex

engine.go

@@ -34,8 +34,7 @@ func New(setters ...Option) (*Engine, error) {
 		return nil, err
 	}
 
-	ownsNetwork := opts.network == nil
-
+	ownsNetwork := opts.hostNetwork == false
 	boot, err := newBootstrap(opts)
 	if err != nil {
 		return nil, fmt.Errorf("bootstrap: %w", err)
@@ -53,7 +52,6 @@ func New(setters ...Option) (*Engine, error) {
 	}
 
 	hooks := boot.hooks.clone()
-
 	// Run init hooks after bootstrap is finalized and before returning the engine.
 	if err := hooks.engine.runInitHooks(); err != nil {
 		initErr := fmt.Errorf("init hooks: %w", err)
@@ -81,7 +79,6 @@ func (e *Engine) Compile(ctx context.Context, src *source.Source) (*Plan, error)
 	}
 
 	prog, err := e.compiler.Compile(src)
-
 	// After-compile hooks always run and receive the compilation error (if any).
 	if hookErr := e.hooks.plan.runAfterCompileHooks(ctx, err); hookErr != nil {
 		return nil, errors.Join(err, fmt.Errorf("after compile hooks: %w", hookErr))

engine_lifecycle_test.go

@@ -179,6 +179,88 @@ func TestEngineCloseDoesNotCleanInjectedNetwork(t *testing.T) {
 	}
 }
 
+func TestEngineNetworkOwnershipFollowsLastOption(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		name        string
+		managedLast bool
+	}{
+		{name: "network options last", managedLast: true},
+		{name: "injected network last", managedLast: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+
+			managedClient := &recordingHTTPClient{}
+			injectedClient := &recordingHTTPClient{}
+			injectedNetwork := mustNewTestNetwork(t, ferretnet.WithHTTPClient(injectedClient))
+
+			var setters []Option
+			if tt.managedLast {
+				setters = []Option{
+					WithNetwork(injectedNetwork),
+					WithNetworkOptions(ferretnet.WithHTTPClient(managedClient)),
+				}
+			} else {
+				setters = []Option{
+					WithNetworkOptions(ferretnet.WithHTTPClient(managedClient)),
+					WithNetwork(injectedNetwork),
+				}
+			}
+
+			eng := mustNewEngine(t, setters...)
+			if tt.managedLast {
+				if got := eng.host.network.HTTP(); got != managedClient {
+					t.Fatalf("expected network options client, got %T", got)
+				}
+			} else if eng.host.network != injectedNetwork {
+				t.Fatalf("expected injected network, got %T", eng.host.network)
+			}
+
+			if err := eng.Close(); err != nil {
+				t.Fatalf("close engine: %v", err)
+			}
+
+			wantManagedCloses := 0
+			if tt.managedLast {
+				wantManagedCloses = 1
+			}
+
+			if got := managedClient.idleCloseCount(); got != wantManagedCloses {
+				t.Fatalf("expected %d managed network cleanup calls, got %d", wantManagedCloses, got)
+			}
+
+			if got := injectedClient.idleCloseCount(); got != 0 {
+				t.Fatalf("expected injected network to remain caller-owned, got %d cleanup calls", got)
+			}
+		})
+	}
+}
+
+func TestNewCleansNetworkCreatedFromOptionsOnInitFailure(t *testing.T) {
+	t.Parallel()
+
+	initErr := errors.New("init failed")
+	client := &recordingHTTPClient{}
+
+	_, err := New(
+		WithNetworkOptions(ferretnet.WithHTTPClient(client)),
+		WithEngineInitHook(func() error {
+			return initErr
+		}),
+	)
+	if !errors.Is(err, initErr) {
+		t.Fatalf("expected init error, got %v", err)
+	}
+
+	if got := client.idleCloseCount(); got != 1 {
+		t.Fatalf("expected construction-failure cleanup, got %d calls", got)
+	}
+}
+
 func TestNewCleansOwnedNetworkOnRegistrationFailure(t *testing.T) {
 	t.Parallel()
 

engine_options.go

@@ -28,6 +28,7 @@ type (
 		stdlib            stdlib.Set
 		logger            []logging.Option
 		network           ferretnet.Network
+		hostNetwork       bool
 		compiler          []compiler.Option
 		modules           []module.Module
 		maxActiveSessions int
@@ -597,13 +598,37 @@ func WithFSReadOnly() Option {
 }
 
 // WithNetwork sets the engine network service used by derived executions.
+// If a network is provided, the engine will use it directly and will not manage its lifecycle.
+// The host application is responsible for closing the network when it is no longer needed.
 func WithNetwork(network ferretnet.Network) Option {
 	return func(opts *options) error {
 		if network == nil {
 			return fmt.Errorf("network cannot be nil")
 		}
 
 		opts.network = network
+		opts.hostNetwork = true
+
+		return nil
+	}
+}
+
+// WithNetworkOptions creates an Option that constructs a new network service using the provided Ferret network options.
+// If no options are provided, the engine will use the default network service.
+func WithNetworkOptions(setters ...ferretnet.Option) Option {
+	return func(opts *options) error {
+		if len(setters) == 0 {
+			return nil
+		}
+
+		net, err := ferretnet.New(setters...)
+
+		if err != nil {
+			return fmt.Errorf("create network: %w", err)
+		}
+
+		opts.network = net
+		opts.hostNetwork = false
 
 		return nil
 	}

engine_options_test.go

@@ -1,10 +1,13 @@
 package ferret
 
 import (
+	"errors"
 	"strings"
 	"testing"
 
 	"github.com/MontFerret/ferret/v2/pkg/compiler"
+	ferretnet "github.com/MontFerret/ferret/v2/pkg/net"
+	ferrethttp "github.com/MontFerret/ferret/v2/pkg/net/http"
 	"github.com/MontFerret/ferret/v2/pkg/runtime"
 	"github.com/MontFerret/ferret/v2/pkg/stdlib"
 )
@@ -172,6 +175,40 @@ func TestNewOptionsRejectsBlankFSRoot(t *testing.T) {
 	}
 }
 
+func TestWithNetworkOptionsWithoutSettersIsNoOp(t *testing.T) {
+	t.Parallel()
+
+	network := mustNewTestNetwork(t)
+	opts := mustNewOptionsForTest(t, WithNetwork(network), WithNetworkOptions())
+
+	if opts.network != network {
+		t.Fatalf("expected injected network to remain configured, got %T", opts.network)
+	}
+
+	if !opts.hostNetwork {
+		t.Fatal("expected injected network to remain caller-owned")
+	}
+}
+
+func TestWithNetworkOptionsReturnsNetworkConstructionError(t *testing.T) {
+	t.Parallel()
+
+	_, err := newOptions([]Option{WithNetworkOptions(
+		ferretnet.WithHTTPPolicies(ferrethttp.WithMaxResponseSize(-1)),
+	)})
+	if err == nil {
+		t.Fatal("expected invalid network options to fail")
+	}
+
+	if !errors.Is(err, ferrethttp.ErrInvalidPolicyConfiguration) {
+		t.Fatalf("expected invalid policy configuration error, got: %v", err)
+	}
+
+	if !strings.Contains(err.Error(), "create network: http client:") {
+		t.Fatalf("expected network construction context, got: %v", err)
+	}
+}
+
 func TestWithStdlibSafeRegistersSelectedGroups(t *testing.T) {
 	t.Parallel()