feat ExistStack: Add a public method to determine if the error has been appended to the stack. #239
Conversation
non-intrusive: Add a public method to determine if the error has been appended to the stack. Avoid adding the stack repeatedly for errors.
|
fix: #210 |
|
fix: #102 |
errors.go
Outdated
| if _, ok := err.(causer); !ok { | ||
| return false | ||
| } | ||
| return ExistStack(err.Cause()) |
There was a problem hiding this comment.
Here, you attempt to call the receiver method Cause on a thing of type error, which does not expose that implementation.
| type causer interface { | ||
| Cause() error | ||
| } |
There was a problem hiding this comment.
Just checking Cause() error ignores the possibility of an Unwrap() error which is kind of the preferred way of unwrapping errors now. So, this would only work with errors from this package, and those designed specifically for pre-unwrap compatibility with this package, which is no longer a strong argument since go1.13.
errors.go
Outdated
| if _, ok := err.(causer); !ok { | ||
| return false | ||
| } | ||
| return ExistStack(err.Cause()) |
There was a problem hiding this comment.
🤔 Not sure if the Go compiler is smart enough to rework end-recursion into the more efficient for-loop.
add ExistStack test
|
@puellanivis already fix |
| if value, ok := err.(causer); ok { | ||
| return ExistStack(err.Cause()) | ||
| } else { | ||
| return false |
There was a problem hiding this comment.
Every code path in the preceding if … { … } block returns from the function, therefore there is no reason to use an else block.
Considering using:
cause, ok := err.(causer)
if !ok {
return false
}
return ExistStack(cause.Cause())
Please check the implementation of Cause(err error) below, and try and mirror that as much as possible.
There was a problem hiding this comment.
Thank you very much for your help
| want bool | ||
| }{ | ||
| {io.EOF, false}, | ||
| {Wrap(io.EOF, "read error"), true}, |
There was a problem hiding this comment.
Missing test cases: WithStack, WithMessage, and errors.New.
|
Thank you for this but I do not intend to merge it. I encourage you to fork the package to add additional methods. Thank you |
non-intrusive:
Add a public method to determine if the error has been appended to the stack.
Avoid adding the stack repeatedly for errors.