Refactoring custom listener

web/listener.go

@@ -3,7 +3,8 @@ package web
 import (
 	"context"
 	"fmt"
-	"time"
+
+	"go.uber.org/zap"
 
 	"github.com/im-kulikov/helium/internal"
 	"github.com/im-kulikov/helium/service"
@@ -17,11 +18,12 @@ type (
 	}
 
 	listener struct {
-		name            string
-		skipErrors      bool
-		ignoreErrors    []error
-		server          Listener
-		shutdownTimeout time.Duration
+		logger *zap.Logger
+
+		name         string
+		skipErrors   bool
+		ignoreErrors []error
+		server       Listener
 	}
 
 	// ListenerOption options that allows to change
@@ -47,17 +49,20 @@ func ListenerIgnoreError(errors ...error) ListenerOption {
 	}
 }
 
-// ListenerShutdownTimeout allows changing the default shutdown timeout.
-func ListenerShutdownTimeout(v time.Duration) ListenerOption {
+// ListenerName allows changing the default listener name.
+func ListenerName(v string) ListenerOption {
 	return func(l *listener) {
-		l.shutdownTimeout = v
+		l.name = v
 	}
 }
 
-// ListenerName allows changing the default listener name.
-func ListenerName(v string) ListenerOption {
+func ListenerWithLogger(log *zap.Logger) ListenerOption {
 	return func(l *listener) {
-		l.name = v
+		if log == nil {
+			return
+		}
+
+		l.logger = log
 	}
 }
 
@@ -68,9 +73,8 @@ func NewListener(lis Listener, opts ...ListenerOption) (service.Service, error)
 	}
 
 	s := &listener{
-		server:          lis,
-		skipErrors:      false,
-		shutdownTimeout: time.Second * 30,
+		server:     lis,
+		skipErrors: false,
 
 		// Default name
 		name: fmt.Sprintf("listener %T", lis),
@@ -89,43 +93,26 @@ func (l *listener) Name() string { return l.name }
 // Start tries to start the Listener and returns an error
 // if the Listener is empty. If something went wrong and
 // errors not ignored should panic.
-func (l *listener) Start(_ context.Context) error {
+func (l *listener) Start(context.Context) error {
 	if l.server == nil {
 		return l.catch(ErrEmptyListener)
 	}
 
-	go func() {
-		if err := l.catch(l.server.ListenAndServe()); err != nil {
-			fmt.Printf("could not start Listener: %v\n", err)
-			fatal(2)
-		}
-	}()
-
-	return nil
+	return l.catch(l.server.ListenAndServe())
 }
 
 // Stop tries to stop the Listener and returns an error
 // if something went wrong. Ignores errors that were passed
 // by options and if used skip errors.
-func (l *listener) Stop() error {
+func (l *listener) Stop(ctx context.Context) {
 	if l.server == nil {
-		return l.catch(ErrEmptyListener)
+		panic(ErrEmptyListener)
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), l.shutdownTimeout)
-	defer cancel()
-
-	ch := make(chan error, 1)
-
-	go func() {
-		ch <- l.catch(l.server.Shutdown(ctx))
-	}()
-
-	select {
-	case <-ctx.Done():
-		return nil
-	case err := <-ch:
-		return err
+	if err := l.catch(l.server.Shutdown(ctx)); err != nil {
+		l.logger.Error("could not stop listener",
+			zap.String("name", l.name),
+			zap.Error(err))
 	}
 }
 

web/listener_test.go

@@ -7,30 +7,19 @@ import (
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
 )
 
 type (
 	fakeListener struct {
 		startError error
 		stopError  error
 	}
-
-	slowlyListener struct{}
 )
 
 const listenerTestName = "test-name"
 
-var (
-	_ Listener = (*fakeListener)(nil)
-	_ Listener = (*slowlyListener)(nil)
-)
-
-func (s slowlyListener) ListenAndServe() error { return nil }
-
-func (s slowlyListener) Shutdown(_ context.Context) error {
-	time.Sleep(time.Second * 10)
-	return nil
-}
+var _ Listener = (*fakeListener)(nil)
 
 func (f fakeListener) ListenAndServe() error {
 	return f.startError
@@ -41,21 +30,26 @@ func (f fakeListener) Shutdown(context.Context) error {
 }
 
 func TestListenerService(t *testing.T) {
-	t.Run("should set network", func(t *testing.T) {
+	log := zap.NewNop()
+	ctx, cancel := context.WithTimeout(context.Background(), time.Millisecond)
+	defer cancel()
+
+	t.Run("should be configured", func(t *testing.T) {
 		serve, err := NewListener(
 			&fakeListener{},
 			ListenerIgnoreError(ErrEmptyListener),
 			ListenerSkipErrors(),
 			ListenerName(listenerTestName),
-			ListenerShutdownTimeout(time.Second))
+			ListenerWithLogger(nil), // should ignore
+			ListenerWithLogger(log))
 		require.NoError(t, err)
 
 		s, ok := serve.(*listener)
 		require.True(t, ok)
 		require.True(t, s.skipErrors)
-		require.Equal(t, time.Second, s.shutdownTimeout)
+		require.Equal(t, log, s.logger)
+		require.Equal(t, listenerTestName, s.Name())
 		require.Equal(t, ErrEmptyListener, s.ignoreErrors[0])
-		require.Equal(t, s.name, listenerTestName)
 	})
 
 	t.Run("should fail on empty server", func(t *testing.T) {
@@ -65,34 +59,37 @@ func TestListenerService(t *testing.T) {
 	})
 
 	t.Run("should fail on Start and Stop", func(t *testing.T) {
-		require.EqualError(t, (&listener{}).Start(context.Background()), ErrEmptyListener.Error())
-		require.EqualError(t, (&listener{}).Stop(), ErrEmptyListener.Error())
+		require.EqualError(t, (&listener{}).Start(ctx), ErrEmptyListener.Error())
+		require.Panics(t, func() {
+			(&listener{logger: log}).Stop(ctx)
+		}, ErrEmptyListener.Error())
 	})
 
 	t.Run("should successfully start and stop", func(t *testing.T) {
-		require.NoError(t, (&listener{server: &fakeListener{}}).Start(context.Background()))
-		require.NoError(t, (&listener{server: &fakeListener{}}).Stop())
+		require.NoError(t, (&listener{server: &fakeListener{}}).Start(ctx))
+		require.NotPanics(t, func() {
+			(&listener{
+				logger: log,
+				server: &fakeListener{stopError: ErrEmptyLogger},
+			}).Stop(ctx)
+		})
 	})
 
 	t.Run("should skip errors", func(t *testing.T) {
 		s := &fakeListener{stopError: errors.New("stopping")}
 		serve, err := NewListener(s, ListenerSkipErrors())
 		require.NoError(t, err)
-		require.NoError(t, serve.Stop())
+		require.NotPanics(t, func() {
+			serve.Stop(ctx)
+		})
 	})
 
 	t.Run("should ignore errors", func(t *testing.T) {
 		s := &fakeListener{stopError: ErrEmptyListener}
 		serve, err := NewListener(s, ListenerIgnoreError(ErrEmptyListener))
 		require.NoError(t, err)
-		require.NoError(t, serve.Stop())
-	})
-
-	t.Run("should fail on stop", func(t *testing.T) {
-		s := &slowlyListener{}
-		serve, err := NewListener(s, ListenerShutdownTimeout(time.Second))
-		require.NoError(t, err)
-		require.NoError(t, serve.Stop())
-		require.NotEmpty(t, serve.Name())
+		require.NotPanics(t, func() {
+			serve.Stop(ctx)
+		})
 	})
 }