Merge pull request #81 from neo4j/improve-errors

Improve error generation using helpers
neo4j · Oct 31, 2024 · 60910b1 · 60910b1
2 parents 7664aa4 + 2c9d125
commit 60910b1
Show file tree

Hide file tree

Showing 11 changed files with 76 additions and 53 deletions.
diff --git a/common/clicfg/credentials/aura.go b/common/clicfg/credentials/aura.go
@@ -2,9 +2,10 @@ package credentials
 
 import (
 	"encoding/json"
-	"fmt"
 	"io"
 	"time"
+
+	"github.com/neo4j/cli/common/clierr"
 )
 
 type AuraCredentials struct {
@@ -32,7 +33,7 @@ func (c *AuraCredentials) Add(name string, clientId string, clientSecret string)
 	auraCredentials := c.Credentials
 	for _, credential := range auraCredentials {
 		if credential.Name == name {
-			return fmt.Errorf("already have credential with name %s", name)
+			return clierr.NewUsageError("already have credential with name %s", name)
 		}
 	}
 
@@ -55,7 +56,7 @@ func (c *AuraCredentials) Remove(name string) error {
 	}
 
 	if indexToRemove == -1 {
-		return fmt.Errorf("could not find credential with name %s to remove", name)
+		return clierr.NewUsageError("could not find credential with name %s to remove", name)
 	}
 
 	if c.DefaultCredential == name {
@@ -69,7 +70,7 @@ func (c *AuraCredentials) Remove(name string) error {
 
 func (c *AuraCredentials) SetDefault(name string) error {
 	if !c.credentialExists(name) {
-		return fmt.Errorf("could not find credential with name %s", name)
+		return clierr.NewUsageError("could not find credential with name %s", name)
 	}
 
 	c.DefaultCredential = name
@@ -79,7 +80,7 @@ func (c *AuraCredentials) SetDefault(name string) error {
 
 func (c *AuraCredentials) GetDefault() (*AuraCredential, error) {
 	if c.DefaultCredential == "" {
-		return nil, fmt.Errorf("default credential not set")
+		return nil, clierr.NewUsageError("default credential not set")
 	}
 	return c.Get(c.DefaultCredential)
 }
@@ -90,13 +91,13 @@ func (c *AuraCredentials) Get(name string) (*AuraCredential, error) {
 			return credential, nil
 		}
 	}
-	return nil, fmt.Errorf("could not find credential with name %s", name)
+	return nil, clierr.NewUsageError("could not find credential with name %s", name)
 }
 
-func (c *AuraCredentials) UpdateAccessToken(cred *AuraCredential, accessToken string, expiresInSeconds int64) (*AuraCredential, error) {
+func (c *AuraCredentials) UpdateAccessToken(cred *AuraCredential, accessToken string, expiresInSeconds int64) *AuraCredential {
 	credential, err := c.Get(cred.Name)
 	if err != nil {
-		return nil, err
+		panic(err)
 	}
 	const expireToleranceSeconds = 60
 
@@ -105,7 +106,7 @@ func (c *AuraCredentials) UpdateAccessToken(cred *AuraCredential, accessToken st
 	credential.TokenExpiry = now + (expiresInSeconds-expireToleranceSeconds)*1000
 	credential.AccessToken = accessToken
 	c.onUpdate()
-	return credential, nil
+	return credential
 }
 
 func (c *AuraCredentials) ClearAccessToken(cred *AuraCredential) (*AuraCredential, error) {

diff --git a/common/clierr/error.go b/common/clierr/error.go
@@ -0,0 +1,18 @@
+package clierr
+
+import "fmt"
+
+// Usage Error, require feedback
+func NewUsageError(msg string, a ...any) error {
+	return fmt.Errorf(msg, a...)
+}
+
+// API errors, retry may solve it
+func NewUpstreamError(msg string, a ...any) error {
+	return fmt.Errorf(msg, a...)
+}
+
+// Fatal error, unrecoverable
+func NewFatalError(msg string, a ...any) error {
+	return fmt.Errorf(msg, a...)
+}
diff --git a/neo4j-cli/aura/internal/api/poll.go b/neo4j-cli/aura/internal/api/poll.go
@@ -7,6 +7,7 @@ import (
 	"time"
 
 	"github.com/neo4j/cli/common/clicfg"
+	"github.com/neo4j/cli/common/clierr"
 )
 
 type PollResponse struct {
@@ -52,13 +53,13 @@ func Poll(cfg *clicfg.Config, url string, cond func(status string) bool) (*PollR
 			Method: http.MethodGet,
 		})
 		if err != nil {
-			return nil, err
+			return nil, clierr.NewUpstreamError("error polling: %w", err)
 		}
 
 		if statusCode == http.StatusOK {
 			var response PollResponse
 			if err := json.Unmarshal(resBody, &response); err != nil {
-				return nil, err
+				return nil, clierr.NewUpstreamError("cannot retrieve response polling: %w", err)
 			}
 
 			// Successful poll, return last response
@@ -68,5 +69,5 @@ func Poll(cfg *clicfg.Config, url string, cond func(status string) bool) (*PollR
 		}
 	}
 
-	return nil, fmt.Errorf("hit max retries [%d] for polling", pollingConfig.MaxRetries)
+	return nil, clierr.NewUpstreamError("hit max retries [%d] polling", pollingConfig.MaxRetries)
 }
diff --git a/neo4j-cli/aura/internal/api/response.go b/neo4j-cli/aura/internal/api/response.go
@@ -10,6 +10,7 @@ import (
 
 	"github.com/neo4j/cli/common/clicfg"
 	"github.com/neo4j/cli/common/clicfg/credentials"
+	"github.com/neo4j/cli/common/clierr"
 )
 
 type ErrorResponse struct {
@@ -27,24 +28,23 @@ type ServerError struct {
 }
 
 func handleResponseError(res *http.Response, credential *credentials.AuraCredential, cfg *clicfg.Config) error {
-	var err error
 	resBody, err := io.ReadAll(res.Body)
 
 	if err != nil {
-		return err
+		panic(clierr.NewFatalError("unexpected error reading response body. %w", err))
 	}
 
 	switch statusCode := res.StatusCode; statusCode {
 	// redirection messages
 	case http.StatusPermanentRedirect:
-		return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+		panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 	// client error responses
 	case http.StatusBadRequest:
 		var errorResponse ErrorResponse
 
 		err = json.Unmarshal(resBody, &errorResponse)
 		if err != nil {
-			return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+			panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 		}
 
 		messages := []string{}
@@ -56,18 +56,18 @@ func handleResponseError(res *http.Response, credential *credentials.AuraCredent
 			messages = append(messages, message)
 		}
 
-		return fmt.Errorf("%s", messages)
+		return clierr.NewUpstreamError("%s", messages)
 	case http.StatusUnauthorized:
 		return formatAuthorizationError(resBody, statusCode, credential, cfg)
 	case http.StatusForbidden:
 		// Requested endpoint is forbidden
 		var serverError ServerError
 		err := json.Unmarshal(resBody, &serverError)
 		if err != nil {
-			return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+			panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 		}
 		if serverError.Error != "" {
-			return fmt.Errorf(serverError.Error)
+			return clierr.NewUpstreamError(serverError.Error)
 		}
 
 		return formatAuthorizationError(resBody, statusCode, credential, cfg)
@@ -76,65 +76,65 @@ func handleResponseError(res *http.Response, credential *credentials.AuraCredent
 
 		err = json.Unmarshal(resBody, &errorResponse)
 		if err != nil {
-			return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+			panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 		}
 
 		messages := []string{}
 		for _, e := range errorResponse.Errors {
 			messages = append(messages, e.Message)
 		}
 
-		return fmt.Errorf("%s", messages)
+		return clierr.NewUpstreamError("%s", messages)
 	case http.StatusMethodNotAllowed:
 		var errorResponse ErrorResponse
 
 		err = json.Unmarshal(resBody, &errorResponse)
 		if err != nil {
-			return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+			panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 		}
 
 		messages := []string{}
 		for _, e := range errorResponse.Errors {
 			messages = append(messages, e.Message)
 		}
 
-		return fmt.Errorf("%s", messages)
+		return clierr.NewUpstreamError("%s", messages)
 	case http.StatusConflict:
 		var errorResponse ErrorResponse
 
 		err = json.Unmarshal(resBody, &errorResponse)
 		if err != nil {
-			return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+			panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 		}
 
 		messages := []string{}
 		for _, e := range errorResponse.Errors {
 			messages = append(messages, e.Message)
 		}
 
-		return fmt.Errorf("%s", messages)
+		return clierr.NewUpstreamError("%s", messages)
 	case http.StatusUnsupportedMediaType:
-		return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+		panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 	case http.StatusTooManyRequests:
 		retryAfter := res.Header.Get("Retry-After")
-		return fmt.Errorf("server rate limit exceeded, suggested cool-off period is %s seconds before rerunning the command", retryAfter)
+		return clierr.NewUpstreamError("server rate limit exceeded, suggested cool-off period is %s seconds before rerunning the command", retryAfter)
 	// server error responses
 	case http.StatusInternalServerError, http.StatusBadGateway, http.StatusServiceUnavailable, http.StatusGatewayTimeout:
 		var errorResponse ErrorResponse
 
 		err = json.Unmarshal(resBody, &errorResponse)
 		if err != nil {
-			return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+			panic(clierr.NewFatalError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:]))
 		}
 
 		messages := []string{}
 		for _, e := range errorResponse.Errors {
 			messages = append(messages, e.Message)
 		}
 
-		return fmt.Errorf("%s", messages)
+		return clierr.NewUpstreamError("%s", messages)
 	default:
-		return fmt.Errorf("unexpected status code %d and body %s running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, resBody, os.Args[1:])
+		panic(clierr.NewFatalError("unexpected status code %d and body %s running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, resBody, os.Args[1:]))
 	}
 }
 
@@ -254,7 +254,7 @@ type ListResponseData struct {
 
 func (d ListResponseData) GetSingleOrError() (map[string]any, error) {
 	if len(d.Data) != 1 {
-		return nil, fmt.Errorf("expected 1 array value: %v", len(d.Data))
+		return nil, clierr.NewFatalError("expected 1 array value: %v", len(d.Data))
 	}
 	return d.Data[0], nil
 }
@@ -310,7 +310,7 @@ func formatAuthorizationError(resBody []byte, statusCode int, credential *creden
 
 	err := json.Unmarshal(resBody, &errorResponse)
 	if err != nil {
-		return fmt.Errorf("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
+		return clierr.NewUsageError("unexpected error [status %d] running CLI with args %s, please report an issue in https://github.com/neo4j/cli", statusCode, os.Args[1:])
 	}
 
 	messages := []string{}
@@ -325,7 +325,7 @@ func formatAuthorizationError(resBody []byte, statusCode int, credential *creden
 		messages = append(messages, "Request failed authorization - access token has been cleared and will be refreshed on next request - please retry the command")
 	}
 
-	return fmt.Errorf(`[
+	return clierr.NewUsageError(`[
 	%s
 ]`, strings.Join(messages, ",\n\t"))
 }
diff --git a/neo4j-cli/aura/internal/api/token.go b/neo4j-cli/aura/internal/api/token.go
@@ -2,7 +2,6 @@ package api
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -11,6 +10,7 @@ import (
 
 	"github.com/neo4j/cli/common/clicfg"
 	"github.com/neo4j/cli/common/clicfg/credentials"
+	"github.com/neo4j/cli/common/clierr"
 )
 
 func getToken(credential *credentials.AuraCredential, cfg *clicfg.Config) (string, error) {
@@ -26,7 +26,7 @@ func getToken(credential *credentials.AuraCredential, cfg *clicfg.Config) (strin
 
 	req, err := http.NewRequest(http.MethodPost, url, strings.NewReader(data.Encode()))
 	if err != nil {
-		panic(err)
+		panic(clierr.NewFatalError("can't retrieve authentication token. %w", err))
 	}
 
 	version := cfg.Version
@@ -41,33 +41,31 @@ func getToken(credential *credentials.AuraCredential, cfg *clicfg.Config) (strin
 
 	res, err := client.Do(req)
 	if err != nil {
-		panic(err)
+		panic(clierr.NewFatalError("can't retrieve authentication token. %w", err))
 	}
 	defer res.Body.Close()
 
 	switch statusCode := res.StatusCode; statusCode {
-	case http.StatusBadRequest:
-		return "", errors.New("request is invalid")
 	case http.StatusUnauthorized:
-		return "", errors.New("the provided credentials are invalid, expired, or revoked")
+		return "", clierr.NewUsageError("the provided credentials are invalid, expired, or revoked")
+	case http.StatusBadRequest:
 	case http.StatusForbidden:
-		return "", errors.New("the request body is invalid")
 	case http.StatusNotFound:
-		return "", errors.New("the request body is missing")
+		panic(clierr.NewFatalError("can't retrieve authentication token. Response status code [%d]", http.StatusBadRequest))
 	}
 
 	resBody, err := io.ReadAll(res.Body)
 	if err != nil {
-		panic(err)
+		panic(clierr.NewFatalError("can't retrieve authentication token. %w", err))
 	}
 
 	var grant Grant
 
 	err = json.Unmarshal(resBody, &grant)
 	if err != nil {
-		panic(err)
+		panic(clierr.NewFatalError("can't retrieve authentication token. %w", err))
 	}
 
-	_, err = cfg.Credentials.Aura.UpdateAccessToken(credential, grant.AccessToken, grant.ExpiresIn)
+	cfg.Credentials.Aura.UpdateAccessToken(credential, grant.AccessToken, grant.ExpiresIn)
 	return grant.AccessToken, err
 }
diff --git a/neo4j-cli/aura/internal/subcommands/config/set.go b/neo4j-cli/aura/internal/subcommands/config/set.go
@@ -1,9 +1,8 @@
 package config
 
 import (
-	"fmt"
-
 	"github.com/neo4j/cli/common/clicfg"
+	"github.com/neo4j/cli/common/clierr"
 	"github.com/spf13/cobra"
 )
 
@@ -17,7 +16,7 @@ func NewSetCmd(cfg *clicfg.Config) *cobra.Command {
 			}
 
 			if !cfg.Aura.IsValidConfigKey(args[0]) {
-				return fmt.Errorf("invalid config key specified: %s", args[0])
+				return clierr.NewUsageError("invalid config key specified: %s", args[0])
 			}
 
 			if args[0] == "output" {
@@ -29,7 +28,7 @@ func NewSetCmd(cfg *clicfg.Config) *cobra.Command {
 					}
 				}
 				if !validOutputValue {
-					return fmt.Errorf("invalid output value specified: %s", args[1])
+					return clierr.NewUsageError("invalid output value specified: %s", args[1])
 				}
 			}
 

diff --git a/neo4j-cli/aura/internal/subcommands/customermanagedkey/customermanagedkey.go b/neo4j-cli/aura/internal/subcommands/customermanagedkey/customermanagedkey.go
@@ -5,6 +5,7 @@ import (
 	"strings"
 
 	"github.com/neo4j/cli/common/clicfg"
+	"github.com/neo4j/cli/common/clierr"
 	"github.com/spf13/cobra"
 )
 
@@ -28,7 +29,7 @@ func NewCmd(cfg *clicfg.Config) *cobra.Command {
 					}
 				}
 				if !validOutputValue {
-					return fmt.Errorf("invalid output value specified: %s", outputValue)
+					return clierr.NewUsageError("invalid output value specified: %s", outputValue)
 				}
 			}