REORG/MEDIUM: use readable configuration errors

This breaks the API of configuration.ConfError! Replace the configuration error codes by error strings, easier to understand. This slightly modifies error messages, but the error variable names remain unchanged. The method ConfError.Code() has been removed. Use ConfError.Is() or errors.Is() the compare errors.
haproxytech · Oct 11, 2023 · 519f1b0 · 519f1b0
1 parent 23efe06
commit 519f1b0
Show file tree

Hide file tree

Showing 12 changed files with 52 additions and 45 deletions.
diff --git a/configuration/backend_test.go b/configuration/backend_test.go
@@ -983,7 +983,7 @@ func TestCreateEditDeleteBackend(t *testing.T) {
 	err = clientTest.DeleteBackend("created", "", 999999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("Should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/cache_test.go b/configuration/cache_test.go
@@ -178,7 +178,7 @@ func TestCreateEditDeleteCache(t *testing.T) {
 	err = clientTest.DeleteCache("created_cache", "", 999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("Should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/configuration.go b/configuration/configuration.go
@@ -95,7 +95,7 @@ func (c *client) GetParser(transactionID string) (parser.Parser, error) {
 	p, ok := c.parsers[transactionID]
 	c.clientMu.Unlock()
 	if !ok {
-		return nil, NewConfError(ErrTransactionDoesNotExist, fmt.Sprintf("Transaction %s does not exist", transactionID))
+		return nil, NewConfError(ErrTransactionDoesNotExist, transactionID)
 	}
 	return p, nil
 }
@@ -109,7 +109,7 @@ func (c *client) AddParser(transactionID string) error {
 	_, ok := c.parsers[transactionID]
 	c.clientMu.Unlock()
 	if ok {
-		return NewConfError(ErrTransactionAlreadyExists, fmt.Sprintf("Transaction %s already exists", transactionID))
+		return NewConfError(ErrTransactionAlreadyExists, transactionID)
 	}
 
 	parserOptions := []parser_options.ParserOption{}
@@ -150,7 +150,7 @@ func (c *client) DeleteParser(transactionID string) error {
 	_, ok := c.parsers[transactionID]
 	c.clientMu.Unlock()
 	if !ok {
-		return NewConfError(ErrTransactionDoesNotExist, fmt.Sprintf("Transaction %s does not exist", transactionID))
+		return NewConfError(ErrTransactionDoesNotExist, transactionID)
 	}
 	delete(c.parsers, transactionID)
 	return nil
@@ -165,7 +165,7 @@ func (c *client) CommitParser(transactionID string) error {
 	p, ok := c.parsers[transactionID]
 	c.clientMu.Unlock()
 	if !ok {
-		return NewConfError(ErrTransactionDoesNotExist, fmt.Sprintf("Transaction %s does not exist", transactionID))
+		return NewConfError(ErrTransactionDoesNotExist, transactionID)
 	}
 	c.parser = p
 	delete(c.parsers, transactionID)
@@ -180,7 +180,7 @@ func (c *client) GetVersion(transactionID string) (int64, error) {
 func (c *client) getVersion(transactionID string) (int64, error) {
 	p, err := c.GetParser(transactionID)
 	if err != nil {
-		return 0, NewConfError(ErrCannotReadVersion, fmt.Sprintf("Cannot read version: %s", err.Error()))
+		return 0, NewConfError(ErrCannotReadVersion, err.Error())
 	}
 
 	data, _ := p.Get(parser.Comments, parser.CommentsSectionName, "# _version", true)
@@ -194,7 +194,7 @@ func (c *client) IncrementVersion() error {
 	ver.Value++
 
 	if err := c.parser.Save(c.ConfigurationFile); err != nil {
-		return NewConfError(ErrCannotSetVersion, fmt.Sprintf("Cannot set version: %s", err.Error()))
+		return NewConfError(ErrCannotSetVersion, err.Error())
 	}
 	return nil
 }

diff --git a/configuration/errors.go b/configuration/errors.go
@@ -16,60 +16,67 @@
 package configuration
 
 import (
+	"errors"
 	"fmt"
 
 	oaerrors "github.com/go-openapi/errors"
 )
 
-const (
+var (
 	// General error, unknown cause
-	ErrGeneralError = 0
+	ErrGeneralError = errors.New("unknown error")
 
 	// Errors regarding configurations
-	ErrNoParentSpecified      = 10
-	ErrParentDoesNotExist     = 11
-	ErrBothVersionTransaction = 12
-	ErrNoVersionTransaction   = 13
-	ErrValidationError        = 14
-	ErrVersionMismatch        = 15
+	ErrNoParentSpecified      = errors.New("unspecified parent")
+	ErrParentDoesNotExist     = errors.New("missing parent")
+	ErrBothVersionTransaction = errors.New("both version and transaction specified, specify only one")
+	ErrNoVersionTransaction   = errors.New("version or transaction not specified")
+	ErrValidationError        = errors.New("validation error")
+	ErrVersionMismatch        = errors.New("version mismatch")
 
-	ErrTransactionDoesNotExist  = 20
-	ErrTransactionAlreadyExists = 21
-	ErrCannotParseTransaction   = 22
+	ErrTransactionDoesNotExist  = errors.New("transaction does not exist")
+	ErrTransactionAlreadyExists = errors.New("transaction already exist")
+	ErrCannotParseTransaction   = errors.New("failed to parse transaction")
 
-	ErrObjectDoesNotExist    = 30
-	ErrObjectAlreadyExists   = 31
-	ErrObjectIndexOutOfRange = 32
+	ErrObjectDoesNotExist    = errors.New("missing object")
+	ErrObjectAlreadyExists   = errors.New("object already exists")
+	ErrObjectIndexOutOfRange = errors.New("index out of range")
 
-	ErrErrorChangingConfig = 40
-	ErrCannotReadConfFile  = 41
-	ErrCannotReadVersion   = 42
-	ErrCannotSetVersion    = 43
+	ErrErrorChangingConfig = errors.New("invalid change")
+	ErrCannotReadConfFile  = errors.New("failed to read configuration")
+	ErrCannotReadVersion   = errors.New("cannot read version")
+	ErrCannotSetVersion    = errors.New("cannot set version")
 
-	ErrCannotFindHAProxy = 50
+	ErrCannotFindHAProxy = errors.New("failed to find HAProxy")
 
-	ErrClientDoesNotExists = 60
+	ErrClientDoesNotExists = errors.New("client does not exist")
 )
 
 // ConfError general configuration client error
 type ConfError struct {
-	code int
-	msg  string
+	err    error
+	reason string // optional
+}
+
+func (e *ConfError) Err() error {
+	return e.err
 }
 
 // Error implementation for ConfError
 func (e *ConfError) Error() string {
-	return fmt.Sprintf("%v: %s", e.code, e.msg)
+	if e.reason == "" {
+		return e.err.Error()
+	}
+	return fmt.Sprintf("%s: %s", e.err.Error(), e.reason)
 }
 
-// Code returns ConfError code, which is one of constants in this package
-func (e *ConfError) Code() int {
-	return e.code
+func (e *ConfError) Is(target error) bool {
+	return e.err == target
 }
 
 // NewConfError constructor for ConfError
-func NewConfError(code int, msg string) *ConfError {
-	return &ConfError{code: code, msg: msg}
+func NewConfError(err error, reason string) *ConfError {
+	return &ConfError{err: err, reason: reason}
 }
 
 // CompositeTransactionError helper function to aggregate multiple errors

diff --git a/configuration/frontend_test.go b/configuration/frontend_test.go
@@ -655,7 +655,7 @@ func TestCreateEditDeleteFrontend(t *testing.T) {
 	err = clientTest.DeleteFrontend("created", "", 999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("Should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/log_forward_test.go b/configuration/log_forward_test.go
@@ -129,7 +129,7 @@ func TestCreateEditDeleteLogForward(t *testing.T) {
 	err = clientTest.DeleteLogForward("created_log_forward", "", 999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/mailers_section_test.go b/configuration/mailers_section_test.go
@@ -145,7 +145,7 @@ func TestCreateEditDeleteMailersSection(t *testing.T) {
 	err = clientTest.DeleteMailersSection("newMailer", "", 999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("Should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/peer_section_test.go b/configuration/peer_section_test.go
@@ -176,7 +176,7 @@ func TestCreateEditDeletePeerSection(t *testing.T) {
 	err = clientTest.DeletePeerSection("testcluster", "", 999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("Should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/resolver_test.go b/configuration/resolver_test.go
@@ -125,7 +125,7 @@ func TestCreateEditDeleteResolver(t *testing.T) {
 	err = clientTest.DeleteResolver("created_resolver", "", 999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("Should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/ring_test.go b/configuration/ring_test.go
@@ -145,7 +145,7 @@ func TestCreateEditDeleteRing(t *testing.T) {
 	err = clientTest.DeleteRing("created_ring", "", 999999)
 	if err != nil {
 		if confErr, ok := err.(*ConfError); ok {
-			if confErr.Code() != ErrVersionMismatch {
+			if !confErr.Is(ErrVersionMismatch) {
 				t.Error("should throw ErrVersionMismatch error")
 			}
 		} else {

diff --git a/configuration/transaction-handling.go b/configuration/transaction-handling.go
@@ -85,7 +85,7 @@ func (c *client) GetFailedParserTransactionVersion(transactionID string) (int64,
 
 	ver, ok := data.(*types.ConfigVersion)
 	if !ok {
-		return 0, NewConfError(ErrCannotReadVersion, "cannot read version")
+		return 0, NewConfError(ErrCannotReadVersion, "")
 	}
 	return ver.Value, nil
 }

diff --git a/configuration/transaction.go b/configuration/transaction.go
@@ -316,7 +316,7 @@ func (t *Transaction) CheckTransactionOrVersion(transactionID string, version in
 	// start an implicit transaction if transaction is not already given
 	tID := ""
 	if transactionID != "" && version != 0 {
-		return "", NewConfError(ErrBothVersionTransaction, "both version and transaction specified, specify only one")
+		return "", NewConfError(ErrBothVersionTransaction, "")
 	}
 	if transactionID == "" && version == 0 {
 		return "", NewConfError(ErrNoVersionTransaction, "version or transaction not specified, specify only one")
@@ -685,7 +685,7 @@ func (t *Transaction) getFailedTransactionVersion(transactionID string) (int64,
 
 	ver, err := t.TransactionClient.GetFailedParserTransactionVersion(transactionID)
 	if err != nil {
-		return 0, NewConfError(ErrCannotReadVersion, "cannot read version")
+		return 0, NewConfError(ErrCannotReadVersion, "")
 	}
 	return ver, nil
 }