Conversation
common/errors.go
Outdated
| TraceAt(n int, format string, a ...interface{}) Error | ||
| Cause() interface{} | ||
| WithType(t interface{}) Error | ||
| Type() interface{} |
There was a problem hiding this comment.
Is there a better name? Switcher?
switch err.Type().(type) is redundant...
switch err.Switcher().(type) is also redundant...
There was a problem hiding this comment.
I'd vote for the first option (maybe even shorten it to err.T().(type). go is redundant language (obj.(Object).SomeMethod).
There was a problem hiding this comment.
I like "T" a lot. It differentiates it from the "type" keyword.
Codecov Report
@@ Coverage Diff @@
## develop #180 +/- ##
===========================================
+ Coverage 61.25% 62.43% +1.18%
===========================================
Files 60 60
Lines 4617 4651 +34
===========================================
+ Hits 2828 2904 +76
+ Misses 1563 1524 -39
+ Partials 226 223 -3
Continue to review full report at Codecov.
|
common/errors.go
Outdated
| // Convenience methods | ||
|
|
||
| // ErrorWrap will just call .TraceAt(), or create a new *cmnError. | ||
| func ErrorWrap(cause interface{}, format string, a ...interface{}) Error { |
| //---------------------------------------- | ||
| // Error & cmnError | ||
|
|
||
| /* |
There was a problem hiding this comment.
why not move this to errors subpackage? then it can be errors.Wrap
There was a problem hiding this comment.
tmlibs errors? I'm down for that.
There was a problem hiding this comment.
well, because cmn is convenient... if we want to have cmn, then it's more convenient to do cmn.ErrorWrap, than importing another module.
There was a problem hiding this comment.
I'd argue that Go2 should support Error natively... so I'd want it in cmn.
There was a problem hiding this comment.
Yeah.. cmn.ErrorWrap is fine too
common/errors.go
Outdated
| TraceCause(cause error, format string, a ...interface{}) Error | ||
| Cause() error | ||
| Type() interface{} | ||
| TraceAt(n int, format string, a ...interface{}) Error |
There was a problem hiding this comment.
TraceFrom(level int or TraceFrom(depth int
common/errors.go
Outdated
| TraceAt(n int, format string, a ...interface{}) Error | ||
| Cause() interface{} | ||
| WithType(t interface{}) Error | ||
| Type() interface{} |
There was a problem hiding this comment.
I'd vote for the first option (maybe even shorten it to err.T().(type). go is redundant language (obj.(Object).SomeMethod).
common/errors.go
Outdated
| traces []traceItem | ||
| msg string // first msg which also appears in msg | ||
| cause interface{} // underlying cause (or panic object) | ||
| type_ interface{} // for switching on error |
There was a problem hiding this comment.
can't use "type" as field name
common/errors.go
Outdated
| // Captures a stacktrace if one was not already captured. | ||
| func (err *cmnError) Stacktrace() Error { | ||
| if err.stacktrace == nil { | ||
| err.stacktrace = captureStacktrace(3, 32) |
There was a problem hiding this comment.
the 3 I think arguably should be magic, though it deserves comments. I'll move the 32 out.
There was a problem hiding this comment.
you don't have to move them out, you can do
var offset = 3
const defaultDepth = 32
inside the function
This replaces pkg/errors.
Intent is so we can call cmn.ErrorWrap instead of errors.Wrap.
This should be more performant, as it doesn't require capturing a stacktrace each wrap.