Decide rules of paust-db error handling

[dragon0170]

errors 패키지( https://github.com/pkg/errors )를 사용해 error 처리 방식을 통일하면 좋을 것 같습니다.

errors 패키지의 Wrap function을 사용하면 기존의 error value를 변경하지 않고 새로운 failure path context를 추가해 디버깅에 도움이 됩니다.

• 예시 (tendermint의 httpclient.go)

  
  func (c *HTTP) BroadcastTxSync(tx types.Tx) (*ctypes.ResultBroadcastTx, error) {
  return c.broadcastTX("broadcast_tx_sync", tx)
  }

func (c HTTP) broadcastTX(route string, tx types.Tx) (ctypes.ResultBroadcastTx, error) { result := new(ctypes.ResultBroadcastTx) _, err := c.rpc.Call(route, map[string]interface{}{"tx": tx}, result) if err != nil { return nil, errors.Wrap(err, route) } return result, nil }

* error 출력 예시
```bash
broadcast_tx_sync: Post http://localhost:26657: dial tcp [::1]:26657: connect: connection refused

위 tendermint의 예시처럼 paust-db도 error를 call stack의 위에서 아래로 전달하고 가장 아래의 call에서 error를 출력하는 방식을 사용할 것으로 압니다.

따라서 function A 내에서 function B를 호출하고 function B의 error값을 이용해 function A의 error값으로 사용하게 될 경우, function A 내에서 errors.Wrap 함수를 통해 function A 함수에 대한 context를 추가하면 될 것 같습니다.

paust-db error handling의 format, 규칙 등에 대한 다양한 의견을 말해주시면 좋겠습니다.

[elon0823]

이런식으로 PaustDB 전용 custom error handling 을 처리하면 어떨까 해서 간단히 작성해봤습니다. Errors 패키지에 Wrap을 사용해도 좋을 것 같은데 다 에러 구조가 다르기 때문에 PaustDB 전용 에러 구조를 하나 두고 거기다가 다른 에러들을 담아서 리턴해주도록 하면 좋을 것 같습니다.

package main
import (
    "fmt"
)

type PaustDBError struct {
    msg    string // description of error
    code int  // error cpde
}

func (e *PaustDBError) Error() (msg string, code int) { 
    msg = e.msg
    code = e.code

    return 
}

func someFunc(testCode int) *PaustDBError {

    if testCode > 0 {
        err := &PaustDBError{msg:"some error occured", code:200}    
        return err
    }

    return nil
}
func main() {

    if error := someFunc(1); error != nil {
        fmt.Println(error.Error())
    }
}

[dragon0170]

https://github.com/paust-team/paust-db/issues/58#issuecomment-452591973 paust-db용 error를 두는 것은 좋은 것 같습니다.

기존에 있는 go의 error interface를 활용하기 위해서 아래 코드와 같이 변경하여 사용하는 것은 어떨까요?

package main
import (
    "fmt"
    "strconv"
)

type PaustDBError struct {
    msg    string // description of error
    code int  // error cpde
}

func (e PaustDBError) Error() string { 
    code := strconv.Itoa(e.code)

    return code + ": " + e.msg
}

func someFunc(testCode int) error {

    if testCode > 0 {
        err := PaustDBError{msg:"some error occured", code:200} 
        return err
    }

    return nil
}
func main() {

    if error := someFunc(1); error != nil {
        fmt.Println(error)
    }
}

[elon0823]

https://github.com/paust-team/paust-db/issues/58#issuecomment-452597310

아 그렇네요 go Error Interface 의 Error() 를 구현해놨으니까 그렇게 사용하면되겠네요. 좋습니다

[kwjooo]

https://github.com/paust-team/paust-db/issues/58#issuecomment-452597310 Error method를 fmt.Sprintf로 수정하면 더 깔끔할 것 같습니다.

package main
import (
    "fmt"
    "strconv"
)

type PaustDBError struct {
    msg    string // description of error
    code int  // error code
}

func (e PaustDBError) Error() string { 
    return fmt.Sprintf("Error code : %d, message : %s", e.code, e.msg)
}

func someFunc(testCode int) error {

    if testCode > 0 {
        err := PaustDBError{msg:"some error occured", code:200} 
        return err
    }

    return nil
}
func main() {

    if error := someFunc(1); error != nil {
        fmt.Println(error)
    }
}

[elon0823]

wrap 과 interface 부분을 잘 조합해서 아래와 같이 쓰는건 어떨까요? 굳이 interface 부분을 가지고 가야할지도 고민되는 부분이긴 하지만, paust db endpoint 에서는 동일한 format 의 error 형태를 제공하고 싶어서요.

package main

import (
    "fmt"
    "github.com/pkg/errors"
)

type PaustDBError struct {
    code    int  // error code
    err     error
}

func (e PaustDBError) Error() string { 
    return fmt.Sprintf("Error code : %d,\nmessage : %s,\ntrace : %+v\n", e.code, e.err, errors.Cause(e.err))
}

// private methods or nested methods
// 내부 함수나 타 라이브러리를 통해 출력되는 error 에 대해선 wrap 을 통해 return 하도록 
func inner_origin() error {
    e := errors.New("origin error message")
    return e
}

func inner2() error {
    return errors.Wrap(inner_origin(), "middle")
}

func inner() error {
    return errors.Wrap(inner2(), "outer")
}

// public method 
// end point 에서 제공하는 공개메소드는 PaustDBError interface 를 통해 return 함

func MyFunction() error {

    if err := inner(); err!= nil {
        return PaustDBError{code:500, err:err}  
    }

    return nil  
}

func main() {

    if err := MyFunction(); err != nil {
        fmt.Println(err)
    }   
}

그럼 아래와 같은 output 을 볼 수 있습니다.

Error code : 500, message : outer: middle: origin error message, trace : origin error message main.inner_origin /private/var/folders/zl/2fr5b9xn3tn9_qrr60jr7lzm0000gn/T/CodeRunner/Go/Untitled.go:20 main.inner2 /private/var/folders/zl/2fr5b9xn3tn9_qrr60jr7lzm0000gn/T/CodeRunner/Go/Untitled.go:26 main.inner /private/var/folders/zl/2fr5b9xn3tn9_qrr60jr7lzm0000gn/T/CodeRunner/Go/Untitled.go:30 main.myFunction /private/var/folders/zl/2fr5b9xn3tn9_qrr60jr7lzm0000gn/T/CodeRunner/Go/Untitled.go:35 main.main /private/var/folders/zl/2fr5b9xn3tn9_qrr60jr7lzm0000gn/T/CodeRunner/Go/Untitled.go:45 runtime.main /usr/local/Cellar/go/1.11.4/libexec/src/runtime/proc.go:201 runtime.goexit /usr/local/Cellar/go/1.11.4/libexec/src/runtime/asm_amd64.s:1333

[elon0823]

위의 comment 는 논의 결과 wrap 만 사용하도록 결정했습니다. 원래 의도한건 Error 를 위한 것 보단 paust db 를 위한 common Response 형태를 만들고자 함이었는데 당장엔 필요하지 않을 것 같습니다.

[kwjooo]

error handling 시 errors.Wrap 혹은 errors.New를 사용하고 message를 담을 텐데, 이 때 error message의 통일성을 갖추는 것은 어떨까요? 이는 사용자들이 한층 더 에러를 파악하기 쉽게 하는데 목적이 있습니다. 예를 들어 errors.Wrap의 메시지의 경우 "Error executing Key method"와 같이 명사형으로 만들고, errors.New 메시지의 경우 "iterator is invalid"와 같이 동사형으로 만드는 것입니다.

@dragon0170 @elon0823 @co1god @code-to-gold