Print errors to stderr

12 files changed, 80 insertions, 59 deletions

diff --git a/client/go/cmd/api_key_test.go b/client/go/cmd/api_key_test.go
index 1deb628c21e..b08758ae21d 100644
--- a/client/go/cmd/api_key_test.go
+++ b/client/go/cmd/api_key_test.go
@@ -17,7 +17,7 @@ func TestAPIKey(t *testing.T) {
 	out, _ := execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil)
 	assert.Contains(t, out, "Success: API private key written to "+keyFile+"\n")
 
-	out, _ = execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil)
-	assert.Contains(t, out, "Error: File "+keyFile+" already exists\nHint: Use -f to overwrite it\n")
+	out, outErr := execute(command{args: []string{"api-key", "-a", "t1.a1.i1"}, homeDir: homeDir}, t, nil)
+	assert.Contains(t, outErr, "Error: File "+keyFile+" already exists\nHint: Use -f to overwrite it\n")
 	assert.Contains(t, out, "This is your public key")
 }
diff --git a/client/go/cmd/cert_test.go b/client/go/cmd/cert_test.go
index cd5f88764b9..96b626b5c98 100644
--- a/client/go/cmd/cert_test.go
+++ b/client/go/cmd/cert_test.go
@@ -28,8 +28,8 @@ func TestCert(t *testing.T) {
 
 	assert.Equal(t, fmt.Sprintf("Success: Certificate written to %s\nSuccess: Certificate written to %s\nSuccess: Private key written to %s\n", pkgCertificate, certificate, privateKey), out)
 
-	out, _ = execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil)
-	assert.Contains(t, out, fmt.Sprintf("Error: Application package %s already contains a certificate", appDir))
+	_, outErr := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil)
+	assert.Contains(t, outErr, fmt.Sprintf("Error: Application package %s already contains a certificate", appDir))
 }
 
 func TestCertCompressedPackage(t *testing.T) {
@@ -41,13 +41,13 @@ func TestCertCompressedPackage(t *testing.T) {
 	_, err = os.Create(zipFile)
 	assert.Nil(t, err)
 
-	out, _ := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil)
-	assert.Contains(t, out, "Error: Cannot add certificate to compressed application package")
+	_, outErr := execute(command{args: []string{"cert", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil)
+	assert.Contains(t, outErr, "Error: Cannot add certificate to compressed application package")
 
 	err = os.Remove(zipFile)
 	assert.Nil(t, err)
 
-	out, _ = execute(command{args: []string{"cert", "-f", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil)
+	out, _ := execute(command{args: []string{"cert", "-f", "-a", "t1.a1.i1", pkgDir}, homeDir: homeDir}, t, nil)
 	assert.Contains(t, out, "Success: Certificate written to")
 	assert.Contains(t, out, "Success: Private key written to")
 }
diff --git a/client/go/cmd/command_tester.go b/client/go/cmd/command_tester.go
index 6929b59decb..8eaf6be2c22 100644
--- a/client/go/cmd/command_tester.go
+++ b/client/go/cmd/command_tester.go
@@ -27,6 +27,18 @@ type command struct {
 	moreArgs []string
 }
 
+func resetFlag(f *pflag.Flag) {
+	switch v := f.Value.(type) {
+	case pflag.SliceValue:
+		_ = v.Replace([]string{})
+	default:
+		switch v.Type() {
+		case "bool", "string", "int":
+			_ = v.Set(f.DefValue)
+		}
+	}
+}
+
 func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string) {
 	if client != nil {
 		util.ActiveHttpClient = client
@@ -44,17 +56,8 @@ func execute(cmd command, t *testing.T, client *mockHttpClient) (string, string)
 	os.Setenv("VESPA_CLI_CACHE_DIR", cmd.cacheDir)
 
 	// Reset flags to their default value - persistent flags in Cobra persists over tests
-	rootCmd.Flags().VisitAll(func(f *pflag.Flag) {
-		switch v := f.Value.(type) {
-		case pflag.SliceValue:
-			_ = v.Replace([]string{})
-		default:
-			switch v.Type() {
-			case "bool", "string", "int":
-				_ = v.Set(f.DefValue)
-			}
-		}
-	})
+	rootCmd.Flags().VisitAll(resetFlag)
+	documentCmd.Flags().VisitAll(resetFlag)
 
 	// Do not exit in tests
 	exitFunc = func(code int) {}
diff --git a/client/go/cmd/config_test.go b/client/go/cmd/config_test.go
index 223c8a396c3..7f7bdbb3edd 100644
--- a/client/go/cmd/config_test.go
+++ b/client/go/cmd/config_test.go
@@ -10,7 +10,7 @@ import (
 
 func TestConfig(t *testing.T) {
 	homeDir := filepath.Join(t.TempDir(), ".vespa")
-	assertConfigCommand(t, "invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar")
+	assertConfigCommandErr(t, "invalid option or value: \"foo\": \"bar\"\n", homeDir, "config", "set", "foo", "bar")
 	assertConfigCommand(t, "foo = <unset>\n", homeDir, "config", "get", "foo")
 	assertConfigCommand(t, "target = local\n", homeDir, "config", "get", "target")
 	assertConfigCommand(t, "", homeDir, "config", "set", "target", "cloud")
@@ -19,7 +19,7 @@ func TestConfig(t *testing.T) {
 	assertConfigCommand(t, "", homeDir, "config", "set", "target", "https://127.0.0.1")
 	assertConfigCommand(t, "target = https://127.0.0.1\n", homeDir, "config", "get", "target")
 
-	assertConfigCommand(t, "invalid application: \"foo\"\n", homeDir, "config", "set", "application", "foo")
+	assertConfigCommandErr(t, "invalid application: \"foo\"\n", homeDir, "config", "set", "application", "foo")
 	assertConfigCommand(t, "application = <unset>\n", homeDir, "config", "get", "application")
 	assertConfigCommand(t, "", homeDir, "config", "set", "application", "t1.a1.i1")
 	assertConfigCommand(t, "application = t1.a1.i1\n", homeDir, "config", "get", "application")
@@ -27,7 +27,7 @@ func TestConfig(t *testing.T) {
 	assertConfigCommand(t, "application = t1.a1.i1\ncolor = auto\ntarget = https://127.0.0.1\nwait = 0\n", homeDir, "config", "get")
 
 	assertConfigCommand(t, "", homeDir, "config", "set", "wait", "60")
-	assertConfigCommand(t, "wait option must be an integer >= 0, got \"foo\"\n", homeDir, "config", "set", "wait", "foo")
+	assertConfigCommandErr(t, "wait option must be an integer >= 0, got \"foo\"\n", homeDir, "config", "set", "wait", "foo")
 	assertConfigCommand(t, "wait = 60\n", homeDir, "config", "get", "wait")
 }
 
@@ -35,3 +35,8 @@ func assertConfigCommand(t *testing.T, expected, homeDir string, args ...string)
 	out, _ := execute(command{homeDir: homeDir, args: args}, t, nil)
 	assert.Equal(t, expected, out)
 }
+
+func assertConfigCommandErr(t *testing.T, expected, homeDir string, args ...string) {
+	_, outErr := execute(command{homeDir: homeDir, args: args}, t, nil)
+	assert.Equal(t, expected, outErr)
+}
diff --git a/client/go/cmd/deploy_test.go b/client/go/cmd/deploy_test.go
index 9614806b968..5bb45e70fad 100644
--- a/client/go/cmd/deploy_test.go
+++ b/client/go/cmd/deploy_test.go
@@ -61,9 +61,10 @@ func TestDeployApplicationDirectoryWithPomAndTarget(t *testing.T) {
 
 func TestDeployApplicationDirectoryWithPomAndEmptyTarget(t *testing.T) {
 	client := &mockHttpClient{}
+	_, outErr := execute(command{args: []string{"deploy", "testdata/applications/withEmptyTarget"}}, t, client)
 	assert.Equal(t,
 		"Error: pom.xml exists but no target/application.zip. Run mvn package first\n",
-		executeCommand(t, client, []string{"deploy", "testdata/applications/withEmptyTarget"}, []string{}))
+		outErr)
 }
 
 func TestDeployApplicationPackageErrorWithUnexpectedNonJson(t *testing.T) {
@@ -85,7 +86,7 @@ func TestDeployApplicationPackageErrorWithExpectedFormat(t *testing.T) {
 		"Invalid XML, error in services.xml:\nelement \"nosuch\" not allowed here",
 		`{
          "error-code": "INVALID_APPLICATION_PACKAGE",
-         "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here\n"
+         "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here"
      }`)
 }
 
@@ -94,7 +95,7 @@ func TestPrepareApplicationPackageErrorWithExpectedFormat(t *testing.T) {
 		"Invalid XML, error in services.xml:\nelement \"nosuch\" not allowed here",
 		`{
          "error-code": "INVALID_APPLICATION_PACKAGE",
-         "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here\n"
+         "message": "Invalid XML, error in services.xml: element \"nosuch\" not allowed here"
      }`)
 }
 
@@ -158,18 +159,20 @@ func assertDeployRequestMade(target string, client *mockHttpClient, t *testing.T
 	assertPackageUpload(-1, target+"/application/v2/tenant/default/prepareandactivate", client, t)
 }
 
-func assertApplicationPackageError(t *testing.T, command string, status int, expectedMessage string, returnBody string) {
+func assertApplicationPackageError(t *testing.T, cmd string, status int, expectedMessage string, returnBody string) {
 	client := &mockHttpClient{}
 	client.NextResponse(status, returnBody)
+	_, outErr := execute(command{args: []string{cmd, "testdata/applications/withTarget/target/application.zip"}}, t, client)
 	assert.Equal(t,
 		"Error: Invalid application package (Status "+strconv.Itoa(status)+")\n\n"+expectedMessage+"\n",
-		executeCommand(t, client, []string{command, "testdata/applications/withTarget/target/application.zip"}, []string{}))
+		outErr)
 }
 
 func assertDeployServerError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{}
 	client.NextResponse(status, errorMessage)
+	_, outErr := execute(command{args: []string{"deploy", "testdata/applications/withTarget/target/application.zip"}}, t, client)
 	assert.Equal(t,
 		"Error: Error from deploy service at 127.0.0.1:19071 (Status "+strconv.Itoa(status)+"):\n"+errorMessage+"\n",
-		executeCommand(t, client, []string{"deploy", "testdata/applications/withTarget/target/application.zip"}, []string{}))
+		outErr)
 }
diff --git a/client/go/cmd/document.go b/client/go/cmd/document.go
index cc5fb948e3b..7f2c9b8a916 100644
--- a/client/go/cmd/document.go
+++ b/client/go/cmd/document.go
@@ -5,9 +5,9 @@
 package cmd
 
 import (
+	"fmt"
 	"io"
 	"io/ioutil"
-	"log"
 	"strings"
 
 	"github.com/spf13/cobra"
@@ -123,20 +123,25 @@ func curlOutput() io.Writer {
 }
 
 func printResult(result util.OperationResult, payloadOnlyOnSuccess bool) {
+	out := stdout
 	if !result.Success {
-		log.Print(color.Red("Error: "), result.Message)
+		out = stderr
+	}
+
+	if !result.Success {
+		fmt.Fprintln(out, color.Red("Error:"), result.Message)
 	} else if !(payloadOnlyOnSuccess && result.Payload != "") {
-		log.Print(color.Green("Success: "), result.Message)
+		fmt.Fprintln(out, color.Green("Success:"), result.Message)
 	}
 
 	if result.Detail != "" {
-		log.Print(color.Yellow(result.Detail))
+		fmt.Fprintln(out, color.Yellow(result.Detail))
 	}
 
 	if result.Payload != "" {
 		if !payloadOnlyOnSuccess {
-			log.Println("")
+			fmt.Fprintln(out)
 		}
-		log.Print(result.Payload)
+		fmt.Fprintln(out, result.Payload)
 	}
 }
diff --git a/client/go/cmd/document_test.go b/client/go/cmd/document_test.go
index 1f82b85f915..649aca8703a 100644
--- a/client/go/cmd/document_test.go
+++ b/client/go/cmd/document_test.go
@@ -67,17 +67,19 @@ func TestDocumentRemoveWithoutIdArg(t *testing.T) {
 func TestDocumentSendMissingId(t *testing.T) {
 	arguments := []string{"document", "put", "testdata/A-Head-Full-of-Dreams-Without-Operation.json"}
 	client := &mockHttpClient{}
+	_, outErr := execute(command{args: arguments}, t, client)
 	assert.Equal(t,
 		"Error: No document id given neither as argument or as a 'put' key in the json file\n",
-		executeCommand(t, client, arguments, []string{}))
+		outErr)
 }
 
 func TestDocumentSendWithDisagreeingOperations(t *testing.T) {
 	arguments := []string{"document", "update", "testdata/A-Head-Full-of-Dreams-Put.json"}
 	client := &mockHttpClient{}
+	_, outErr := execute(command{args: arguments}, t, client)
 	assert.Equal(t,
 		"Error: Wanted document operation is update but the JSON file specifies put\n",
-		executeCommand(t, client, arguments, []string{}))
+		outErr)
 }
 
 func TestDocumentPutDocumentError(t *testing.T) {
@@ -139,21 +141,23 @@ func assertDocumentGet(arguments []string, documentId string, t *testing.T) {
 func assertDocumentError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{}
 	client.NextResponse(status, errorMessage)
+	_, outErr := execute(command{args: []string{"document", "put",
+		"id:mynamespace:music::a-head-full-of-dreams",
+		"testdata/A-Head-Full-of-Dreams-Put.json"}}, t, client)
 	assert.Equal(t,
 		"Error: Invalid document operation: Status "+strconv.Itoa(status)+"\n\n"+errorMessage+"\n",
-		executeCommand(t, client, []string{"document", "put",
-			"id:mynamespace:music::a-head-full-of-dreams",
-			"testdata/A-Head-Full-of-Dreams-Put.json"}, []string{}))
+		outErr)
 }
 
 func assertDocumentServerError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{}
 	client.NextResponse(status, errorMessage)
+	_, outErr := execute(command{args: []string{"document", "put",
+		"id:mynamespace:music::a-head-full-of-dreams",
+		"testdata/A-Head-Full-of-Dreams-Put.json"}}, t, client)
 	assert.Equal(t,
 		"Error: Container (document API) at 127.0.0.1:8080: Status "+strconv.Itoa(status)+"\n\n"+errorMessage+"\n",
-		executeCommand(t, client, []string{"document", "put",
-			"id:mynamespace:music::a-head-full-of-dreams",
-			"testdata/A-Head-Full-of-Dreams-Put.json"}, []string{}))
+		outErr)
 }
 
 func documentServiceURL(client *mockHttpClient) string {
diff --git a/client/go/cmd/helpers.go b/client/go/cmd/helpers.go
index 98d6814d16f..b5525cf11fe 100644
--- a/client/go/cmd/helpers.go
+++ b/client/go/cmd/helpers.go
@@ -32,16 +32,16 @@ func fatalErr(err error, msg ...interface{}) {
 func printErrHint(err error, hints ...string) {
 	printErr(nil, err.Error())
 	for _, hint := range hints {
-		log.Print(color.Cyan("Hint: "), hint)
+		fmt.Fprintln(stderr, color.Cyan("Hint:"), hint)
 	}
 }
 
 func printErr(err error, msg ...interface{}) {
 	if len(msg) > 0 {
-		log.Print(color.Red("Error: "), fmt.Sprint(msg...))
+		fmt.Fprintln(stderr, color.Red("Error:"), fmt.Sprint(msg...))
 	}
 	if err != nil {
-		log.Print(color.Yellow(err))
+		fmt.Fprintln(stderr, color.Yellow(err))
 	}
 }
 
@@ -215,11 +215,9 @@ func waitForService(service string, sessionOrRunID int64) {
 	if status/100 == 2 {
 		log.Print(s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Green("ready"))
 	} else {
-		log.Print(s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Red("not ready"))
 		if err == nil {
-			log.Print(color.Yellow(fmt.Sprintf("Status %d", status)))
-		} else {
-			log.Print(color.Yellow(err))
+			err = fmt.Errorf("Status %d", status)
 		}
+		fatalErr(err, s.Description(), " at ", color.Cyan(s.BaseURL), " is ", color.Red("not ready"))
 	}
 }
diff --git a/client/go/cmd/log_test.go b/client/go/cmd/log_test.go
index 7808d429e4f..f239bebc488 100644
--- a/client/go/cmd/log_test.go
+++ b/client/go/cmd/log_test.go
@@ -23,6 +23,6 @@ func TestLog(t *testing.T) {
 	expected := "[2021-09-27 10:31:30.905535] host1a.dev.aws-us-east-1c info    logserver-container Container.com.yahoo.container.jdisc.ConfiguredApplication	Switching to the latest deployed set of configurations and components. Application config generation: 52532\n"
 	assert.Equal(t, expected, out)
 
-	out, _ = execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T13:12:49Z", "--to", "2021-09-27T13:15:00", "1h"}}, t, httpClient)
-	assert.Equal(t, "Error: Invalid period\ncannot combine --from/--to with relative value: 1h\n", out)
+	_, errOut := execute(command{homeDir: homeDir, args: []string{"log", "--from", "2021-09-27T13:12:49Z", "--to", "2021-09-27T13:15:00", "1h"}}, t, httpClient)
+	assert.Equal(t, "Error: Invalid period\ncannot combine --from/--to with relative value: 1h\n", errOut)
 }
diff --git a/client/go/cmd/query.go b/client/go/cmd/query.go
index 7de0addb9b8..5e2b268865d 100644
--- a/client/go/cmd/query.go
+++ b/client/go/cmd/query.go
@@ -47,7 +47,7 @@ func query(arguments []string) {
 
 	response, err := service.Do(&http.Request{URL: url}, time.Second*10)
 	if err != nil {
-		log.Print(color.Red("Error: "), "Request failed: ", err)
+		printErr(nil, "Request failed: ", err)
 		return
 	}
 	defer response.Body.Close()
@@ -55,11 +55,9 @@ func query(arguments []string) {
 	if response.StatusCode == 200 {
 		log.Print(util.ReaderToJSON(response.Body))
 	} else if response.StatusCode/100 == 4 {
-		log.Print(color.Red("Error: "), "Invalid query: ", response.Status, "\n")
-		log.Print(util.ReaderToJSON(response.Body))
+		printErr(nil, "Invalid query: ", response.Status, "\n", util.ReaderToJSON(response.Body))
 	} else {
-		log.Print(color.Red("Error: "), response.Status, " from container at ", color.Cyan(url.Host), "\n")
-		log.Print(util.ReaderToJSON(response.Body))
+		printErr(nil, response.Status, " from container at ", color.Cyan(url.Host), "\n", util.ReaderToJSON(response.Body))
 	}
 }
 
diff --git a/client/go/cmd/query_test.go b/client/go/cmd/query_test.go
index b4ae63d777f..81dc03766be 100644
--- a/client/go/cmd/query_test.go
+++ b/client/go/cmd/query_test.go
@@ -57,18 +57,20 @@ func assertQuery(t *testing.T, expectedQuery string, query ...string) {
 func assertQueryError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{}
 	client.NextResponse(status, errorMessage)
+	_, outErr := execute(command{args: []string{"query", "yql=select from sources * where title contains 'foo'"}}, t, client)
 	assert.Equal(t,
 		"Error: Invalid query: Status "+strconv.Itoa(status)+"\n"+errorMessage+"\n",
-		executeCommand(t, client, []string{"query"}, []string{"yql=select from sources * where title contains 'foo'"}),
+		outErr,
 		"error output")
 }
 
 func assertQueryServiceError(t *testing.T, status int, errorMessage string) {
 	client := &mockHttpClient{}
 	client.NextResponse(status, errorMessage)
+	_, outErr := execute(command{args: []string{"query", "yql=select from sources * where title contains 'foo'"}}, t, client)
 	assert.Equal(t,
 		"Error: Status "+strconv.Itoa(status)+" from container at 127.0.0.1:8080\n"+errorMessage+"\n",
-		executeCommand(t, client, []string{"query"}, []string{"yql=select from sources * where title contains 'foo'"}),
+		outErr,
 		"error output")
 }
 
diff --git a/client/go/cmd/status_test.go b/client/go/cmd/status_test.go
index fc2ba3d4586..757ef5f3b06 100644
--- a/client/go/cmd/status_test.go
+++ b/client/go/cmd/status_test.go
@@ -78,8 +78,11 @@ func assertDocumentStatus(target string, args []string, t *testing.T) {
 func assertQueryStatusError(target string, args []string, t *testing.T) {
 	client := &mockHttpClient{}
 	client.NextStatus(500)
+	cmd := []string{"status", "container"}
+	cmd = append(cmd, args...)
+	_, outErr := execute(command{args: cmd}, t, client)
 	assert.Equal(t,
-		"Container (query API) at "+target+" is not ready\nStatus 500\n",
-		executeCommand(t, client, []string{"status", "container"}, args),
+		"Error: Container (query API) at "+target+" is not ready\nStatus 500\n",
+		outErr,
 		"vespa status container")
 }