Remove leaking logs from SDK

commands/deploy.go

@@ -276,9 +276,15 @@ Error: %s`, fprocessErr.Error())
 			if msg := checkTLSInsecure(services.Provider.GatewayURL, deploySpec.TLSInsecure); len(msg) > 0 {
 				fmt.Println(msg)
 			}
-			statusCode := proxyClient.DeployFunction(ctx, deploySpec)
-			if badStatusCode(statusCode) {
-				failedStatusCodes[k] = statusCode
+			dRes, res, err := proxyClient.DeployFunction(ctx, deploySpec)
+
+			if err == nil {
+				fmt.Println(dRes.Message)
+				fmt.Printf("URL: %s\n\n", dRes.URL)
+			}
+
+			if badStatusCode(res.StatusCode) {
+				failedStatusCodes[k] = res.StatusCode
 			}
 		}
 	} else {
@@ -375,9 +381,14 @@ func deployImage(
 		fmt.Println(msg)
 	}
 
-	statusCode = client.DeployFunction(ctx, deploySpec)
+	dRes, res, err := client.DeployFunction(ctx, deploySpec)
+	if err != nil {
+		return res.StatusCode, err
+	}
 
-	return statusCode, nil
+	fmt.Println(dRes.Message)
+	fmt.Printf("URL: %s\n\n", dRes.URL)
+	return res.StatusCode, nil
 }
 
 func mergeSlice(values []string, overlay []string) []string {

commands/describe.go

@@ -82,9 +82,9 @@ func runDescribe(cmd *cobra.Command, args []string) error {
 	}
 
 	//To get correct value for invocation count from /system/functions endpoint
-	functionList, err := cliClient.ListFunctions(ctx, functionNamespace)
+	functionList, _, err := cliClient.ListFunctions(ctx, functionNamespace)
 	if err != nil {
-		return err
+		return actionableErrorMessage(err)
 	}
 
 	var invocationCount int

commands/errors.go

@@ -4,7 +4,10 @@
 package commands
 
 import (
+	"fmt"
 	"strings"
+
+	"github.com/openfaas/faas-cli/proxy"
 )
 
 const (
@@ -24,3 +27,13 @@ func checkTLSInsecure(gateway string, tlsInsecure bool) string {
 	}
 	return ""
 }
+
+//actionableErrorMessage print actionable error message based on APIError check
+func actionableErrorMessage(err error) error {
+	if proxy.IsUnknown(err) {
+		return fmt.Errorf("server returned unexpected status response: %s", err.Error())
+	} else if proxy.IsUnauthorized(err) {
+		return fmt.Errorf("unauthorized access, run \"faas-cli login\" to setup authentication for this server")
+	}
+	return err
+}

commands/list.go

@@ -73,9 +73,9 @@ func runList(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	functions, err := proxyClient.ListFunctions(context.Background(), functionNamespace)
+	functions, _, err := proxyClient.ListFunctions(context.Background(), functionNamespace)
 	if err != nil {
-		return err
+		return actionableErrorMessage(err)
 	}
 
 	if sortOrder == "name" {

proxy/client.go

@@ -1,9 +1,12 @@
 package proxy
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
+	"io/ioutil"
 	"net/http"
 	"net/http/httputil"
 	"net/url"
@@ -129,3 +132,30 @@ func addQueryParams(u string, params map[string]string) (string, error) {
 func (c *Client) AddCheckRedirect(checkRedirect func(*http.Request, []*http.Request) error) {
 	c.httpClient.CheckRedirect = checkRedirect
 }
+
+// parseResponse function parses HTTP response body into a typed struct
+// or copies to io.Writer object. If it fails to decode response body
+// then it re-construct it so that it can be read later
+func parseResponse(res *http.Response, v interface{}) error {
+	var err error
+	defer res.Body.Close()
+
+	switch v := v.(type) {
+	case nil:
+	case io.Writer:
+		_, err = io.Copy(v, res.Body)
+	default:
+		data, err := ioutil.ReadAll(res.Body)
+		if err == io.EOF {
+			err = nil // ignore EOF errors caused by empty response body
+		}
+
+		decErr := json.Unmarshal(data, v)
+		if decErr != nil {
+			err = decErr
+			// In case of JSON decode error, re-construct response body
+			res.Body = ioutil.NopCloser(bytes.NewBuffer(data))
+		}
+	}
+	return err
+}

proxy/deploy.go

@@ -8,7 +8,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"io/ioutil"
 	"net/http"
 	"time"
 
@@ -57,29 +56,48 @@ func generateFuncStr(spec *DeployFunctionSpec) string {
 	return spec.FunctionName
 }
 
+type DeployResponse struct {
+	Message       string
+	RollingUpdate bool
+	URL           string
+}
+
 // DeployFunction first tries to deploy a function and if it exists will then attempt
 // a rolling update. Warnings are suppressed for the second API call (if required.)
-func (c *Client) DeployFunction(context context.Context, spec *DeployFunctionSpec) int {
+func (c *Client) DeployFunction(context context.Context, spec *DeployFunctionSpec) (*DeployResponse, *http.Response, error) {
 
 	rollingUpdateInfo := fmt.Sprintf("Function %s already exists, attempting rolling-update.", spec.FunctionName)
-	statusCode, deployOutput := c.deploy(context, spec, spec.Update)
+	res, err := c.deploy(context, spec, spec.Update)
+
+	if err != nil && IsUnknown(err) {
+		return nil, res, err
+	}
+
+	var deployResponse DeployResponse
+	if err == nil {
+		deployResponse.Message = fmt.Sprintf("Deployed. %s.", res.Status)
+		deployResponse.URL = fmt.Sprintf("%s/function/%s", c.GatewayURL.String(), generateFuncStr(spec))
+	}
 
-	if spec.Update == true && statusCode == http.StatusNotFound {
+	if spec.Update == true && IsNotFound(err) {
 		// Re-run the function with update=false
+		res, err = c.deploy(context, spec, false)
+		if err == nil {
+			deployResponse.Message = fmt.Sprintf("Deployed. %s.", res.Status)
+			deployResponse.URL = fmt.Sprintf("%s/function/%s", c.GatewayURL.String(), generateFuncStr(spec))
+		}
+
+	} else if res.StatusCode == http.StatusOK {
+		deployResponse.Message += rollingUpdateInfo
+		deployResponse.RollingUpdate = true
 
-		statusCode, deployOutput = c.deploy(context, spec, false)
-	} else if statusCode == http.StatusOK {
-		fmt.Println(rollingUpdateInfo)
 	}
-	fmt.Println()
-	fmt.Println(deployOutput)
-	return statusCode
+
+	return &deployResponse, res, err
 }
 
 // deploy a function to an OpenFaaS gateway over REST
-func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, update bool) (int, string) {
-
-	var deployOutput string
+func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, update bool) (*http.Response, error) {
 	// Need to alter Gateway to allow nil/empty string as fprocess, to avoid this repetition.
 	var fprocessTemplate string
 	if len(spec.FProcess) > 0 {
@@ -146,37 +164,20 @@ func (c *Client) deploy(context context.Context, spec *DeployFunctionSpec, updat
 	request, err = c.newRequest(method, "/system/functions", reader)
 
 	if err != nil {
-		deployOutput += fmt.Sprintln(err)
-		return http.StatusInternalServerError, deployOutput
+		return nil, err
 	}
 
 	res, err := c.doRequest(context, request)
 
 	if err != nil {
-		deployOutput += fmt.Sprintln("Is OpenFaaS deployed? Do you need to specify the --gateway flag?")
-		deployOutput += fmt.Sprintln(err)
-		return http.StatusInternalServerError, deployOutput
-	}
-
-	if res.Body != nil {
-		defer res.Body.Close()
+		errMessage := fmt.Sprintln("Is OpenFaaS deployed? Do you need to specify the --gateway flag?")
+		errMessage += fmt.Sprintln(err)
+		return res, NewUnknown(errMessage, 0)
 	}
 
-	switch res.StatusCode {
-	case http.StatusOK, http.StatusCreated, http.StatusAccepted:
-		deployOutput += fmt.Sprintf("Deployed. %s.\n", res.Status)
-
-		deployedURL := fmt.Sprintf("URL: %s/function/%s", c.GatewayURL.String(), generateFuncStr(spec))
-		deployOutput += fmt.Sprintln(deployedURL)
-	case http.StatusUnauthorized:
-		deployOutput += fmt.Sprintln("unauthorized access, run \"faas-cli login\" to setup authentication for this server")
-
-	default:
-		bytesOut, err := ioutil.ReadAll(res.Body)
-		if err == nil {
-			deployOutput += fmt.Sprintf("Unexpected status: %d, message: %s\n", res.StatusCode, string(bytesOut))
-		}
+	err = checkForAPIError(res)
+	if err != nil {
+		return res, err
 	}
-
-	return res.StatusCode, deployOutput
+	return res, nil
 }

proxy/deploy_test.go

@@ -21,7 +21,8 @@ type deployProxyTest struct {
 	mockServerResponses []int
 	replace             bool
 	update              bool
-	expectedOutput      string
+	expectedMessage     string
+	statusCode          int
 }
 
 func runDeployProxyTest(t *testing.T, deployTest deployProxyTest) {
@@ -34,32 +35,34 @@ func runDeployProxyTest(t *testing.T, deployTest deployProxyTest) {
 	cliAuth := NewTestAuth(nil)
 	proxyClient, _ := NewClient(cliAuth, s.URL, nil, &defaultCommandTimeout)
 
-	stdout := test.CaptureStdout(func() {
-		proxyClient.DeployFunction(context.TODO(), &DeployFunctionSpec{
-			"fprocess",
-			"function",
-			"image",
-			"dXNlcjpwYXNzd29yZA==",
-			"language",
-			deployTest.replace,
-			nil,
-			"network",
-			[]string{},
-			deployTest.update,
-			[]string{},
-			map[string]string{},
-			map[string]string{},
-			FunctionResourceRequest{},
-			false,
-			tlsNoVerify,
-			"",
-			"",
-		})
+	dRes, httpRes, _ := proxyClient.DeployFunction(context.TODO(), &DeployFunctionSpec{
+		"fprocess",
+		"function",
+		"image",
+		"dXNlcjpwYXNzd29yZA==",
+		"language",
+		deployTest.replace,
+		nil,
+		"network",
+		[]string{},
+		deployTest.update,
+		[]string{},
+		map[string]string{},
+		map[string]string{},
+		FunctionResourceRequest{},
+		false,
+		tlsNoVerify,
+		"",
+		"",
 	})
 
-	r := regexp.MustCompile(deployTest.expectedOutput)
-	if !r.MatchString(stdout) {
-		t.Fatalf("Output not matched: %s", stdout)
+	if httpRes.StatusCode != deployTest.statusCode {
+		t.Fatalf("StatuCode did not match. expected: %d, got: %d", deployTest.statusCode, httpRes.StatusCode)
+	}
+
+	r := regexp.MustCompile(deployTest.expectedMessage)
+	if !r.MatchString(dRes.Message) {
+		t.Fatalf("Output not matched: %s", dRes.Message)
 	}
 }
 
@@ -70,21 +73,24 @@ func Test_RunDeployProxyTests(t *testing.T) {
 			mockServerResponses: []int{http.StatusOK, http.StatusOK},
 			replace:             true,
 			update:              false,
-			expectedOutput:      `(?m:Deployed)`,
+			statusCode:          http.StatusOK,
+			expectedMessage:     `(?m:Deployed)`,
 		},
 		{
 			title:               "404_Deploy",
 			mockServerResponses: []int{http.StatusOK, http.StatusNotFound},
 			replace:             true,
 			update:              false,
-			expectedOutput:      `(?m:Unexpected status: 404)`,
+			statusCode:          http.StatusNotFound,
+			expectedMessage:     "",
 		},
 		{
 			title:               "UpdateFailedDeployed",
 			mockServerResponses: []int{http.StatusNotFound, http.StatusOK},
 			replace:             false,
 			update:              true,
-			expectedOutput:      `(?m:Deployed)`,
+			statusCode:          http.StatusOK,
+			expectedMessage:     `(?m:Deployed)`,
 		},
 	}
 	for _, tst := range deployProxyTests {