From bf6740f4742b4fe6949d332211afc78a5a2dd52b Mon Sep 17 00:00:00 2001 From: Shubham Date: Mon, 4 Aug 2025 16:07:02 +0530 Subject: [PATCH 1/2] fix: improve OopsError.Is() method to properly handle error comparisons This commit improves the Is() method to correctly handle error comparisons without causing panics. The key improvements are: 1. Compare underlying errors when both OopsError instances have them 2. Only compare messages when neither has underlying errors 3. Handle mixed cases (one with underlying error, one without) correctly 4. Add proper recursive error chain traversal via errors.Is() 5. Remove redundant checks and simplify comparison logic 6. Add comprehensive test coverage for all comparison scenarios The fix ensures that: - errors.Is(wrappedErr, originalErr) works correctly for wrapped oops errors - Multiple levels of wrapping are handled properly - Different error instances with same messages are correctly distinguished - Standard error comparisons work as expected - No panics occur during error comparisons All existing tests pass, ensuring backward compatibility. --- error.go | 22 +++++++++++++++++++++- error_test.go | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/error.go b/error.go index e134092..99f89ec 100644 --- a/error.go +++ b/error.go @@ -2,6 +2,7 @@ package oops import ( "encoding/json" + "errors" "fmt" "log/slog" "net/http" @@ -83,7 +84,26 @@ func (o OopsError) Unwrap() error { // Is checks if this error matches the target error. // This method implements the errors.Is interface for error comparison. func (c OopsError) Is(err error) bool { - return c.err == err + // If the target is also an OopsError, compare their underlying errors + if target, ok := err.(OopsError); ok { + // If both have underlying errors, compare them + if c.err != nil && target.err != nil { + return errors.Is(c.err, target.err) + } + // If both have no underlying errors, compare messages + if c.err == nil && target.err == nil { + return c.msg == target.msg + } + // If one has underlying error and other doesn't, they're different + return false + } + + // For non-OopsError targets, check if our underlying error matches + if c.err != nil { + return errors.Is(c.err, err) + } + + return false } // Error returns the error message without additional context. diff --git a/error_test.go b/error_test.go index 81e5195..10101ca 100644 --- a/error_test.go +++ b/error_test.go @@ -66,3 +66,52 @@ func TestErrorsAs(t *testing.T) { }, "Error: %w", assert.AnError) is.True(errors.As(err, &target)) } + +// TestErrorsIsWithOopsErrors tests the fix for comparing wrapped oops errors without panics +func TestErrorsIsWithOopsErrors(t *testing.T) { + is := assert.New(t) + + // Test case 1: Compare wrapped oops error with original oops error + // This was the main use case that was causing panics + originalErr := New("user not found") + wrappedErr := In("User/GetByEmail").Wrap(originalErr) + + // This should not panic and should return true + is.True(errors.Is(wrappedErr, originalErr)) + + // Test case 2: Compare multiple levels of wrapping + level1Err := New("database error") + level2Err := In("Repository").Wrap(level1Err) + level3Err := In("Service").Wrap(level2Err) + + is.True(errors.Is(level3Err, level1Err)) + is.True(errors.Is(level3Err, level2Err)) + + // Test case 3: Compare with different oops errors (should return false) + differentErr := New("different error") + is.False(errors.Is(wrappedErr, differentErr)) + + // Test case 4: Compare wrapped oops error with standard error + standardErr := errors.New("user not found") + wrappedStandardErr := In("User/GetByEmail").Wrap(standardErr) + is.True(errors.Is(wrappedStandardErr, standardErr)) + + // Test case 5: Compare oops error with wrapped standard error + oopsErr := New("user not found") + wrappedStandardErr2 := In("User/GetByEmail").Wrap(standardErr) + is.False(errors.Is(wrappedStandardErr2, oopsErr)) // Different error types + + // Test case 6: Test with nil errors + is.False(errors.Is(wrappedErr, nil)) + is.False(errors.Is(nil, originalErr)) + + // Test case 7: Test with same message but different instances + // Note: New() creates different underlying error instances, so they shouldn't match + err1 := New("same message") + err2 := New("same message") + is.False(errors.Is(err1, err2)) // Different instances, should not match + + // Test case 8: Test with different messages + err3 := New("different message") + is.False(errors.Is(err1, err3)) +} From f3ce9be08c9eae0ffe8f525ad72f80b5661df10e Mon Sep 17 00:00:00 2001 From: Shubham Date: Mon, 4 Aug 2025 16:54:46 +0530 Subject: [PATCH 2/2] fix: update module path to github.com/MathGaps/oops --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 651d872..45cf8b3 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/samber/oops +module github.com/MathGaps/oops go 1.21

AltStyle によって変換されたページ (->オリジナル) /