Remove panics from web package

- refactored tests
- removed panics from web package
- fixed typo for method comment

service/service.go

@@ -22,7 +22,7 @@ type (
 		Name() string
 	}
 
-	// Group wrapper around group of servicesl.
+	// Group wrapper around group of services.
 	Group interface {
 		Run(context.Context) error
 	}

web/grpc.go

@@ -122,7 +122,7 @@ func (g *gRPC) Name() string {
 
 // Start tries to start gRPC service.
 // If something went wrong it returns an error.
-// If could not start server panics.
+// If service could not start returns an error.
 func (g *gRPC) Start(context.Context) error {
 	if g.server == nil {
 		return ErrEmptyGRPCServer

web/grpc_test.go

@@ -49,10 +49,14 @@ func TestGRPCService(t *testing.T) {
 	})
 
 	t.Run("should fail on Start and Stop", func(t *testing.T) {
-		require.EqualError(t, (&gRPC{}).Start(nil), ErrEmptyGRPCServer.Error())
-		require.Panics(t, func() {
-			(&gRPC{}).Stop(nil)
-		}, ErrEmptyGRPCServer.Error())
+		log := newTestLogger()
+
+		require.EqualError(t, (&gRPC{logger: log.Logger}).Start(nil), ErrEmptyGRPCServer.Error())
+		log.Cleanup()
+
+		(&gRPC{logger: log.Logger}).Stop(nil)
+		require.NoError(t, log.Decode())
+		require.EqualError(t, log, ErrEmptyGRPCServer.Error())
 	})
 
 	t.Run("should fail on net.Listen", func(t *testing.T) {

web/http_test.go

@@ -1,22 +1,83 @@
 package web
 
 import (
+	"bytes"
 	"context"
 	"crypto/tls"
+	"encoding/json"
+	"io"
 	"net"
 	"net/http"
 	"testing"
 	"time"
 
 	"github.com/stretchr/testify/require"
+	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 	"go.uber.org/zap/zaptest"
 	"google.golang.org/grpc/test/bufconn"
 
 	"github.com/im-kulikov/helium/group"
 	"github.com/im-kulikov/helium/internal"
 )
 
+type testLogger struct {
+	*zap.Logger
+	*bytes.Buffer
+
+	Result *testLogResult
+}
+
+type fakeWriteSyncer struct {
+	io.Writer
+}
+
+var _ zapcore.WriteSyncer = (*fakeWriteSyncer)(nil)
+
+func (f *fakeWriteSyncer) Sync() error { return nil }
+
+type testLogResult struct {
+	L, T, M string
+	E       string `json:"error"`
+	N       string `json:"name"`
+}
+
+func newTestLogger() *testLogger {
+	buf := new(bytes.Buffer)
+
+	return &testLogger{
+		Buffer: buf,
+
+		Logger: zap.New(zapcore.NewCore(
+			zapcore.NewJSONEncoder(zap.NewDevelopmentEncoderConfig()),
+			&fakeWriteSyncer{Writer: buf},
+			zapcore.DebugLevel)),
+
+		Result: new(testLogResult),
+	}
+}
+
+func (tl *testLogger) Error() string {
+	return tl.Result.E
+}
+
+func (tl *testLogger) Cleanup() {
+	tl.Buffer.Reset()
+	tl.Result = new(testLogResult)
+}
+
+func (tl *testLogger) Empty() bool {
+	return tl.Buffer.String() == ""
+}
+
+func (tl *testLogger) Decode() error {
+	return json.NewDecoder(tl.Buffer).Decode(&tl.Result)
+}
+
 func TestHTTPService(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), time.Second*15)
+	defer cancel()
+
 	t.Run("should set network and address", func(t *testing.T) {
 		lis, err := net.Listen("tcp", "127.0.0.1:0")
 		require.NoError(t, err)
@@ -51,10 +112,14 @@ func TestHTTPService(t *testing.T) {
 	})
 
 	t.Run("should fail on Start and Stop", func(t *testing.T) {
-		require.EqualError(t, (&httpService{}).Start(context.Background()), ErrEmptyHTTPServer.Error())
-		require.Panics(t, func() {
-			(&httpService{}).Stop(context.Background())
-		}, ErrEmptyHTTPServer.Error())
+		log := newTestLogger()
+
+		require.EqualError(t, (&httpService{logger: log.Logger}).Start(ctx), ErrEmptyHTTPServer.Error())
+		log.Cleanup()
+
+		(&httpService{logger: log.Logger}).Stop(ctx)
+		require.NoError(t, log.Decode())
+		require.EqualError(t, log, ErrEmptyHTTPServer.Error())
 	})
 
 	t.Run("should fail on net.Listen", func(t *testing.T) {
@@ -74,11 +139,11 @@ func TestHTTPService(t *testing.T) {
 		require.NoError(t, err)
 		require.NotEmpty(t, serve.Name())
 
-		ctx, cancel := context.WithCancel(context.Background())
+		top, stop := context.WithCancel(ctx)
 		require.NoError(t, group.New().
-			Add(serve.Start, func(context.Context) { serve.Stop(ctx) }).
+			Add(serve.Start, func(context.Context) { serve.Stop(top) }).
 			Add(func(ctx context.Context) error {
-				cancel()
+				stop()
 
 				con, errConn := lis.Dial()
 				if errConn != nil {
@@ -94,7 +159,7 @@ func TestHTTPService(t *testing.T) {
 
 				return nil
 			}, func(context.Context) {}).
-			Run(ctx))
+			Run(top))
 	})
 
 	t.Run("should not fail for tls", func(t *testing.T) {
@@ -114,14 +179,14 @@ func TestHTTPService(t *testing.T) {
 		require.NoError(t, err)
 		require.NotEmpty(t, serve.Name())
 
-		ctx, cancel := context.WithCancel(context.Background())
+		top, stop := context.WithCancel(ctx)
 		require.NoError(t, group.New().
 			Add(serve.Start, serve.Stop).
 			Add(func(context.Context) error {
-				cancel()
+				stop()
 
 				return nil
 			}, func(context.Context) {}).
-			Run(ctx))
+			Run(top))
 	})
 }

web/listener.go

@@ -93,8 +93,8 @@ func NewListener(lis Listener, opts ...ListenerOption) (service.Service, error)
 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.
+// if the Listener is empty. If something went wrong
+// returns an error.
 func (l *listener) Start(context.Context) error {
 	if l.server == nil {
 		return l.catch(ErrEmptyListener)
@@ -108,7 +108,10 @@ func (l *listener) Start(context.Context) error {
 // by options and if used skip errors.
 func (l *listener) Stop(ctx context.Context) {
 	if l.server == nil {
-		panic(ErrEmptyListener)
+		l.logger.Error(ErrEmptyListener.Error(),
+			zap.String("name", l.name))
+
+		return
 	}
 
 	if err := l.catch(l.server.Shutdown(ctx)); err != nil {

web/listener_test.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zapcore"
 
 	"github.com/im-kulikov/helium/internal"
 )
@@ -64,38 +65,66 @@ func TestListenerService(t *testing.T) {
 		require.EqualError(t, err, ErrEmptyListener.Error())
 	})
 
-	t.Run("should fail on Start and Stop", func(t *testing.T) {
-		require.EqualError(t, (&listener{}).Start(ctx), ErrEmptyListener.Error())
-		require.Panics(t, func() {
-			(&listener{logger: log}).Stop(ctx)
-		}, ErrEmptyListener.Error())
+	t.Run("should error on Start and Stop", func(t *testing.T) {
+		l := newTestLogger()
+
+		require.EqualError(t, (&listener{logger: l.Logger}).Start(ctx), ErrEmptyListener.Error())
+		l.Cleanup()
+
+		(&listener{name: listenerTestName, logger: l.Logger}).Stop(ctx)
+		require.NoError(t, l.Decode())
+
+		require.Equal(t, l.Result.N, listenerTestName)
+		require.Equal(t, l.Result.M, ErrEmptyListener.Error())
+		require.Equal(t, l.Result.L, zapcore.ErrorLevel.CapitalString())
 	})
 
 	t.Run("should successfully start and stop", func(t *testing.T) {
-		require.NoError(t, (&listener{server: &fakeListener{}}).Start(ctx))
-		require.NotPanics(t, func() {
-			(&listener{
-				logger: log,
-				server: &fakeListener{stopError: ErrEmptyLogger},
-			}).Stop(ctx)
-		})
+		l := newTestLogger()
+
+		require.NoError(t, (&listener{
+			logger: l.Logger,
+			server: &fakeListener{},
+		}).Start(ctx))
+
+		l.Cleanup()
+
+		(&listener{
+			name:   listenerTestName,
+			logger: l.Logger,
+			server: &fakeListener{stopError: ErrEmptyListener},
+		}).Stop(ctx)
+		require.NoError(t, l.Decode())
+
+		require.Equal(t, l.Result.N, listenerTestName)
+		require.Equal(t, l.Result.E, ErrEmptyListener.Error())
+		require.Equal(t, l.Result.M, "could not stop listener")
+		require.Equal(t, l.Result.L, zapcore.ErrorLevel.CapitalString())
 	})
 
 	t.Run("should skip errors", func(t *testing.T) {
-		s := &fakeListener{stopError: errStopping}
-		serve, err := NewListener(s, ListenerSkipErrors())
+		l := newTestLogger()
+		lis := &fakeListener{stopError: errStopping}
+
+		serve, err := NewListener(lis,
+			ListenerSkipErrors(),
+			ListenerWithLogger(l.Logger))
 		require.NoError(t, err)
-		require.NotPanics(t, func() {
-			serve.Stop(ctx)
-		})
+
+		serve.Stop(ctx)
+		require.True(t, l.Empty())
 	})
 
 	t.Run("should ignore errors", func(t *testing.T) {
-		s := &fakeListener{stopError: ErrEmptyListener}
-		serve, err := NewListener(s, ListenerIgnoreError(ErrEmptyListener))
+		l := newTestLogger()
+		lis := &fakeListener{stopError: ErrEmptyListener}
+
+		serve, err := NewListener(lis,
+			ListenerIgnoreError(ErrEmptyListener),
+			ListenerWithLogger(l.Logger))
 		require.NoError(t, err)
-		require.NotPanics(t, func() {
-			serve.Stop(ctx)
-		})
+
+		serve.Stop(ctx)
+		require.True(t, l.Empty())
 	})
 }