fix(node): keep node running when a service exits without error

node/node.go

@@ -736,15 +736,36 @@ func (n *Node) StartService(
 	wg *conc.WaitGroup, ctx context.Context, cancel context.CancelFunc, s service.Service,
 ) {
 	wg.Go(func() {
-		// Immediately acknowledge panicing services by shutting down the node
-		// Without the deffered cancel(), we would have to wait for user to hit Ctrl+C
-		defer cancel()
+		// A panicking service is critical: trigger a node-wide shutdown so the
+		// other services stop without waiting for the user to hit Ctrl+C, then
+		// re-panic so the WaitGroup can propagate it (with its stack) to the
+		// caller. Registered first so it also covers a nil service.
+		defer func() {
+			if r := recover(); r != nil {
+				cancel()
+				panic(r)
+			}
+		}()
+
+		var name string
+		if s != nil {
+			name = reflect.TypeOf(s).String()
+		}
+
 		if err := s.Run(ctx); err != nil {
-			n.logger.Error(
-				"Service error",
-				zap.String("name", reflect.TypeOf(s).String()),
-				zap.Error(err),
-			)
+			// A service that returns an error is critical: bring the node down.
+			n.logger.Error("Service error", zap.String("name", name), zap.Error(err))
+			cancel()
+			return
+		}
+
+		// The service returned without an error. Let it exit on its own without
+		// taking the rest of the node down. A clean return before shutdown was
+		// requested is unexpected for a long-running service, so flag it.
+		if ctx.Err() == nil {
+			n.logger.Warn("Service stopped before node shutdown was requested", zap.String("name", name))
+		} else {
+			n.logger.Debug("Service stopped", zap.String("name", name))
 		}
 	})
 }

node/node_service_test.go

@@ -0,0 +1,119 @@
+package node_test
+
+import (
+	"context"
+	"errors"
+	"testing"
+	"time"
+
+	"github.com/NethermindEth/juno/blockchain/networks"
+	"github.com/NethermindEth/juno/node"
+	"github.com/NethermindEth/juno/utils/log"
+	"github.com/sourcegraph/conc"
+	"github.com/stretchr/testify/require"
+)
+
+// fakeService lets each test drive what Run does.
+type fakeService struct {
+	run func(ctx context.Context) error
+}
+
+func (f *fakeService) Run(ctx context.Context) error { return f.run(ctx) }
+
+// TestStartService covers the three ways a service's Run can return and the
+// expected effect on the node-wide context:
+//   - error  -> shut the node down
+//   - panic  -> shut the node down and propagate the panic
+//   - nil    -> let the service exit without taking the node down
+func TestStartService(t *testing.T) {
+	// newNode builds a node through the public constructor with the minimal
+	// offline config also used by TestNetworkVerificationOnNonEmptyDB.
+	newNode := func(t *testing.T) *node.Node {
+		n, err := node.New(&node.Config{
+			DatabasePath:                       t.TempDir(),
+			DBCompression:                      "zstd",
+			Network:                            networks.Sepolia,
+			DisableL1Verification:              true,
+			SubmittedTransactionsCacheEntryTTL: time.Second,
+		}, "test", log.NewLevel(log.INFO))
+		require.NoError(t, err)
+		return n
+	}
+
+	t.Run("error shuts the node down", func(t *testing.T) {
+		n := newNode(t)
+		wg := conc.NewWaitGroup()
+		ctx, cancel := context.WithCancel(t.Context())
+		defer cancel()
+
+		failing := &fakeService{run: func(context.Context) error {
+			return errors.New("boom")
+		}}
+		n.StartService(wg, ctx, cancel, failing)
+		wg.Wait()
+
+		require.Error(t, ctx.Err(), "a service error should shut the node down")
+	})
+
+	t.Run("panic shuts the node down and propagates", func(t *testing.T) {
+		n := newNode(t)
+		wg := conc.NewWaitGroup()
+		ctx, cancel := context.WithCancel(t.Context())
+		defer cancel()
+
+		panicking := &fakeService{run: func(context.Context) error {
+			panic("boom")
+		}}
+		n.StartService(wg, ctx, cancel, panicking)
+
+		require.Panics(t, func() { wg.Wait() }, "a panicking service should propagate the panic")
+		require.Error(t, ctx.Err(), "a panicking service should shut the node down")
+	})
+
+	t.Run("clean return keeps the node running", func(t *testing.T) {
+		n := newNode(t)
+		wg := conc.NewWaitGroup()
+		ctx, cancel := context.WithCancel(t.Context())
+		defer cancel()
+
+		// Short-lived service: finishes its work and returns nil.
+		exited := make(chan struct{})
+		shortLived := &fakeService{run: func(context.Context) error {
+			close(exited)
+			return nil
+		}}
+		// Long-running service: lives until the node is shut down.
+		stopped := make(chan struct{})
+		longRunning := &fakeService{run: func(ctx context.Context) error {
+			<-ctx.Done()
+			close(stopped)
+			return nil
+		}}
+
+		n.StartService(wg, ctx, cancel, longRunning)
+		n.StartService(wg, ctx, cancel, shortLived)
+
+		select {
+		case <-exited: // the short-lived service has returned cleanly
+		case <-time.After(10 * time.Second):
+			t.Fatal("short-lived service did not exit in time")
+		}
+
+		// Its clean exit must not take the long-running service down with it.
+		select {
+		case <-stopped:
+			t.Fatal("long-running service stopped: node was shut down unexpectedly")
+		default:
+		}
+		require.NoError(t, ctx.Err(), "a clean exit should not shut the node down")
+
+		// A real shutdown still stops everything.
+		cancel()
+		wg.Wait()
+		select {
+		case <-stopped:
+		case <-time.After(10 * time.Second):
+			t.Fatal("long-running service did not stop in time")
+		}
+	})
+}

service/service.go

@@ -3,7 +3,9 @@ package service
 import "context"
 
 type Service interface {
-	// Run starts the service and returns an error. Services should not return non-critical errors,
-	// instead they should log and retry. Services should be blocking.
+	// Run starts the service. Services should not return non-critical errors,
+	// instead they should log and retry. If a service returns an error the node
+	// shuts down. If a service returns a nil error, it is assumed its work has
+	// finished safely.
 	Run(ctx context.Context) error
 }