From 8fad570c1855cf2800d711289b8c825e292cabb6 Mon Sep 17 00:00:00 2001 From: Evgeniy Kulikov Date: Mon, 19 Apr 2021 03:55:13 +0300 Subject: [PATCH] Refactoring custom listener --- web/listener.go | 67 ++++++++++++++++++-------------------------- web/listener_test.go | 59 ++++++++++++++++++-------------------- 2 files changed, 55 insertions(+), 71 deletions(-) diff --git a/web/listener.go b/web/listener.go index e9d1df1..a51d28c 100644 --- a/web/listener.go +++ b/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)) } } diff --git a/web/listener_test.go b/web/listener_test.go index 26f7605..b3bf1e0 100644 --- a/web/listener_test.go +++ b/web/listener_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/stretchr/testify/require" + "go.uber.org/zap" ) type ( @@ -14,23 +15,11 @@ type ( 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) + }) }) }