From 8f6d3f5df4c8250b75499265c98b42dba93fa69c Mon Sep 17 00:00:00 2001
From: Ivan Kozlovic <ivan.kozlovic@apcera.com>
Date: Thu, 7 Sep 2017 10:28:35 -0600
Subject: [PATCH] Added error check for fs.Parse() call

This is just for tests since from main.go, the FlagSet is set
with ExitOnError so Parse() would call os.Exit(2).
Regardless, I wanted to add error checking and a test for that.

Related to #578
---
 server/opts.go      | 7 ++++++-
 server/opts_test.go | 8 ++++++++
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/server/opts.go b/server/opts.go
index 66af080d3bb..fb4c45eea42 100644
--- a/server/opts.go
+++ b/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)
 	}
 
diff --git a/server/opts_test.go b/server/opts_test.go
index fbd97a81561..0723db18e33 100644
--- a/server/opts_test.go
+++ b/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 unkown flag
+	expectToFail([]string{"-xxx", "foo"}, "flag")
+
 	// Should fail because of config file missing
 	expectToFail([]string{"-c", "xxx.cfg"}, "file")