Review: fail fast on CLI arg validation + simplify check

Signed-off-by: Alexander Bezzubov <[email protected]>

Author: bzz

cmd/util.go

@@ -16,33 +16,29 @@ const (
 	maxMsgSizeCLIDesc = "max. message size to send/receive to/from clients (in MB)"
 
 	// DefaulGRPCMaxSendRecvMsgSizeMB is maximum msg size for gRPC in MB.
-	DefaulGRPCMaxSendRecvMsgSizeMB = 100
+	defaulGRPCMaxSendRecvMsgSizeMB = 100
 	maxMsgSizeCapMB                = 2048
 )
 
 // GRPCSizeOptions returns a slice of gRPC server options with the max
 // message size the server can send/receive set.
-// If a >2GB value is requested: the maximum size limit is capped
-// at 100 MB and an error is returned.
+// Error is returned if requested size is bigger than 2GB.
 // It is intended to be shared by gRPC in bblfshd Server and Drivers.
-func GRPCSizeOptions(maxMessageSizeMB int) ([]grpc.ServerOption, error) {
-	var err error
-	sizeMB := maxMessageSizeMB
+func GRPCSizeOptions(sizeMB int) ([]grpc.ServerOption, error) {
 	if sizeMB >= maxMsgSizeCapMB || sizeMB <= 0 {
-		err = fmt.Errorf("%s=%d is too big (limit is %dMB), using %d instead",
-			maxMsgSizeCLIName, sizeMB, maxMsgSizeCapMB-1, DefaulGRPCMaxSendRecvMsgSizeMB)
-		sizeMB = DefaulGRPCMaxSendRecvMsgSizeMB
+		return nil, fmt.Errorf("%s=%d value should be in between 1 and %dMB",
+			maxMsgSizeCLIName, sizeMB, maxMsgSizeCapMB-1)
 	}
 
 	sizeBytes := sizeMB * 1024 * 1024
 	return []grpc.ServerOption{
 		grpc.MaxRecvMsgSize(sizeBytes),
 		grpc.MaxSendMsgSize(sizeBytes),
-	}, err
+	}, nil
 }
 
-// MaxSendRecvMsgSizeMB sets the CLI configuation flag for max
+// FlagMaxGRPCMsgSizeMB sets the CLI configuation flag for max
 // gRPC send/recive msg size.
-func MaxSendRecvMsgSizeMB(fs *flag.FlagSet) *int {
-	return fs.Int(maxMsgSizeCLIName, DefaulGRPCMaxSendRecvMsgSizeMB, maxMsgSizeCLIDesc)
+func FlagMaxGRPCMsgSizeMB(fs *flag.FlagSet) *int {
+	return fs.Int(maxMsgSizeCLIName, defaulGRPCMaxSendRecvMsgSizeMB, maxMsgSizeCLIDesc)
 }

cmd/util_test.go

@@ -7,19 +7,24 @@ import (
 )
 
 func TestGRPCOptions_InvalidInput(t *testing.T) {
+	// too small
 	opts, err := GRPCSizeOptions(-1)
+	require.Error(t, err, "a small value should not be applied")
+	require.Nil(t, opts)
 
-	require.Error(t, err, "a given value was not applied")
-	require.NotNil(t, opts)
-	require.Len(t, opts, 2)
-	// does not work as expected for Func values like grpc.ServerOption
-	//require.Contains(t, opts, grpc.MaxRecvMsgSize(DefaulGRPCMaxSendRecvMsgSizeMB))
-	//require.Contains(t, opts, grpc.MaxSendMsgSize(DefaulGRPCMaxSendRecvMsgSizeMB))
+	// too big
+	opts, err = GRPCSizeOptions(maxMsgSizeCapMB + 1)
+	require.Error(t, err, "a big value should not be applied")
+	require.Nil(t, opts)
 }
 
 func TestGRPCOptions_ValidInput(t *testing.T) {
 	opts, err := GRPCSizeOptions(32)
 
 	require.Nil(t, err)
 	require.NotNil(t, opts)
+	require.Len(t, opts, 2)
+	// does not work as expected for Func values like grpc.ServerOption
+	//require.Contains(t, opts, grpc.MaxRecvMsgSize(DefaulGRPCMaxSendRecvMsgSizeMB))
+	//require.Contains(t, opts, grpc.MaxSendMsgSize(DefaulGRPCMaxSendRecvMsgSizeMB))
 }

driver/server/server.go

@@ -75,7 +75,8 @@ func (s *Server) initialize() error {
 
 	grpcOpts, err := cmdutil.GRPCSizeOptions(*maxMessageSize)
 	if err != nil {
-		s.Logger.Warningf(err.Error())
+		s.Logger.Errorf(err.Error())
+		os.Exit(1)
 	}
 	s.Options = grpcOpts
 
@@ -103,7 +104,7 @@ func (s *Server) initializeFlags() {
 	cmd := flag.NewFlagSet("server", flag.ExitOnError)
 	network = cmd.String("network", defaultNetwork, "network type: tcp, tcp4, tcp6, unix or unixpacket.")
 	address = cmd.String("address", defaultAddress, "address to listen.")
-	maxMessageSize = cmdutil.MaxSendRecvMsgSizeMB(cmd)
+	maxMessageSize = cmdutil.FlagMaxGRPCMsgSizeMB(cmd)
 
 	logs.level = cmd.String("log-level", defaultVerbose, "log level: panic, fatal, error, warning, info, debug.")
 	logs.format = cmd.String("log-format", defaultFormat, "format of the logs: text or json.")