-
Notifications
You must be signed in to change notification settings - Fork 489
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
using standard error code to replace terror (#982) #1033
Conversation
terror/terror.go
Outdated
if ec, has := rfcCode2errClass[string(rfcCode)[:index]]; has { | ||
class = ec | ||
} else { | ||
log.Warn("Unknown error class", zap.String("class", string(rfcCode)[:index])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to warn this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fixed, PTAL
terror/terror.go
Outdated
func (ec ErrClass) New(code ErrCode, message string) *Error { | ||
rfcCode := ec.initError(code) | ||
err := errors.Normalize(message, errors.MySQLErrorCode(int(code)), errors.RFCCodeText(rfcCode)) | ||
errCodeMap[code] = err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any concurrent problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, this method is not groutine-safe, but always used for global init.
terror/terror.go
Outdated
func (ec ErrClass) NewStdErr(code ErrCode, message string, desc string, workaround string) *Error { | ||
rfcCode := ec.initError(code) | ||
err := errors.Normalize(message, errors.MySQLErrorCode(int(code)), errors.RFCCodeText(rfcCode), errors.Description(desc), errors.Workaround(workaround)) | ||
errCodeMap[code] = err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
// NotEqual checks if err is not equal to e. | ||
func (e *Error) NotEqual(err error) bool { | ||
return !e.Equal(err) | ||
return errors.Normalize(message, errors.MySQLErrorCode(int(code)), errors.RFCCodeText(fmt.Sprintf("%s:%d", errClass2Desc[ec], code))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if errClass2Desc[ec]
is empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errClass2Desc[ec] will not be empty, because the caller has been init at error class register.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What problem does this PR solve?
cherry-pick #982 to release-4.0
According to RFC, TiDB need to adjust error format and using standard error to replace terror. The standard error has been implemented in https://github.com/pingcap/errors/tree/standard-error
What is changed and how it works?
Since standard error is very similar with
terror
. We can change very few api to replace theterror
. Just renameterror.Error
witherrors.Error
Check List
Tests
Code changes
Side effects
Related changes