Improve Invalid Configuration Errors

.github/workflows/test.yaml

@@ -20,17 +20,17 @@ jobs:
         working-directory: ${{ env.GOPATH }}/src/go.rtnl.ai/confire
     steps:
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v6
         with:
-          go-version: 1.19
+          go-version: 1.26
 
       - name: Checkout Code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v6
         with:
           path: ${{ env.GOPATH }}/src/go.rtnl.ai/confire
 
       - name: Install Staticcheck
-        run: go install honnef.co/go/tools/cmd/staticcheck@2023.1.3
+        run: go install honnef.co/go/tools/cmd/staticcheck@2026.1
 
       - name: Lint Go Code
         run: staticcheck ./...
@@ -41,7 +41,7 @@ jobs:
     strategy:
       fail-fast: true
       matrix:
-        go-version: [1.18.x, 1.19.x, 1.20.x]
+        go-version: [1.20.x, 1.25.x, 1.26.x]
     env:
       GOPATH: ${{ github.workspace }}/go
       GOBIN: ${{ github.workspace }}/go/bin
@@ -50,20 +50,20 @@ jobs:
         working-directory: ${{ env.GOPATH }}/src/go.rtnl.ai/confire
     steps:
       - name: Set up Go
-        uses: actions/setup-go@v3
+        uses: actions/setup-go@v6
         with:
           go-version: ${{ matrix.go-version }}
 
       - name: Cache Speedup
-        uses: actions/cache@v3
+        uses: actions/cache@v5
         with:
           path: ~/go/pkg/mod
           key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }}
           restore-keys: |
             ${{ runner.os }}-go-
 
       - name: Checkout Code
-        uses: actions/checkout@v3
+        uses: actions/checkout@v6
         with:
           path: ${{ env.GOPATH }}/src/go.rtnl.ai/confire
 

errors.go

@@ -21,17 +21,26 @@ func IsParseError(err error) bool {
 	return errors.As(err, &target)
 }
 
-// Extract a validation error from an error if it is one.
-func ValidationError(err error) (*confireErrors.ValidationError, bool) {
-	target := &confireErrors.ValidationError{}
+// Extract a configuration error from an error if it is one.
+func InvalidConfig(err error) (*confireErrors.InvalidConfig, bool) {
+	target := &confireErrors.InvalidConfig{}
 	if ok := errors.As(err, &target); ok {
 		return target, true
 	}
 	return nil, false
 }
 
-// Returns true if the underlying error is a validation error.
-func IsValidationError(err error) bool {
-	target := &confireErrors.ValidationError{}
+// Returns true if the underlying error is a configuration error.
+func IsInvalidConfig(err error) bool {
+	target := &confireErrors.InvalidConfig{}
 	return errors.As(err, &target)
 }
+
+// Bringing in the errors from the errors package for convenience.
+var (
+	Required = confireErrors.Required
+	Invalid  = confireErrors.Invalid
+	Parse    = confireErrors.Parse
+	Wrap     = confireErrors.Wrap
+	Join     = confireErrors.Join
+)

errors/config.go

@@ -0,0 +1,73 @@
+package errors
+
+import (
+	"errors"
+	"fmt"
+)
+
+func Required(conf, field string) *InvalidConfig {
+	return &InvalidConfig{
+		conf:  conf,
+		field: field,
+		issue: "is required but not set",
+		err:   ErrMissingRequired,
+	}
+}
+
+func Invalid(conf, field, issue string, args ...any) *InvalidConfig {
+	return &InvalidConfig{
+		conf:  conf,
+		field: field,
+		issue: fmt.Sprintf(issue, args...),
+	}
+}
+
+func Parse(conf, field string, err error) *InvalidConfig {
+	return &InvalidConfig{
+		conf:  conf,
+		field: field,
+		issue: fmt.Sprintf("could not parse value: %s", err.Error()),
+		err:   err,
+	}
+}
+
+func Wrap(conf, field, issue string, err error, args ...any) *InvalidConfig {
+	return &InvalidConfig{
+		conf:  conf,
+		field: field,
+		issue: fmt.Sprintf(issue, args...),
+		err:   err,
+	}
+}
+
+// Invalid is a field-specific configuration validation error and is returned either by
+// field specification validation or by the user in a custom Validate() method.
+type InvalidConfig struct {
+	conf  string
+	field string
+	issue string
+	err   error
+}
+
+func (e *InvalidConfig) Error() string {
+	field := e.field
+	if e.conf != "" {
+		field = e.conf + "." + e.field
+	}
+	return fmt.Sprintf("invalid configuration: %s %s", field, e.issue)
+}
+
+func (e *InvalidConfig) Field() string {
+	if e.conf != "" {
+		return e.conf + "." + e.field
+	}
+	return e.field
+}
+
+func (e *InvalidConfig) Is(target error) bool {
+	return errors.Is(e.err, target)
+}
+
+func (e *InvalidConfig) Unwrap() error {
+	return e.err
+}

errors/config_test.go

@@ -0,0 +1,87 @@
+package errors
+
+import (
+	"errors"
+	"testing"
+
+	"go.rtnl.ai/confire/assert"
+)
+
+func TestRequired(t *testing.T) {
+	err := Required("", "bind_addr")
+	assert.Equals(t, "invalid configuration: bind_addr is required but not set", err.Error())
+	assert.Assert(t, err.Is(ErrMissingRequired), "required should wrap a missing required field error")
+	assert.Equals(t, ErrMissingRequired, err.Unwrap())
+}
+
+func TestInvalid(t *testing.T) {
+	err := Invalid("", "mode", "invalid mode %q", "foo")
+	assert.Equals(t, "invalid configuration: mode invalid mode \"foo\"", err.Error())
+	assert.Equals(t, nil, err.Unwrap())
+}
+
+func TestParse(t *testing.T) {
+	err := Parse("", "bind_addr", errors.New("invalid bind address"))
+	assert.Equals(t, "invalid configuration: bind_addr could not parse value: invalid bind address", err.Error())
+	assert.Equals(t, errors.New("invalid bind address"), err.Unwrap())
+}
+
+func TestWrap(t *testing.T) {
+	err := Wrap("", "bind_addr", "invalid bind address %q", errors.New("invalid bind address"), "foo")
+	assert.Equals(t, "invalid configuration: bind_addr invalid bind address \"foo\"", err.Error())
+	assert.Equals(t, errors.New("invalid bind address"), err.Unwrap())
+}
+
+func TestInvalidConfig(t *testing.T) {
+	testCases := []struct {
+		conf     string
+		field    string
+		issue    string
+		err      error
+		errstr   string
+		fieldstr string
+	}{
+		{
+			conf:     "",
+			field:    "bind_addr",
+			issue:    "is required but not set",
+			errstr:   "invalid configuration: bind_addr is required but not set",
+			fieldstr: "bind_addr",
+		},
+		{
+			conf:     "",
+			field:    "bind_addr",
+			issue:    "is required but not set",
+			err:      errors.New("required field is zero valued"),
+			errstr:   "invalid configuration: bind_addr is required but not set",
+			fieldstr: "bind_addr",
+		},
+		{
+			conf:     "telemetry",
+			field:    "service_name",
+			issue:    "cannot have spaces or start with a number",
+			errstr:   "invalid configuration: telemetry.service_name cannot have spaces or start with a number",
+			fieldstr: "telemetry.service_name",
+		},
+		{
+			conf:     "telemetry",
+			field:    "service_name",
+			issue:    "cannot have spaces or start with a number",
+			err:      errors.New("invalid service name"),
+			errstr:   "invalid configuration: telemetry.service_name cannot have spaces or start with a number",
+			fieldstr: "telemetry.service_name",
+		},
+	}
+
+	for _, tc := range testCases {
+		err := Wrap(tc.conf, tc.field, tc.issue, tc.err)
+		assert.Equals(t, tc.errstr, err.Error())
+		assert.Equals(t, tc.fieldstr, err.Field())
+		if tc.err != nil {
+			assert.Assert(t, err.Is(tc.err), "wrap should wrap the error")
+			assert.Equals(t, tc.err, err.Unwrap())
+		} else {
+			assert.Equals(t, nil, err.Unwrap())
+		}
+	}
+}

errors/errors.go

@@ -11,57 +11,89 @@ var (
 	ErrNotAStruct           = errors.New("cannot wrap a non-struct type")
 	ErrNotExported          = errors.New("field is not exported")
 	ErrNotSettable          = errors.New("field is not settable")
-	ErrMissingRequiredField = errors.New("required field is zero valued")
+	ErrMissingRequired      = errors.New("required field is zero valued")
 )
 
-type ParseError struct {
-	Source string
-	Field  string
-	Type   string
-	Value  string
-	Err    error
-}
-
-func (e *ParseError) Error() string {
-	return fmt.Sprintf("confire: could not parse %[2]s from %[1]s: converting %[4]q to type %[3]s: %[5]s", e.Source, e.Field, e.Type, e.Value, e.Err)
-}
+type ValidationErrors []*InvalidConfig
 
-func (e *ParseError) Is(target error) bool {
-	return errors.Is(e.Err, target)
-}
+func (e ValidationErrors) Error() string {
+	if len(e) == 1 {
+		return e[0].Error()
+	}
 
-func (e *ParseError) Unwrap() error {
-	return e.Err
+	sb := strings.Builder{}
+	sb.WriteString(fmt.Sprintf("%d validation errors occurred:", len(e)))
+	for _, err := range e {
+		sb.WriteString(fmt.Sprintf("\n    - %s", err.Error()))
+	}
+	return sb.String()
 }
 
-type ValidationError struct {
-	Source string
-	Err    error
+func (e ValidationErrors) Is(target error) bool {
+	for _, err := range e {
+		if errors.Is(err, target) {
+			return true
+		}
+	}
+	return false
 }
 
-func (e *ValidationError) Error() string {
-	return fmt.Sprintf("invalid configuration: %s", e.Err)
+func (e ValidationErrors) Contains(target error) bool {
+	return e.Is(target)
 }
 
-func (e *ValidationError) Is(target error) bool {
-	return errors.Is(e.Err, target)
-}
+func Join(err error, errs ...error) error {
+	var (
+		verrs  ValidationErrors
+		isverr bool
+	)
 
-func (e *ValidationError) Unwrap() error {
-	return e.Err
-}
+	// If the first error is not nil and it is not ValidationErrors then use the
+	// regular errors.Join function. If it is a ValidationError then continue. If the
+	// first error is nil, then create new ValidationErrors.
+	if err != nil {
+		// If the error is an InvalidConfig error then create a new ValidationErrors
+		if cerr, ok := err.(*InvalidConfig); ok {
+			verrs = ValidationErrors{cerr}
+		} else {
+			// If the error is ValidationErrors then flatten it into the current ValidationErrors.
+			verrs, isverr = err.(ValidationErrors)
+			if !isverr {
+				errs = append([]error{err}, errs...)
+				return errors.Join(errs...)
+			}
+		}
+	} else {
+		verrs = make(ValidationErrors, 0, len(errs))
+	}
 
-type ValidationErrors []*ValidationError
+	// Loop through the remaining errors and append them to the ValidationErrors if
+	// they are not nil and they are InvalidConfig errors. If they are not InvalidConfig
+	// errors then use the regular errors.Join function.
+	for _, err := range errs {
+		if err != nil {
+			// If the error is ValidationErrors then flatten it into the current ValidationErrors.
+			if errs, ok := err.(ValidationErrors); ok {
+				verrs = append(verrs, errs...)
+				continue
+			}
 
-func (e ValidationErrors) Error() string {
-	if len(e) == 1 {
-		return e[0].Error()
+			cerr, iscerr := err.(*InvalidConfig)
+			if !iscerr {
+				errs = append([]error{err}, errs...)
+				return errors.Join(errs...)
+			}
+			verrs = append(verrs, cerr)
+		}
 	}
 
-	sb := strings.Builder{}
-	sb.WriteString(fmt.Sprintf("%d validation errors occurred:", len(e)))
-	for _, err := range e {
-		sb.WriteString(fmt.Sprintf("\n    - %s", err.Error()))
+	// If the ValidationErrors is empty, then return nil.
+	switch len(verrs) {
+	case 0:
+		return nil
+	case 1:
+		return verrs[0]
+	default:
+		return verrs
 	}
-	return sb.String()
 }