*: refactor error system #216
Conversation
amyangfei
added
priority/important
Codecov Report
@@ Coverage Diff @@
## master #216 +/- ##
================================================
+ Coverage 59.1362% 59.5102% +0.3739%
================================================
Files 123 125 +2
Lines 14240 14374 +134
================================================
+ Hits 8421 8554 +133
- Misses 4957 4959 +2
+ Partials 862 861 -1 |
|
/run-all-tests |
pkg/terror/terror.go
Outdated
| ClassBinlogOp: "binlog-op", | ||
| ClassCheckpoint: "checkpoint", | ||
| ClassTaskCheck: "task-check", | ||
| ClassRelayAPI: "relay-api", |
pkg/terror/terror.go
Outdated
| } | ||
|
|
||
| // levelGeneratef is an inner interface to generate new *Error | ||
| func (e *Error) levelGeneratef(stackSkipLevel int, format string, args ...interface{}) error { |
There was a problem hiding this comment.
When I unveiled its mysterious veil, then I fell into meditation
| if !ok { | ||
| return errors.Annotatef(err, format, args...) | ||
| } | ||
| e.message = fmt.Sprintf("%s: %s", format, e.getMsg()) |
There was a problem hiding this comment.
should it be e.message = e.message + ": " + fmt.Sprintf(fmt, args)?
There was a problem hiding this comment.
nope, this keeps the same as errors.Annotatef we used in pingcap/errors
such as https://github.com/pingcap/errors/blob/master/format_test.go#L140-L154
pkg/terror/terror.go
Outdated
|
|
||
| // Generatef generates a new *Error with the same class and code, and a new formatted message. | ||
| func (e *Error) Generatef(format string, args ...interface{}) error { | ||
| return e.levelGeneratef(1, format, args...) |
There was a problem hiding this comment.
discard old message directly? or make it like annotatef?
There was a problem hiding this comment.
Generate creates a new error with arguments while Generatef replaces the error message and arguments. They share the same error code, error status, but may have different error messages.
|
/run-all-tests |
| case <-ctx.Done(): | ||
| return errors.Trace(ctx.Err()) | ||
| return ctx.Err() |
There was a problem hiding this comment.
do we need add error code for context canceled or deadline?
There was a problem hiding this comment.
yep, most of the ctx.Err() are used to check context cancel or timeout and not exposed to users. I didn't wrap ctx.Err() with new error in the consideration of less code logic change. Besides ctx.Err() here is returned in a GRPC round trip, the context cancel behavior is in the client-side, maybe using a new error with call stack changes nothing. We may refine this later.
csuzhangxc
added
status/LGT1
WangXiangUSTC
added
status/LGT2
What problem does this PR solve?
implement error system based on RFC introduced in #211
What is changed and how it works?
add new error library
classify errors and use new error library for all error handling, classification is working in progress:
Check List
Tests
Code changes
Side effects
Related changes