Quality checks

Check listener for nil and nil value and return error.
Check ChainHandler for nil and nil value and return error.
Check ChainHandler for returned nil and return error.
Some extra logging

AB#8685

grpcserver/grpcserver.go

@@ -28,13 +28,15 @@ type RegisterServer func(*grpcServer)
 func defaultRegisterServer(g *grpcServer) {}
 
 type GRPCServer struct {
-	name         string
-	log          Logger
-	listenStr    string
-	health       *grpchealth.HealthCheckingService
-	interceptors []grpcUnaryServerInterceptor
-	register     RegisterServer
-	server       *grpcServer
+	name          string
+	log           Logger
+	listenStr     string
+	health        bool
+	healthService *grpchealth.HealthCheckingService
+	interceptors  []grpcUnaryServerInterceptor
+	register      RegisterServer
+	server        *grpcServer
+	reflection    bool
 }
 
 type GRPCServerOption func(*GRPCServer)
@@ -57,6 +59,18 @@ func WithRegisterServer(r RegisterServer) GRPCServerOption {
 	}
 }
 
+func WithoutHealth() GRPCServerOption {
+	return func(g *GRPCServer) {
+		g.health = false
+	}
+}
+
+func WithReflection(r bool) GRPCServerOption {
+	return func(g *GRPCServer) {
+		g.reflection = r
+	}
+}
+
 func tracingFilter(ctx context.Context, fullMethodName string) bool {
 	if fullMethodName == grpcHealth.Health_Check_FullMethodName {
 		return false
@@ -69,17 +83,15 @@ func tracingFilter(ctx context.Context, fullMethodName string) bool {
 func New(log Logger, name string, opts ...GRPCServerOption) GRPCServer {
 	listenStr := fmt.Sprintf(":%s", env.GetOrFatal("PORT"))
 
-	health := grpchealth.New(log)
-
 	g := GRPCServer{
 		name:      strings.ToLower(name),
 		listenStr: listenStr,
-		health:    &health,
 		register:  defaultRegisterServer,
 		interceptors: []grpc.UnaryServerInterceptor{
 			grpc_otrace.UnaryServerInterceptor(grpc_otrace.WithFilterFunc(tracingFilter)),
 			grpc_validator.UnaryServerInterceptor(),
 		},
+		health: true,
 	}
 	for _, opt := range opts {
 		opt(&g)
@@ -93,8 +105,16 @@ func New(log Logger, name string, opts ...GRPCServerOption) GRPCServer {
 	// RegisterAccessPoliciesServer(s grpc.ServiceRegistrar, srv AccessPoliciesServer)
 	//accessPolicyV1API.RegisterAccessPoliciesServer(server, s)
 	g.register(server)
-	grpcHealth.RegisterHealthServer(server, &health)
-	reflection.Register(server)
+
+	if g.health {
+		healthService := grpchealth.New(log)
+		g.healthService = &healthService
+		grpcHealth.RegisterHealthServer(server, g.healthService)
+	}
+
+	if g.reflection {
+		reflection.Register(server)
+	}
 
 	g.server = server
 	g.log = log.WithIndex("grpcserver", g.String())
@@ -112,7 +132,9 @@ func (g *GRPCServer) Listen() error {
 		return fmt.Errorf("failed to listen %s: %w", g, err)
 	}
 
-	g.health.Ready() // readiness
+	if g.healthService != nil {
+		g.healthService.Ready() // readiness
+	}
 
 	g.log.Infof("Listen")
 	err = g.server.Serve(listen)
@@ -124,8 +146,10 @@ func (g *GRPCServer) Listen() error {
 
 func (g *GRPCServer) Shutdown(_ context.Context) error {
 	g.log.Infof("Shutdown")
-	g.health.NotReady() // readiness
-	g.health.Dead()     // liveness
+	if g.healthService != nil {
+		g.healthService.NotReady() // readiness
+		g.healthService.Dead()     // liveness
+	}
 	g.server.GracefulStop()
 	return nil
 }

httpserver/httpserver.go

@@ -5,10 +5,17 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"reflect"
 	"strings"
 	"time"
 )
 
+var (
+	ErrNilHandler            = errors.New("Nil Handler")
+	ErrNilHandlerValue       = errors.New("Nil Handler value")
+	ErrHandlerFuncReturnsNil = errors.New("Handler function returns nil")
+)
+
 type HandleChainFunc func(http.Handler) http.Handler
 
 // A http server that has an inbuilt logger, name and complies wuth the Listener interface in
@@ -18,47 +25,46 @@ type Server struct {
 	log      Logger
 	name     string
 	server   http.Server
+	handler  http.Handler
 	handlers []HandleChainFunc
 }
 
 type ServerOption func(*Server)
 
-// WithHandler adds a handler on the http endpoint.
-func WithHandler(h HandleChainFunc) ServerOption {
+// WithHandlers adds a handler on the http endpoint. If the handler is nil
+// then an error will occur when executing the Listen() method.
+func WithHandlers(handlers ...HandleChainFunc) ServerOption {
 	return func(s *Server) {
-		if h != nil {
-			s.handlers = append(s.handlers, h)
-		}
+		s.handlers = append(s.handlers, handlers...)
 	}
 }
 
-// WithHandlers adds a handler on the http endpoint.
-func WithHandlers(h []HandleChainFunc) ServerOption {
+// WithOptionalHandlers adds a handler on the http endpoint. If the handler is nil
+// it is ignored.
+func WithOptionalHandlers(handlers ...HandleChainFunc) ServerOption {
 	return func(s *Server) {
-		s.handlers = append(s.handlers, h...)
+		for i := 0; i < len(handlers); i++ {
+			handler := handlers[i]
+			if handler != nil && !reflect.ValueOf(handler).IsNil() {
+				s.handlers = append(s.handlers, handler)
+			}
+		}
 	}
 }
 
+// New creates a new httpserver. ChainHandlers that return nil are ignored.
 func New(log Logger, name string, port string, h http.Handler, opts ...ServerOption) *Server {
 	s := Server{
 		server: http.Server{
 			Addr: ":" + port,
 		},
-		name: strings.ToLower(name),
+		handler: h,
+		name:    strings.ToLower(name),
 	}
 	s.log = log.WithIndex("httpserver", s.String())
 	for _, opt := range opts {
 		opt(&s)
 	}
-
-	s.log.Debugf("Initialise handlers %v", h)
-	for _, handler := range s.handlers {
-		if handler != nil {
-			h = handler(h)
-		}
-	}
-	s.server.Handler = h
-
 	// It is preferable to return a copy rather than a reference. Unfortunately http.Server has an
 	// internal mutex and this cannot or should not be copied so we will return a reference instead.
 	return &s
@@ -71,6 +77,24 @@ func (s *Server) String() string {
 
 func (s *Server) Listen() error {
 	s.log.Infof("Listen")
+	h := s.handler
+	for i, handler := range s.handlers {
+		s.log.Debugf("%d: h %v handler %v", i, handler)
+		if handler == nil {
+			return ErrNilHandler
+		}
+		if reflect.ValueOf(handler).IsNil() {
+			return ErrNilHandlerValue
+		}
+		h1 := handler(h)
+		if h1 == nil {
+			return ErrHandlerFuncReturnsNil
+		}
+		h = h1
+	}
+	s.server.Handler = h
+
+	// this is a blocking operation
 	err := s.server.ListenAndServe()
 	if err != nil {
 		return fmt.Errorf("%s server terminated: %v", s, err)