Browse Source

Remvoe errors package from libs (#4157)

*libs/common/errors: remove package

- remove errors file from cmn pkg

- use errorf instead of wrap in async function

- add changelog entry

- closes #3862
pull/4162/head
Marko 5 years ago
committed by GitHub
parent
commit
aca94fd9a0
No known key found for this signature in database GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 38 additions and 347 deletions
  1. +24
    -20
      CHANGELOG_PENDING.md
  2. +3
    -1
      libs/common/async.go
  3. +6
    -5
      libs/common/async_test.go
  4. +0
    -214
      libs/common/errors.go
  5. +0
    -101
      libs/common/errors_test.go
  6. +2
    -3
      lite/base_verifier.go
  7. +3
    -3
      lite/proxy/query.go

+ 24
- 20
CHANGELOG_PENDING.md View File

@ -11,6 +11,7 @@ program](https://hackerone.com/tendermint).
### BREAKING CHANGES:
- CLI/RPC/Config
- [rpc] \#3471 Paginate `/validators` response (default: 30 vals per page)
- [rpc] \#3188 Remove `BlockMeta` in `ResultBlock` in favor of `BlockId` for `/block`
- [rpc] `/block_results` response format updated (see RPC docs for details)
@ -28,54 +29,57 @@ program](https://hackerone.com/tendermint).
}
}
```
- [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) Remove `#event` suffix from the ID in event responses.
- [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) Remove `#event` suffix from the ID in event responses.
`{"jsonrpc": "2.0", "id": 0, "result": ...}`
- [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) Switch to integer IDs instead of `json-client-XYZ`
- [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) Switch to integer IDs instead of `json-client-XYZ`
```
id=0 method=/subscribe
id=0 result=...
id=1 method=/abci_query
id=1 result=...
```
* ID is unique for each request;
* Request.ID is now optional. Notification is a Request without an ID. Previously ID="" or ID=0 were considered as notifications.
- ID is unique for each request;
- Request.ID is now optional. Notification is a Request without an ID. Previously ID="" or ID=0 were considered as notifications.
- Apps
- Go API
- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) `Query#(Matches|Conditions)` returns an error.
- [libs/pubsub][\#4070](https://github.com/tendermint/tendermint/pull/4070) `Query#(Matches|Conditions)` returns an error.
- [rpc/client] \#3471 `Validators` now requires two more args: `page` and `perPage`
- [libs/common] \#3262 Make error the last parameter of `Task` (@PSalant726)
- [libs/common] \#3862 Remove `errors.go` from `libs/common`
- Blockchain Protocol
- [abci] \#2521 Remove `TotalTxs` and `NumTxs` from `Header`
- [types] [\#4151](https://github.com/tendermint/tendermint/pull/4151) Enforce ordering of votes in DuplicateVoteEvidence to be lexicographically sorted on BlockID
- P2P Protocol
- [p2p] [\3668](https://github.com/tendermint/tendermint/pull/3668) Make `SecretConnection` non-malleable
- [p2p][\3668](https://github.com/tendermint/tendermint/pull/3668) Make `SecretConnection` non-malleable
### FEATURES:
### IMPROVEMENTS:
- [mempool] [\#4083](https://github.com/tendermint/tendermint/pull/4083) Added TxInfo parameter to CheckTx(), and removed CheckTxWithInfo() (@erikgrinaker)
- [mempool] [\#4057](https://github.com/tendermint/tendermint/issues/4057) Include peer ID when logging rejected txns (@erikgrinaker)
- [tools] [\#4023](https://github.com/tendermint/tendermint/issues/4023) Improved `tm-monitor` formatting of start time and avg tx throughput (@erikgrinaker)
- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) No longer panic in `Query#(Matches|Conditions)` preferring to return an error instead.
- [libs/pubsub] [\#4070](https://github.com/tendermint/tendermint/pull/4070) Strip out non-numeric characters when attempting to match numeric values.
- [p2p] [\#3991](https://github.com/tendermint/tendermint/issues/3991) Log "has been established or dialed" as debug log instead of Error for connected peers (@whunmr)
- [rpc] [\#4077](https://github.com/tendermint/tendermint/pull/4077) Added support for `EXISTS` clause to the Websocket query interface.
- [mempool][\#4083](https://github.com/tendermint/tendermint/pull/4083) Added TxInfo parameter to CheckTx(), and removed CheckTxWithInfo() (@erikgrinaker)
- [mempool][\#4057](https://github.com/tendermint/tendermint/issues/4057) Include peer ID when logging rejected txns (@erikgrinaker)
- [tools][\#4023](https://github.com/tendermint/tendermint/issues/4023) Improved `tm-monitor` formatting of start time and avg tx throughput (@erikgrinaker)
- [libs/pubsub][\#4070](https://github.com/tendermint/tendermint/pull/4070) No longer panic in `Query#(Matches|Conditions)` preferring to return an error instead.
- [libs/pubsub][\#4070](https://github.com/tendermint/tendermint/pull/4070) Strip out non-numeric characters when attempting to match numeric values.
- [p2p][\#3991](https://github.com/tendermint/tendermint/issues/3991) Log "has been established or dialed" as debug log instead of Error for connected peers (@whunmr)
- [rpc][\#4077](https://github.com/tendermint/tendermint/pull/4077) Added support for `EXISTS` clause to the Websocket query interface.
- [privval] Add `SignerDialerEndpointRetryWaitInterval` option (@cosmostuba)
- [crypto] Add `RegisterKeyType` to amino to allow external key types registration (@austinabell)
- [rpc] \#3188 Added `block_size` to `BlockMeta` this is reflected in `/blockchain`
- [types] \#2521 Add `NumTxs` to `BlockMeta` and `EventDataNewBlockHeader`
- [docs] [\#4111](https://github.com/tendermint/tendermint/issues/4111) Replaced dead whitepaper link in README.md
- [docs][\#4111](https://github.com/tendermint/tendermint/issues/4111) Replaced dead whitepaper link in README.md
### BUG FIXES:
- [tools] [\#4023](https://github.com/tendermint/tendermint/issues/4023) Refresh `tm-monitor` health when validator count is updated (@erikgrinaker)
- [state] [\#4104](https://github.com/tendermint/tendermint/pull/4104) txindex/kv: Fsync data to disk immediately after receiving it (@guagualvcha)
- [state] [\#4095](https://github.com/tendermint/tendermint/pull/4095) txindex/kv: Return an error if there's one when the user searches for a tx (hash=X) (@hsyis)
- [rpc/lib] [\#4051](https://github.com/tendermint/tendermint/pull/4131) Fix RPC client, which was previously resolving https protocol to http (@yenkhoon)
- [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) JSONRPCClient: validate that Response.ID matches Request.ID
- [rpc] [\#4141](https://github.com/tendermint/tendermint/pull/4141) WSClient: check for unsolicited responses
- [tools][\#4023](https://github.com/tendermint/tendermint/issues/4023) Refresh `tm-monitor` health when validator count is updated (@erikgrinaker)
- [state][\#4104](https://github.com/tendermint/tendermint/pull/4104) txindex/kv: Fsync data to disk immediately after receiving it (@guagualvcha)
- [state][\#4095](https://github.com/tendermint/tendermint/pull/4095) txindex/kv: Return an error if there's one when the user searches for a tx (hash=X) (@hsyis)
- [rpc/lib][\#4051](https://github.com/tendermint/tendermint/pull/4131) Fix RPC client, which was previously resolving https protocol to http (@yenkhoon)
- [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) JSONRPCClient: validate that Response.ID matches Request.ID
- [rpc][\#4141](https://github.com/tendermint/tendermint/pull/4141) WSClient: check for unsolicited responses

+ 3
- 1
libs/common/async.go View File

@ -2,6 +2,8 @@ package common
import (
"sync/atomic"
"github.com/pkg/errors"
)
//----------------------------------------
@ -142,7 +144,7 @@ func Parallel(tasks ...Task) (trs *TaskResultSet, ok bool) {
if pnk := recover(); pnk != nil {
atomic.AddInt32(numPanics, 1)
// Send panic to taskResultCh.
taskResultCh <- TaskResult{nil, ErrorWrap(pnk, "Panic in task")}
taskResultCh <- TaskResult{nil, errors.Errorf("panic in task %v", pnk)}
// Closing taskResultCh lets trs.Wait() work.
close(taskResultCh)
// Decrement waitgroup.


+ 6
- 5
libs/common/async_test.go View File

@ -1,12 +1,12 @@
package common
import (
"errors"
"fmt"
"sync/atomic"
"testing"
"time"
"github.com/pkg/errors"
"github.com/stretchr/testify/assert"
)
@ -125,20 +125,21 @@ func TestParallelRecover(t *testing.T) {
// Verify task #0, #1, #2.
checkResult(t, taskResultSet, 0, 0, nil, nil)
checkResult(t, taskResultSet, 1, 1, errors.New("some error"), nil)
checkResult(t, taskResultSet, 2, nil, nil, 2)
checkResult(t, taskResultSet, 2, nil, nil, errors.Errorf("panic in task %v", 2).Error())
}
// Wait for result
func checkResult(t *testing.T, taskResultSet *TaskResultSet, index int, val interface{}, err error, pnk interface{}) {
func checkResult(t *testing.T, taskResultSet *TaskResultSet, index int,
val interface{}, err error, pnk interface{}) {
taskResult, ok := taskResultSet.LatestResult(index)
taskName := fmt.Sprintf("Task #%v", index)
assert.True(t, ok, "TaskResultCh unexpectedly closed for %v", taskName)
assert.Equal(t, val, taskResult.Value, taskName)
switch {
case err != nil:
assert.Equal(t, err, taskResult.Error, taskName)
assert.Equal(t, err.Error(), taskResult.Error.Error(), taskName)
case pnk != nil:
assert.Equal(t, pnk, taskResult.Error.(Error).Data(), taskName)
assert.Equal(t, pnk, taskResult.Error.Error(), taskName)
default:
assert.Nil(t, taskResult.Error, taskName)
}


+ 0
- 214
libs/common/errors.go View File

@ -1,214 +0,0 @@
package common
import (
"fmt"
"runtime"
)
//----------------------------------------
// Convenience method.
func ErrorWrap(cause interface{}, format string, args ...interface{}) Error {
if causeCmnError, ok := cause.(*cmnError); ok { //nolint:gocritic
msg := fmt.Sprintf(format, args...)
return causeCmnError.Stacktrace().Trace(1, msg)
} else if cause == nil {
return newCmnError(FmtError{format, args}).Stacktrace()
} else {
// NOTE: causeCmnError is a typed nil here.
msg := fmt.Sprintf(format, args...)
return newCmnError(cause).Stacktrace().Trace(1, msg)
}
}
//----------------------------------------
// Error & cmnError
/*
Usage with arbitrary error data:
```go
// Error construction
type MyError struct{}
var err1 error = NewErrorWithData(MyError{}, "my message")
...
// Wrapping
var err2 error = ErrorWrap(err1, "another message")
if (err1 != err2) { panic("should be the same")
...
// Error handling
switch err2.Data().(type){
case MyError: ...
default: ...
}
```
*/
type Error interface {
Error() string
Stacktrace() Error
Trace(offset int, format string, args ...interface{}) Error
Data() interface{}
}
// New Error with formatted message.
// The Error's Data will be a FmtError type.
func NewError(format string, args ...interface{}) Error {
err := FmtError{format, args}
return newCmnError(err)
}
// New Error with specified data.
func NewErrorWithData(data interface{}) Error {
return newCmnError(data)
}
type cmnError struct {
data interface{} // associated data
msgtraces []msgtraceItem // all messages traced
stacktrace []uintptr // first stack trace
}
var _ Error = &cmnError{}
// NOTE: do not expose.
func newCmnError(data interface{}) *cmnError {
return &cmnError{
data: data,
msgtraces: nil,
stacktrace: nil,
}
}
// Implements error.
func (err *cmnError) Error() string {
return fmt.Sprintf("%v", err)
}
// Captures a stacktrace if one was not already captured.
func (err *cmnError) Stacktrace() Error {
if err.stacktrace == nil {
var offset = 3
var depth = 32
err.stacktrace = captureStacktrace(offset, depth)
}
return err
}
// Add tracing information with msg.
// Set n=0 unless wrapped with some function, then n > 0.
func (err *cmnError) Trace(offset int, format string, args ...interface{}) Error {
msg := fmt.Sprintf(format, args...)
return err.doTrace(msg, offset)
}
// Return the "data" of this error.
// Data could be used for error handling/switching,
// or for holding general error/debug information.
func (err *cmnError) Data() interface{} {
return err.data
}
func (err *cmnError) doTrace(msg string, n int) Error {
pc, _, _, _ := runtime.Caller(n + 2) // +1 for doTrace(). +1 for the caller.
// Include file & line number & msg.
// Do not include the whole stack trace.
err.msgtraces = append(err.msgtraces, msgtraceItem{
pc: pc,
msg: msg,
})
return err
}
func (err *cmnError) Format(s fmt.State, verb rune) {
switch verb {
case 'p':
s.Write([]byte(fmt.Sprintf("%p", &err)))
default:
if s.Flag('#') {
s.Write([]byte("--= Error =--\n"))
// Write data.
s.Write([]byte(fmt.Sprintf("Data: %#v\n", err.data)))
// Write msg trace items.
s.Write([]byte(fmt.Sprintf("Msg Traces:\n")))
for i, msgtrace := range err.msgtraces {
s.Write([]byte(fmt.Sprintf(" %4d %s\n", i, msgtrace.String())))
}
// Write stack trace.
if err.stacktrace != nil {
s.Write([]byte(fmt.Sprintf("Stack Trace:\n")))
for i, pc := range err.stacktrace {
fnc := runtime.FuncForPC(pc)
file, line := fnc.FileLine(pc)
s.Write([]byte(fmt.Sprintf(" %4d %s:%d\n", i, file, line)))
}
}
s.Write([]byte("--= /Error =--\n"))
} else {
// Write msg.
s.Write([]byte(fmt.Sprintf("%v", err.data)))
}
}
}
//----------------------------------------
// stacktrace & msgtraceItem
func captureStacktrace(offset int, depth int) []uintptr {
var pcs = make([]uintptr, depth)
n := runtime.Callers(offset, pcs)
return pcs[0:n]
}
type msgtraceItem struct {
pc uintptr
msg string
}
func (mti msgtraceItem) String() string {
fnc := runtime.FuncForPC(mti.pc)
file, line := fnc.FileLine(mti.pc)
return fmt.Sprintf("%s:%d - %s",
file, line,
mti.msg,
)
}
//----------------------------------------
// fmt error
/*
FmtError is the data type for NewError() (e.g. NewError().Data().(FmtError))
Theoretically it could be used to switch on the format string.
```go
// Error construction
var err1 error = NewError("invalid username %v", "BOB")
var err2 error = NewError("another kind of error")
...
// Error handling
switch err1.Data().(cmn.FmtError).Format() {
case "invalid username %v": ...
case "another kind of error": ...
default: ...
}
```
*/
type FmtError struct {
format string
args []interface{}
}
func (fe FmtError) Error() string {
return fmt.Sprintf(fe.format, fe.args...)
}
func (fe FmtError) String() string {
return fmt.Sprintf("FmtError{format:%v,args:%v}",
fe.format, fe.args)
}
func (fe FmtError) Format() string {
return fe.format
}

+ 0
- 101
libs/common/errors_test.go View File

@ -1,101 +0,0 @@
package common
import (
fmt "fmt"
"testing"
"github.com/stretchr/testify/assert"
)
func TestErrorPanic(t *testing.T) {
type pnk struct {
msg string
}
capturePanic := func() (err Error) {
defer func() {
if r := recover(); r != nil {
err = ErrorWrap(r, "This is the message in ErrorWrap(r, message).")
}
}()
panic(pnk{"something"})
}
var err = capturePanic()
assert.Equal(t, pnk{"something"}, err.Data())
assert.Equal(t, "{something}", fmt.Sprintf("%v", err))
assert.Contains(t, fmt.Sprintf("%#v", err), "This is the message in ErrorWrap(r, message).")
assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0")
}
func TestErrorWrapSomething(t *testing.T) {
var err = ErrorWrap("something", "formatter%v%v", 0, 1)
assert.Equal(t, "something", err.Data())
assert.Equal(t, "something", fmt.Sprintf("%v", err))
assert.Regexp(t, `formatter01\n`, fmt.Sprintf("%#v", err))
assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0")
}
func TestErrorWrapNothing(t *testing.T) {
var err = ErrorWrap(nil, "formatter%v%v", 0, 1)
assert.Equal(t,
FmtError{"formatter%v%v", []interface{}{0, 1}},
err.Data())
assert.Equal(t, "formatter01", fmt.Sprintf("%v", err))
assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`)
assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0")
}
func TestErrorNewError(t *testing.T) {
var err = NewError("formatter%v%v", 0, 1)
assert.Equal(t,
FmtError{"formatter%v%v", []interface{}{0, 1}},
err.Data())
assert.Equal(t, "formatter01", fmt.Sprintf("%v", err))
assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`)
assert.NotContains(t, fmt.Sprintf("%#v", err), "Stack Trace")
}
func TestErrorNewErrorWithStacktrace(t *testing.T) {
var err = NewError("formatter%v%v", 0, 1).Stacktrace()
assert.Equal(t,
FmtError{"formatter%v%v", []interface{}{0, 1}},
err.Data())
assert.Equal(t, "formatter01", fmt.Sprintf("%v", err))
assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`)
assert.Contains(t, fmt.Sprintf("%#v", err), "Stack Trace:\n 0")
}
func TestErrorNewErrorWithTrace(t *testing.T) {
var err = NewError("formatter%v%v", 0, 1)
err.Trace(0, "trace %v", 1)
err.Trace(0, "trace %v", 2)
err.Trace(0, "trace %v", 3)
assert.Equal(t,
FmtError{"formatter%v%v", []interface{}{0, 1}},
err.Data())
assert.Equal(t, "formatter01", fmt.Sprintf("%v", err))
assert.Contains(t, fmt.Sprintf("%#v", err), `Data: common.FmtError{format:"formatter%v%v", args:[]interface {}{0, 1}}`)
dump := fmt.Sprintf("%#v", err)
assert.NotContains(t, dump, "Stack Trace")
assert.Regexp(t, `common/errors_test\.go:[0-9]+ - trace 1`, dump)
assert.Regexp(t, `common/errors_test\.go:[0-9]+ - trace 2`, dump)
assert.Regexp(t, `common/errors_test\.go:[0-9]+ - trace 3`, dump)
}
func TestErrorWrapError(t *testing.T) {
var err1 error = NewError("my message")
var err2 error = ErrorWrap(err1, "another message")
assert.Equal(t, err1, err2)
}

+ 2
- 3
lite/base_verifier.go View File

@ -5,7 +5,6 @@ import (
"github.com/pkg/errors"
cmn "github.com/tendermint/tendermint/libs/common"
lerr "github.com/tendermint/tendermint/lite/errors"
"github.com/tendermint/tendermint/types"
)
@ -46,13 +45,13 @@ func (bv *BaseVerifier) Verify(signedHeader types.SignedHeader) error {
// We can't verify commits for a different chain.
if signedHeader.ChainID != bv.chainID {
return cmn.NewError("BaseVerifier chainID is %v, cannot verify chainID %v",
return errors.Errorf("BaseVerifier chainID is %v, cannot verify chainID %v",
bv.chainID, signedHeader.ChainID)
}
// We can't verify commits older than bv.height.
if signedHeader.Height < bv.height {
return cmn.NewError("BaseVerifier height is %v, cannot verify height %v",
return errors.Errorf("BaseVerifier height is %v, cannot verify height %v",
bv.height, signedHeader.Height)
}


+ 3
- 3
lite/proxy/query.go View File

@ -24,7 +24,7 @@ func GetWithProof(prt *merkle.ProofRuntime, key []byte, reqHeight int64, node rp
val cmn.HexBytes, height int64, proof *merkle.Proof, err error) {
if reqHeight < 0 {
err = cmn.NewError("Height cannot be negative")
err = errors.New("Height cannot be negative")
return
}
@ -54,7 +54,7 @@ func GetWithProofOptions(prt *merkle.ProofRuntime, path string, key []byte, opts
// Validate the response, e.g. height.
if resp.IsErr() {
err = cmn.NewError("Query error for key %d: %d", key, resp.Code)
err = errors.Errorf("Query error for key %d: %d", key, resp.Code)
return nil, err
}
@ -62,7 +62,7 @@ func GetWithProofOptions(prt *merkle.ProofRuntime, path string, key []byte, opts
return nil, lerr.ErrEmptyTree()
}
if resp.Height == 0 {
return nil, cmn.NewError("Height returned is zero")
return nil, errors.New("Height returned is zero")
}
// AppHash for height H is in header H+1


Loading…
Cancel
Save