jiacai2050 commented on code in PR #1504:
URL: 
https://github.com/apache/incubator-horaedb/pull/1504#discussion_r1535002291


##########
horaemeta/cmd/horaemeta-server/main.go:
##########
@@ -95,7 +95,7 @@ func main() {
                panicf("fail to init global logger, err:%v", err)
        }
        defer logger.Sync() //nolint:errcheck
-       log.Info(fmt.Sprintf("server start with version: %s", buildVersion()))
+       log.Info(fmt.Sprintf("server start with version:%s", buildVersion()))

Review Comment:
   ```suggestion
        log.Info("Start server", zap.String("version", buildVersion()))
   ```



##########
horaemeta/pkg/coderr/error.go:
##########
@@ -25,82 +25,135 @@ import (
        "github.com/pkg/errors"
 )
 
-var _ CodeError = &codeError{code: 0, desc: "", cause: nil}
+var _ CodeErrorDef = &codeErrorDef{code: 0, desc: ""}
+var _ CodeError = &codeError{code: 0, msg: "", hasCause: false, 
causeWithStack: nil}
 
-// CodeError is an error with code.
 type CodeError interface {
        error
        Code() Code
-       // WithCausef should generate a new CodeError instance with the 
provided cause details.
-       WithCausef(format string, a ...any) CodeError
+}
+
+// CodeErrorDef is an error definition, and able to generate a real CodeError
+type CodeErrorDef interface {
+       Code() Code
+       // WithMessagef should generate a new CodeError instance with the 
provided message.
+       WithMessagef(format string, a ...any) CodeError
        // WithCause should generate a new CodeError instance with the provided 
cause details.
        WithCause(cause error) CodeError
+       // WithCausef should generate a new CodeError instance with the 
provided cause details and error message.
+       WithCausef(cause error, format string, a ...any) CodeError
 }
 
 // Is checks whether the cause of `err` is the kind of error specified by the 
`expectCode`.
 // Returns false if the cause of `err` is not CodeError.
 func Is(err error, expectCode Code) bool {
-       code, b := GetCauseCode(err)
-       if b && code == expectCode {
+       code, ok := GetCauseCode(err)
+       if ok && code == expectCode {
                return true
        }
 
        return false
 }
 
-func GetCauseCode(err error) (Code, bool) {
-       if err == nil {
-               return Invalid, false
-       }
+// codeError is the default implementation of CodeError.
+type codeError struct {
+       code           Code
+       msg            string
+       causeWithStack error
+       hasCause       bool
+}
 
-       cause := errors.Cause(err)
-       cerr, ok := cause.(CodeError)
-       if !ok {
-               return Invalid, false
+func (e *codeError) Error() string {
+       if e.hasCause {
+               return fmt.Sprintf("[err_code=%d]%s, cause:%v", e.code, e.msg, 
e.causeWithStack)
+       } else {
+               return fmt.Sprintf("[err_code=%d]%s", e.code, e.msg)
        }
-       return cerr.Code(), true
 }
 
-// NewCodeError creates a base CodeError definition.
-// The provided code should be defined in the code.go in this package.
-func NewCodeError(code Code, desc string) CodeError {
-       return &codeError{
-               code:  code,
-               desc:  desc,
-               cause: nil,
-       }
+func (e *codeError) Code() Code {
+       return e.code
 }
 
-// codeError is the default implementation of CodeError.
-type codeError struct {
-       code  Code
-       desc  string
-       cause error
+// NewCodeErrorDef creates an CodeError definition.
+// The provided code should be defined in the code.go in this package.
+func NewCodeErrorDef(code Code, desc string) CodeErrorDef {
+       return &codeErrorDef{
+               code: code,
+               desc: desc,
+       }
 }
 
-func (e *codeError) Error() string {
-       return fmt.Sprintf("(#%d)%s, cause:%+v", e.code, e.desc, e.cause)
+// codeErrorDef is the default implementation of CodeErrorDef.
+type codeErrorDef struct {
+       code Code
+       desc string
 }
 
-func (e *codeError) Code() Code {
+func (e *codeErrorDef) Code() Code {
        return e.code
 }
 
-func (e *codeError) WithCausef(format string, a ...any) CodeError {
+func (e *codeErrorDef) WithMessagef(format string, a ...any) CodeError {
        errMsg := fmt.Sprintf(format, a...)
-       causeWithStack := errors.WithStack(errors.New(errMsg))
+       causeWithStack := errors.WithStack(errors.New(""))

Review Comment:
   IMO, this function should not add stack, users can use `WithCausef` to do 
this. Otherwise this function is same with `WithCausef`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to