Merge pull request #579 from nats-io/check_parse_returned_error

Added error check for fs.Parse() call

server/opts.go

@@ -986,7 +986,9 @@ func ConfigureOptions(fs *flag.FlagSet, args []string) (*Options, error) {
 	// Calling Parse() before processing config file is necessary since configFile
 	// itself is a command line argument, and also Parse() is required in order
 	// to know if user wants simply to show "help" or "version", etc...
-	fs.Parse(args)
+	if err := fs.Parse(args); err != nil {
+		return nil, err
+	}
 
 	// Show version and exit
 	if showVersion {
@@ -1026,6 +1028,9 @@ func ConfigureOptions(fs *flag.FlagSet, args []string) (*Options, error) {
 			return nil, err
 		}
 		// Call this again to override config file options with options from command line.
+		// Note: We don't need to check error here since if there was an error, it would
+		// have been caught the first time this function was called (after setting up the
+		// flags).
 		fs.Parse(args)
 	}
 

server/opts_test.go

@@ -3,6 +3,7 @@
 package server
 
 import (
+	"bytes"
 	"crypto/tls"
 	"flag"
 	"io/ioutil"
@@ -853,6 +854,10 @@ func TestConfigureOptions(t *testing.T) {
 	// Helper function that expect configuration to fail.
 	expectToFail := func(args []string, errContent ...string) {
 		fs := flag.NewFlagSet("test", flag.ContinueOnError)
+		// Silence the flagSet so that on failure nothing is printed.
+		// (flagSet would print error message about unknown flags, etc..)
+		silenceOuput := &bytes.Buffer{}
+		fs.SetOutput(silenceOuput)
 		opts, err := ConfigureOptions(fs, args)
 		if opts != nil || err == nil {
 			stackFatalf(t, "Expected no option and an error, got opts=%v and err=%v", opts, err)
@@ -875,6 +880,9 @@ func TestConfigureOptions(t *testing.T) {
 	// Should fail because of unknown parameter
 	expectToFail([]string{"foo"}, "command")
 
+	// Should fail because unknown flag
+	expectToFail([]string{"-xxx", "foo"}, "flag")
+
 	// Should fail because of config file missing
 	expectToFail([]string{"-c", "xxx.cfg"}, "file")