Skip to content

feat: Add IdentifierStartsWith#27

Merged
anthony-treuillier-scality merged 1 commit into
mainfrom
improvement/add-IdentifierStartsWith
May 27, 2026
Merged

feat: Add IdentifierStartsWith#27
anthony-treuillier-scality merged 1 commit into
mainfrom
improvement/add-IdentifierStartsWith

Conversation

@anthony-treuillier-scality
Copy link
Copy Markdown
Contributor

@anthony-treuillier-scality anthony-treuillier-scality commented May 26, 2026

IdentifierStartsWith allows error comparison with a prefix. Useful when matching specific errors with HTTP Code in API handlers

Example: If e.Identifier: "3-2-1"

IdentifierStartsWith(e, "3-2") // return true
IdentifierStartsWith(e, "2-1") // return false

@anthony-treuillier-scality anthony-treuillier-scality requested a review from a team as a code owner May 26, 2026 09:49
@anthony-treuillier-scality anthony-treuillier-scality force-pushed the improvement/add-IdentifierStartsWith branch from 4af34f8 to 14661e4 Compare May 26, 2026 09:54
Comment thread errors_test.go Outdated
Expect(IdentifierStartsWith(e, "1")).To(BeTrue())
})

It("should return true when error's identifier starts with the prefix", func() {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add some tests with error Identifier = 31-22-12 and test with the following prefix:

  • 3
  • 31
  • 31-2
  • 31-22
  • 31-22-1
  • 31-22-12

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For sure, I can, but what is the difference with these tests:

It("should return true when error's identifier starts with the prefix", func() {
	e1_1 := Wrap(ErrForbidden, WithIdentifier(1))
	e1_2 := Wrap(e1_1, WithIdentifier(2))
	e1_3 := Wrap(e1_2, WithIdentifier(3))
	Expect(IdentifierStartsWith(e1_3, "3")).To(BeTrue())
	Expect(IdentifierStartsWith(e1_3, "3-2")).To(BeTrue())
	Expect(IdentifierStartsWith(e1_3, "3-2-1")).To(BeTrue())
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to know if you expect false for 3, 31-2 and 31-21-1 Meaning ig you treat the prefix as a pure string or if the value in dash means something

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some changes to catch edge cases

Comment thread errors.go Outdated
Comment on lines +181 to +189
// IdentifierStartsWith checks if the error's identifier starts with the given prefix.
func IdentifierStartsWith(err error, prefix string) bool {
var e *Error
if ok := errors.As(err, &e); !ok {
return false
}
return strings.HasPrefix(e.GetIdentifier(), prefix)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since Identifier is a slice of int32, we can do this:

func IdentifierStartsWith(err error, prefix ...uint32) bool {
        var e *Error

        if !errors.As(err, &e) {
                return false
        }

        if len(prefix) > len(e.Identifier) {
                return false
        }

        n := len(e.Identifier)
        for i, p := range prefix {
                if e.Identifier[n-1-i] != p {
                        return false
                }
        }

        return true
 }

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found another implementation based on string. Your solution should work, but it seems to me more complicated as we have to deal with the reverse order

Comment thread errors_test.go Outdated
Comment on lines +449 to +454
It("should return true when error's identifier starts with the prefix", func() {
e := Wrap(ErrForbidden, WithIdentifier(1))
Expect(IdentifierStartsWith(e, "1")).To(BeTrue())
})

It("should return true when error's identifier starts with the prefix", func() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests have the same title

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change this

Comment thread errors_test.go Outdated
Comment on lines +463 to +468
It("should return false when error's identifier does not start with the prefix", func() {
e := Wrap(ErrForbidden, WithIdentifier(1))
Expect(IdentifierStartsWith(e, "2")).To(BeFalse())
})

It("should return false when error's identifier does not start with the prefix", func() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same, we can deduplicate

Comment thread README.md
Comment on lines +185 to +195
### Specific use-case with IdentifierStartsWith()

`IdentifierStartsWith(error, prefix)` checks whether this error's identifier, formatted as a string, starts with the given prefix.

For example:
If e.Identifier: "3-2-1", then
```go
IdentifierStartsWith(e, "3-2") return True
IdentifierStartsWith(e, "2-1") return False
```

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe to explain the differences between Is() and IdentifierStartsWith()

 e := Wrap(Wrap(Wrap(Base, WithIdentifier(1)), WithIdentifier(2)), WithIdentifier(3))
 IdentifierStartsWith(e, 3, 2) // true — matches the most recent wraps (outermost)
 e.Is(parent)                  // matches an ancestor (innermost)

@anthony-treuillier-scality anthony-treuillier-scality force-pushed the improvement/add-IdentifierStartsWith branch 2 times, most recently from 644e9ef to 10e3c1b Compare May 26, 2026 15:21
Comment thread errors.go Outdated
// As is a wrapper around errors.As to check if the error is of a specific type.
func As(err error, target any) bool { return errors.As(err, target) }

// IdentifierStartsWith checks if the error's identifier starts with the given prefix.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// IdentifierStartsWith checks if the error's identifier starts with the given prefix.
// IdentifierStartsWith checks if the error's identifier string starts with the given prefix.

@anthony-treuillier-scality anthony-treuillier-scality force-pushed the improvement/add-IdentifierStartsWith branch from 10e3c1b to 2a30dd3 Compare May 26, 2026 15:44
Signed-off-by: Anthony TREUILLIER <anthony.treuillier@scality.com>
@anthony-treuillier-scality anthony-treuillier-scality force-pushed the improvement/add-IdentifierStartsWith branch from 2a30dd3 to 81f2317 Compare May 26, 2026 15:45
@anthony-treuillier-scality anthony-treuillier-scality merged commit 9245b47 into main May 27, 2026
2 checks passed
@anthony-treuillier-scality anthony-treuillier-scality deleted the improvement/add-IdentifierStartsWith branch May 27, 2026 04:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants