Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: simplify BatchError #4292

Merged
merged 2 commits into from
Aug 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
- name: Set up Go 1.x
uses: actions/setup-go@v5
with:
# use 1.20 to guarantee Go 1.20 compatibility
# make sure Go version compatible with go-zero
go-version-file: go.mod
check-latest: true
cache: true
Expand Down
49 changes: 15 additions & 34 deletions core/errorx/batcherror.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,13 @@ import (
"sync"
)

type (
// A BatchError is an error that can hold multiple errors.
BatchError struct {
errs errorArray
lock sync.Mutex
}

errorArray []error
)
// BatchError is an error that can hold multiple errors.
type BatchError struct {
errs []error
lock sync.RWMutex
}

// Add adds errs to be, nil errors are ignored.
// Add adds one or more non-nil errors to the BatchError instance.
func (be *BatchError) Add(errs ...error) {
be.lock.Lock()
defer be.lock.Unlock()
Expand All @@ -27,35 +23,20 @@ func (be *BatchError) Add(errs ...error) {
}
}

// Err returns an error that represents all errors.
// Err returns an error that represents all accumulated errors.
// It returns nil if there are no errors.
func (be *BatchError) Err() error {
be.lock.Lock()
defer be.lock.Unlock()
be.lock.RLock()
defer be.lock.RUnlock()

switch len(be.errs) {
case 0:
return nil
case 1:
return be.errs[0]
default:
return be.errs
}
// If there are no non-nil errors, errors.Join(...) returns nil.
return errors.Join(be.errs...)
}

// NotNil checks if any error inside.
// NotNil checks if there is at least one error inside the BatchError.
func (be *BatchError) NotNil() bool {
be.lock.Lock()
defer be.lock.Unlock()
be.lock.RLock()
defer be.lock.RUnlock()

return len(be.errs) > 0
}

// Error returns a string that represents inside errors.
func (ea errorArray) Error() string {
return errors.Join(ea...).Error()
}

// Unwrap combine the errors in the errorArray into a single error return
func (ea errorArray) Unwrap() error {
return errors.Join(ea...)
}
50 changes: 50 additions & 0 deletions core/errorx/batcherror_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,53 @@ func TestBatchError_Unwrap(t *testing.T) {
assert.False(t, errors.Is(be.Err(), errBaz))
})
}

func TestBatchError_Add(t *testing.T) {
var be BatchError

// Test adding nil errors
be.Add(nil, nil)
assert.False(t, be.NotNil(), "Expected BatchError to be empty after adding nil errors")

// Test adding non-nil errors
err1 := errors.New("error 1")
err2 := errors.New("error 2")
be.Add(err1, err2)
assert.True(t, be.NotNil(), "Expected BatchError to be non-empty after adding errors")

// Test adding a mix of nil and non-nil errors
err3 := errors.New("error 3")
be.Add(nil, err3, nil)
assert.True(t, be.NotNil(), "Expected BatchError to be non-empty after adding a mix of nil and non-nil errors")
}

func TestBatchError_Err(t *testing.T) {
var be BatchError

// Test Err() on empty BatchError
assert.Nil(t, be.Err(), "Expected nil error for empty BatchError")

// Test Err() with multiple errors
err1 := errors.New("error 1")
err2 := errors.New("error 2")
be.Add(err1, err2)

combinedErr := be.Err()
assert.NotNil(t, combinedErr, "Expected nil error for BatchError with multiple errors")

// Check if the combined error contains both error messages
errString := combinedErr.Error()
assert.Truef(t, errors.Is(combinedErr, err1), "Combined error doesn't contain first error: %s", errString)
assert.Truef(t, errors.Is(combinedErr, err2), "Combined error doesn't contain second error: %s", errString)
}

func TestBatchError_NotNil(t *testing.T) {
var be BatchError

// Test NotNil() on empty BatchError
assert.Nil(t, be.Err(), "Expected nil error for empty BatchError")

// Test NotNil() after adding an error
be.Add(errors.New("test error"))
assert.NotNil(t, be.Err(), "Expected non-nil error after adding an error")
}
2 changes: 1 addition & 1 deletion tools/goctl/compare/compare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -94,4 +94,4 @@ else
echo "a diff found"
execute_command "diff -r $OLD_CODE $NEW_CODE"
exit 1
fi
fi
2 changes: 1 addition & 1 deletion tools/goctl/compare/rpc/test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,4 @@ service Test_Service {
rpc ClientStream (stream Req) returns (Reply);
// stream
rpc Stream(stream Req) returns (stream Reply);
}
}
2 changes: 1 addition & 1 deletion tools/goctl/rpc/generator/test.proto
Original file line number Diff line number Diff line change
Expand Up @@ -62,4 +62,4 @@ service Test_Service {
rpc ClientStream (stream Req) returns (Reply);
// stream
rpc Stream(stream Req) returns (stream Reply);
}
}