refactor(services): discard the 'force' flag to operation that stop services

Author: renatomaia

internal/advancer/advancer.go

@@ -311,7 +311,7 @@ func (s *Service) processInputs(ctx context.Context, app *Application, inputs []
 				"epoch", input.EpochIndex,
 				"index", input.Index,
 				"error", err)
-			s.Supervisor.Stop(true) // triggers graceful shutdown of all services
+			s.Supervisor.Stop() // shutdown all services
 			return err
 		}
 

internal/advancer/advancer_test.go

@@ -118,8 +118,7 @@ func (s *AdvancerSuite) TestServiceInterface() {
 		require.Contains(tickErr.Error(), "list epochs error")
 
 		// Stop must be called last to cleanly shut down the service.
-		// It should complete without returning any errors.
-		require.NoError(advancer.Supervisor.Stop(false))
+		advancer.Supervisor.Stop()
 	})
 }
 

internal/advancer/service.go

@@ -138,15 +138,15 @@ func (s *Service) Tick(ctx context.Context) (bool, error) {
 	return hadWork, err
 }
 
-func (s *Service) Stop(b bool) error {
+func (s *Service) Stop() error {
 	var errs []error
 	if s.machineManager != nil {
 		s.Logger.Info("Closing machine manager")
 		if err := s.machineManager.Close(); err != nil {
 			errs = append(errs, fmt.Errorf("failed to close machine manager: %w", err))
 		}
 	}
-	if err := s.TickServiceTemplate.Stop(b); err != nil {
+	if err := s.TickServiceTemplate.Stop(); err != nil {
 		errs = append(errs, err)
 	}
 	return errors.Join(errs...)

internal/evmreader/service_config_test.go

@@ -97,7 +97,7 @@ func TestCreateAcceptsRequestTimeoutBelowPollingInterval(t *testing.T) {
 		Repository: repo,
 	})
 	require.NoError(t, err)
-	defer svc.Stop(false)
+	defer svc.Stop()
 
 	repo.AssertExpectations(t)
 }

internal/node/node.go

@@ -102,7 +102,7 @@ func createServices(
 
 	if len(errs) > 0 {
 		for _, svc := range services {
-			stopErr := svc.Stop(true)
+			stopErr := svc.Stop()
 			if stopErr != nil {
 				errs = append(errs, stopErr)
 			}

pkg/service/http_service.go

@@ -91,7 +91,7 @@ func InitHTTPServiceTemplate(
 	return nil
 }
 
-func (s *HTTPServiceTemplate) Stop(bool) error {
+func (s *HTTPServiceTemplate) Stop() error {
 	s.Logger.Info("Shutting down HTTP service", "addr", s.Server.Addr)
 	ctx, cancel := context.WithTimeout(context.Background(), s.shutdownTimeout)
 	defer cancel()

pkg/service/service.go

@@ -66,19 +66,19 @@ var (
 	ErrServiceStopped = fmt.Errorf("service was stopped")
 )
 
-// Public interface with methods to manipulate the service.
-type IService interface {
+// Basic methods to manipulate a service.
+type BaseService interface {
 	Alive() bool
 	Ready() bool
 	Reload() error
-	Stop(bool) error
 	Serve() error
 	String() string
 }
 
 // Service interface with a service supervisor.
 type SupervisedService interface {
-	IService
+	BaseService
+	Stop() error
 	SetSupervisor(*Supervisor)
 }
 
@@ -116,10 +116,10 @@ func InitServiceTemplate(c *BaseConfigs, s *BaseTemplate) error {
 
 // Default implementation of some abstract methods (except `Serve`).
 // Remove them to force concrete services to provide implementation for them.
-func (s *BaseTemplate) Reload() error  { return nil }
-func (s *BaseTemplate) Stop(bool) error    { return nil }
-func (s *BaseTemplate) Alive() bool    { return true }
-func (s *BaseTemplate) Ready() bool    { return true }
+func (s *BaseTemplate) Reload() error { return nil }
+func (s *BaseTemplate) Stop() error { return nil }
+func (s *BaseTemplate) Alive() bool { return true }
+func (s *BaseTemplate) Ready() bool { return true }
 func (s *BaseTemplate) String() string { return s.Name }
 
 func (s *BaseTemplate) SetSupervisor(supervisor *Supervisor) {

pkg/service/service_test.go

@@ -15,7 +15,7 @@ type delayedCloseImpl struct {
 	onStopInitChan chan struct{}
 }
 
-func (s *delayedCloseImpl) Stop(bool) error {
+func (s *delayedCloseImpl) Stop() error {
 	<-s.onStopInitChan  // wait signal to initiate stop
 	return nil
 }
@@ -51,13 +51,12 @@ func (s *ServeSuite) TestServeExitsAfterStopIsComplete() {
 		close(onServeEndChan)
 	}()
 
-	onStopEndChan := make(chan error)
+	onStopEndChan := make(chan struct{})
 	select {
 	case <-svc.onServeInitChan:  // wait service to initiate, so can be stopped.
 		// initiate service shutdown through context cancelation
 		go func() {
-			err := supervisor.Stop(true)
-			onStopEndChan <- err  // signal stop ended and provide the errors
+			supervisor.Stop()
 			close(onStopEndChan)
 		}()
 	case <-time.After(2 * time.Second):
@@ -86,8 +85,7 @@ func (s *ServeSuite) TestServeExitsAfterStopIsComplete() {
 
 	// Stop() should exit without errors.
 	select {
-	case err := <-onStopEndChan:
-		s.NoError(err)
+	case <-onStopEndChan:
 	case <-time.After(2 * time.Second):
 		s.Fail("Stop() did not exit within 2 seconds after 'OnStop' concluded")
 	}

pkg/service/supervisor.go

@@ -14,6 +14,11 @@ import (
 	"time"
 )
 
+type IService interface {
+	BaseService
+	Stop()
+}
+
 type SupervisorConfigs struct {
 	BaseConfigs
 	Services             []SupervisedService
@@ -164,13 +169,12 @@ func (s *Supervisor) Serve() error {
 				if err != nil {
 					s.Logger.Error("Service failed to restart", "error", err)
 				}
+				continue
 			case <-s.sigShutdown:
-				s.Stop(false) // Graceful shutdown; errors are logged by Stop.
-				return
 			case <-s.context.Done():
-				s.Stop(true) // Stop logs errors internally.
-				return
 			}
+			s.Stop()
+			return
 		}
 	}()
 
@@ -188,7 +192,7 @@ func (s *Supervisor) Serve() error {
 					"service", svc.String(),
 					"error", err,
 				)
-				s.Stop(false)
+				s.Stop()
 			case s.stopped.Load():
 				s.Logger.Info("Service stopped",
 					"service", svc.String(),
@@ -209,19 +213,19 @@ func (s *Supervisor) Serve() error {
 		}
 	}
 
-	go s.Stop(true)
+	go s.Stop()
 	<-s.stoppedChan
 
 	return errors.Join(allErrs...)
 }
 
-func (s *Supervisor) Stop(force bool) error {
+func (s *Supervisor) Stop() {
 	// CAS achieves once-semantics: the second caller returns immediately
 	// (fire-and-forget) rather than blocking like sync.Once. This is safe
 	// because the orchestrator calls Cancel() after Stop() and waits for
 	// the Serve goroutine to exit.
 	if !s.stopped.CompareAndSwap(false, true) {
-		return nil // already stopped
+		return // already stopped
 	}
 
 	if s.sigShutdown != nil {
@@ -231,29 +235,19 @@ func (s *Supervisor) Stop(force bool) error {
 		signal.Stop(s.sigHangUp)
 	}
 
-	var err error
 	for i := len(s.services)-1; i >= 0; i-- {
 		svc := s.services[i]
 		start := time.Now()
-		svcErr := svc.Stop(force)
+		err := svc.Stop()
 		elapsed := time.Since(start)
 
-		if svcErr != nil {
-			s.Logger.Error("Stop",
-				"force", force,
-				"duration", elapsed,
-				"error", svcErr)
+		if err != nil {
+			s.Logger.Error("Stop", "duration", elapsed, "error", err)
 		} else {
-			s.Logger.Info("Stop",
-				"force", force,
-				"duration", elapsed)
+			s.Logger.Info("Stop", "duration", elapsed)
 		}
-
-		err = errors.Join(err, svcErr)
 	}
 
 	s.cancelContext()
 	close(s.stoppedChan)
-
-	return err
 }

pkg/service/supervisor_test.go

@@ -83,7 +83,7 @@ func (s *SupervisorSuite) TestNodeStopCancelsChildContexts() {
 		s.Fail("child-2 did not start")
 	}
 
-	supervisor.Stop(false)
+	supervisor.Stop()
 
 	select {
 	case <-child1.done:
@@ -144,7 +144,7 @@ func (c *stopAwareChildImpl) Serve() error {
 	return nil
 }
 
-func (c *stopAwareChildImpl) Stop(bool) error {
+func (c *stopAwareChildImpl) Stop() error {
 	c.stopOnce.Do(func() { close(c.stopped) })
 	return nil
 }