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

Only support Error.Is with better algorithm #381

Closed
ethanfrey opened this issue Mar 6, 2019 · 0 comments
Closed

Only support Error.Is with better algorithm #381

ethanfrey opened this issue Mar 6, 2019 · 0 comments
Assignees

Comments

@ethanfrey
Copy link
Contributor

Based on part of #368

Replace Is function with Error.Is method.

Currently to check an error type, Is function must be used.

if _, err := doThings(); errors.Is(errors.ErrNotFound, err) {
    println("not found")
}

After making Is a method, above example would be

if _, err := doThings(); errors.ErrNotFound.Is(err) {
    println("not found")
}
  • Current implementation can not support internal error check.
  • Using a method would make it clear what we are comparing with (nicer notation?).
  • Comparison with ErrInternal would be possible to implement (requires more changes).

Change how errors comparison is implemented

Current implementation is mostly comparing the ABCI Code value. This is limiting the error testing and prevents from implementing ErrInternal comparison.
Instead of the current implementation the following one could be used.

// Is returns true if given error or its cause is the same kind.
// If cause error provides Cause method then a comparison is made with all
// parents as well.
func (kind *Error) Is(err error) bool {
    type causer interface {
        Cause() error
    }
    for {
        if err == kind {
            return true
        }
        if e, ok := err.(causer); ok {
            err = e.Cause()
        } else {
            return false
        }
    }
}

Our error instances are global and therefore comparable as instances. Using the above mechanism would also allow to "inherit" errors. Below can be true.

root := Wrap(nil, "root")
child1 := Wrap(root, "child one")
child2 := Wrap(root, "child two")

fmt.Println("child 1 is root", root.Is(child1))
// child 1 is root true

fmt.Println("child 2 is root", root.Is(child2))
// child 2 is root true

fmt.Println("root is child 1", child1.Is(root))
// root is child 1 false

fmt.Println("child 2 is child 1", child1.Is(child2))
// child 2 is child 1 false

These can both be tackled together and since we always compare against an intention (and not "any random error is ErrInternal"), this makes more sense

@husio husio self-assigned this Mar 11, 2019
husio added a commit that referenced this issue Mar 11, 2019
Instead of errors.Is errors.Error.Is method is now used.

All code and tests were updated accordingly.

resolve #381
@husio husio closed this as completed in e757e20 Mar 12, 2019
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

No branches or pull requests

2 participants