Get rid of modifiers in the clierror package (#2092)

* implement own clierror interface

* get rid of modifiers

internal/btp/auth/credentials.go

@@ -46,16 +46,16 @@ type UAA struct {
 	ZoneID            string `json:"zoneid"`
 }
 
-func LoadCISCredentials(path string) (*CISCredentials, error) {
+func LoadCISCredentials(path string) (*CISCredentials, clierror.Error) {
 	credentialsBytes, err := os.ReadFile(path)
 	if err != nil {
-		return nil, clierror.Wrap(err, clierror.Message("failed to read credentials file"), clierror.Hints("Make sure the path to the credentials file is correct."))
+		return nil, clierror.Wrap(err, clierror.New("failed to read credentials file", "Make sure the path to the credentials file is correct."))
 	}
 
 	credentials := CISCredentials{}
 	err = json.Unmarshal(credentialsBytes, &credentials)
 	if err != nil {
-		return nil, clierror.Wrap(err, clierror.Message("failed to unmarshal file data"), clierror.Hints("Make sure the credentials file is in the correct format."))
+		return nil, clierror.Wrap(err, clierror.New("failed to unmarshal file data", "Make sure the credentials file is in the correct format."))
 	}
 
 	return &credentials, nil

internal/btp/auth/credentials_test.go

@@ -40,8 +40,8 @@ func TestLoadCISCredentials(t *testing.T) {
 		err := os.WriteFile(filename, []byte(correctCredentials), os.ModePerm)
 		require.NoError(t, err)
 
-		credentials, err := LoadCISCredentials(filename)
-		require.Nil(t, err)
+		credentials, cliError := LoadCISCredentials(filename)
+		require.Nil(t, cliError)
 		require.Equal(t, correctCredentialsStruct, *credentials)
 	})
 
@@ -51,16 +51,16 @@ func TestLoadCISCredentials(t *testing.T) {
 		err := os.WriteFile(filename, []byte("\n{\n"), os.ModePerm)
 		require.NoError(t, err)
 
-		credentials, err := LoadCISCredentials(filename)
-		require.Error(t, err)
+		credentials, cliError := LoadCISCredentials(filename)
+		require.NotNil(t, cliError)
 		require.Nil(t, credentials)
 	})
 
 	t.Run("incorrect credentials file error", func(t *testing.T) {
 		filename := fmt.Sprintf("%s/doesnotexist-creds.txt", tmpDirPath)
 
-		credentials, err := LoadCISCredentials(filename)
-		require.Error(t, err)
+		credentials, cliError := LoadCISCredentials(filename)
+		require.NotNil(t, cliError)
 		require.Nil(t, credentials)
 	})
 }

internal/btp/auth/xsuaa.go

@@ -26,7 +26,7 @@ type XSUAAToken struct {
 	JTI         string `json:"jti"`
 }
 
-func GetOAuthToken(grantType, serverURL, username, password string) (*XSUAAToken, error) {
+func GetOAuthToken(grantType, serverURL, username, password string) (*XSUAAToken, clierror.Error) {
 	urlBody := url.Values{}
 	urlBody.Set("grant_type", grantType)
 
@@ -36,7 +36,7 @@ func GetOAuthToken(grantType, serverURL, username, password string) (*XSUAAToken
 		strings.NewReader(urlBody.Encode()),
 	)
 	if err != nil {
-		return nil, clierror.Wrap(err, clierror.Message("failed to build request"), clierror.Hints("Make sure the server URL in the config is correct."))
+		return nil, clierror.Wrap(err, clierror.New("failed to build request", "Make sure the server URL in the config is correct."))
 	}
 	defer request.Body.Close()
 
@@ -45,7 +45,7 @@ func GetOAuthToken(grantType, serverURL, username, password string) (*XSUAAToken
 
 	response, err := http.DefaultClient.Do(request)
 	if err != nil {
-		return nil, clierror.Wrap(err, clierror.Message("failed to get token from server"))
+		return nil, clierror.Wrap(err, clierror.New("failed to get token from server"))
 	}
 	defer response.Body.Close()
 
@@ -56,22 +56,24 @@ func GetOAuthToken(grantType, serverURL, username, password string) (*XSUAAToken
 	return decodeAuthSuccessResponse(response)
 }
 
-func decodeAuthSuccessResponse(response *http.Response) (*XSUAAToken, error) {
+func decodeAuthSuccessResponse(response *http.Response) (*XSUAAToken, clierror.Error) {
 	token := XSUAAToken{}
 	err := json.NewDecoder(response.Body).Decode(&token)
 	if err != nil {
-		return nil, clierror.Wrap(err, clierror.MessageF("failed to decode response with Status %s", response.Status))
+		errMsg := fmt.Sprintf("failed to decode response with Status %s", response.Status)
+		return nil, clierror.Wrap(err, clierror.New(errMsg))
 	}
 
 	return &token, nil
 }
 
-func decodeAuthErrorResponse(response *http.Response) error {
+func decodeAuthErrorResponse(response *http.Response) clierror.Error {
 	errorData := xsuaaErrorResponse{}
 	err := json.NewDecoder(response.Body).Decode(&errorData)
 	if err != nil {
-		return clierror.Wrap(err, clierror.Message("failed to decode error response"))
+		return clierror.Wrap(err, clierror.New("failed to decode error response"))
 	}
-	// TODO: replace it with New func
-	return clierror.Wrap(errors.New(errorData.ErrorDescription), clierror.MessageF("error response: %s", response.Status))
+
+	errMsg := fmt.Sprintf("error response: %s", response.Status)
+	return clierror.Wrap(errors.New(errorData.ErrorDescription), clierror.New(errMsg))
 }

internal/btp/auth/xsuaa_test.go

@@ -31,7 +31,7 @@ func TestGetOAuthToken(t *testing.T) {
 		name        string
 		credentials *CISCredentials
 		want        *XSUAAToken
-		expectedErr error
+		expectedErr clierror.Error
 	}{
 		{
 			name: "Correct credentials",
@@ -57,8 +57,7 @@ func TestGetOAuthToken(t *testing.T) {
 			want: nil,
 			expectedErr: clierror.Wrap(
 				errors.New("parse \"?\\n?/oauth/token\": net/url: invalid control character in URL"),
-				clierror.Message("failed to build request"),
-				clierror.Hints("Make sure the server URL in the config is correct."),
+				clierror.New("failed to build request", "Make sure the server URL in the config is correct."),
 			),
 		},
 		{
@@ -71,7 +70,7 @@ func TestGetOAuthToken(t *testing.T) {
 			want: nil,
 			expectedErr: clierror.Wrap(
 				errors.New("Post \"http://doesnotexist/oauth/token\": dial tcp: lookup doesnotexist: no such host"),
-				clierror.Message("failed to get token from server"),
+				clierror.New("failed to get token from server"),
 			),
 		},
 		{
@@ -84,7 +83,7 @@ func TestGetOAuthToken(t *testing.T) {
 			want: nil,
 			expectedErr: clierror.Wrap(
 				errors.New("description"),
-				clierror.Message("error response: 401 Unauthorized"),
+				clierror.New("error response: 401 Unauthorized"),
 			),
 		},
 	}

internal/btp/cis/provision.go

@@ -56,10 +56,10 @@ type ProvisionResponse struct {
 	PlanName        string `json:"planName"`
 }
 
-func (c *LocalClient) Provision(pe *ProvisionEnvironment) (*ProvisionResponse, error) {
+func (c *LocalClient) Provision(pe *ProvisionEnvironment) (*ProvisionResponse, clierror.Error) {
 	reqData, err := json.Marshal(pe)
 	if err != nil {
-		return nil, clierror.New(clierror.Message(err.Error()))
+		return nil, clierror.New(err.Error())
 	}
 
 	provisionURL := fmt.Sprintf("%s/%s", c.credentials.Endpoints.ProvisioningServiceURL, provisionEndpoint)
@@ -84,18 +84,18 @@ func (c *LocalClient) Provision(pe *ProvisionEnvironment) (*ProvisionResponse, e
 			hints = append(hints, "check if subaccount have enabled Kyma entitlement with correct plan")
 		}
 
-		return nil, clierror.New(clierror.Message(err.Error()), clierror.Hints(hints...))
+		return nil, clierror.New(err.Error(), hints...)
 	}
 	defer response.Body.Close()
 
 	return decodeProvisionSuccessResponse(response)
 }
 
-func decodeProvisionSuccessResponse(response *http.Response) (*ProvisionResponse, error) {
+func decodeProvisionSuccessResponse(response *http.Response) (*ProvisionResponse, clierror.Error) {
 	provisionResponse := ProvisionResponse{}
 	err := json.NewDecoder(response.Body).Decode(&provisionResponse)
 	if err != nil {
-		return nil, clierror.Wrap(err, clierror.Message("failed to decode response"))
+		return nil, clierror.Wrap(err, clierror.New("failed to decode response"))
 	}
 
 	return &provisionResponse, nil

internal/btp/cis/provision_test.go

@@ -65,7 +65,7 @@ func TestCISClient_Provision(t *testing.T) {
 		token          *auth.XSUAAToken
 		pe             *ProvisionEnvironment
 		wantedResponse *ProvisionResponse
-		expectedErr    error
+		expectedErr    clierror.Error
 	}{
 		{
 			name: "Correct data",
@@ -95,7 +95,7 @@ func TestCISClient_Provision(t *testing.T) {
 				Name: "name",
 			},
 			wantedResponse: nil,
-			expectedErr:    clierror.New(clierror.Message("failed to build request: parse \"?\\n?/provisioning/v1/environments\": net/url: invalid control character in URL")),
+			expectedErr:    clierror.New("failed to build request: parse \"?\\n?/provisioning/v1/environments\": net/url: invalid control character in URL"),
 		},
 		{
 			name: "Wrong URL",
@@ -109,7 +109,7 @@ func TestCISClient_Provision(t *testing.T) {
 				Name: "name",
 			},
 			wantedResponse: nil,
-			expectedErr:    clierror.New(clierror.Message("failed to get data from server: Post \"http://doesnotexist/provisioning/v1/environments\": dial tcp: lookup doesnotexist: no such host")),
+			expectedErr:    clierror.New("failed to get data from server: Post \"http://doesnotexist/provisioning/v1/environments\": dial tcp: lookup doesnotexist: no such host"),
 		},
 		{
 			name: "Error response",
@@ -123,7 +123,7 @@ func TestCISClient_Provision(t *testing.T) {
 				Name: "name",
 			},
 			wantedResponse: nil,
-			expectedErr:    clierror.New(clierror.Message("error")),
+			expectedErr:    clierror.New("error"),
 		},
 	}
 	for _, tt := range tests {

internal/clierror/check.go

@@ -0,0 +1,14 @@
+package clierror
+
+import (
+	"fmt"
+	"os"
+)
+
+// Check prints error and executes os.Exit(1) if the error is not nil
+func Check(err Error) {
+	if err != nil {
+		fmt.Println(err.String())
+		os.Exit(1)
+	}
+}

internal/clierror/error.go

@@ -4,23 +4,30 @@ import (
 	"fmt"
 )
 
+type Error interface {
+	String() string
+}
+
 type clierror struct {
 	message string
 	details string
 	hints   []string
 }
 
 // New creates a new error with the given modifiers
-func New(modifiers ...modifier) *clierror {
-	err := &clierror{}
-	for _, m := range modifiers {
-		m(err)
+func New(message string, hints ...string) Error {
+	return new(message, hints...)
+}
+
+func new(message string, hints ...string) *clierror {
+	return &clierror{
+		message: message,
+		hints:   hints,
 	}
-	return err
 }
 
 // Error returns the error string, compatible with the error interface
-func (e clierror) Error() string {
+func (e *clierror) String() string {
 	output := fmt.Sprintf("Error:\n  %s\n\n", e.message)
 	if e.details != "" {
 		output += fmt.Sprintf("Error Details:\n  %s\n\n", e.details)

internal/clierror/error_test.go

@@ -44,7 +44,7 @@ func Test_CLIError_String(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			assert.Equal(t, tt.want, tt.err.Error())
+			assert.Equal(t, tt.want, tt.err.String())
 		})
 	}
 }
@@ -157,7 +157,7 @@ func Test_CLIError_Wrap(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			err := tt.err.wrap(tt.outside)
-			assert.Equal(t, tt.want, err.Error())
+			assert.Equal(t, tt.want, err.String())
 		})
 	}
 }

internal/clierror/wrap.go

@@ -1,9 +1,13 @@
 package clierror
 
-func Wrap(inside error, modifiers ...modifier) error {
-	if err, ok := inside.(*clierror); ok {
-		return err.wrap(New(modifiers...))
-	}
+import "fmt"
 
-	return New(Message(inside.Error())).wrap(New(modifiers...))
+// Wrap allows to cover and error with additional information
+func Wrap(inside error, outside Error) Error {
+	return WrapE(new(fmt.Sprintf("%v", inside)), outside)
+}
+
+// WrapE allows to cover clierror with additional information
+func WrapE(inside, outside Error) Error {
+	return inside.(*clierror).wrap(outside.(*clierror))
 }