Minor Error Message Cleanup #3691
Conversation
alexanderbez
added
WIP
Type: Code Hygiene
alexanderbez
requested review from
alessio and
jackzampolin
and removed request for
alessio and
jackzampolin
Codecov Report
@@ Coverage Diff @@
## develop #3691 +/- ##
==========================================
- Coverage 61.21% 61.2% -0.02%
==========================================
Files 190 190
Lines 13995 13995
==========================================
- Hits 8567 8565 -2
- Misses 4892 4894 +2
Partials 536 536 |
alexanderbez
requested review from
cwgoes,
ebuchman and
rigelrozanski
as code owners
alexanderbez
requested review from
fedekunze,
alessio and
jackzampolin
and removed request for
ebuchman
alexanderbez
added
the
T: API Breaking
| Code int `json:"code,omitempty"` | ||
| Message string `json:"message"` | ||
| Code int `json:"code,omitempty"` | ||
| Error string `json:"error"` |
| } | ||
| return acc.GetSequence(), nil | ||
| } | ||
|
|
||
| func (ak AccountKeeper) setSequence(ctx sdk.Context, addr sdk.AccAddress, newSequence uint64) sdk.Error { | ||
| acc := ak.GetAccount(ctx, addr) | ||
| if acc == nil { | ||
| return sdk.ErrUnknownAddress(addr.String()) | ||
| return sdk.ErrUnknownAddress(fmt.Sprintf("account %s does not exist", addr)) |
There was a problem hiding this comment.
Should the ErrUnknownAddress take an addr and print out the error in the correct format?
There was a problem hiding this comment.
Sure, but that would be break the general convention of these error functions where they all just take a simple msg string param.
There was a problem hiding this comment.
yeah, not really crazy about that pattern. It seems to leave a lot of subtly different errors for the same issues. I think we should standardize these, but that may be out of the scope of the PR.
| @@ -232,7 +234,7 @@ func queryDelegatorValidators(ctx sdk.Context, cdc *codec.Codec, req abci.Reques | |||
|
|
|||
| errRes := cdc.UnmarshalJSON(req.Data, ¶ms) | |||
| if errRes != nil { | |||
| return []byte{}, sdk.ErrUnknownAddress("") | |||
| return []byte{}, sdk.ErrInternal(fmt.Sprintf("failed to parse params: %s", err)) | |||
There was a problem hiding this comment.
Maybe sdk.ErrFailedParams or something similar?
There was a problem hiding this comment.
Hmmm, I'm not sure that is totally a good idea as we could have a multitude of decoding failures and we don't want a unique failure type/function for reach. Maybe ErrFailedDecode but that can be for another PR.
|
Just a pending fix then I will merge. |
|
Fixed pending @jackzampolin |
Targeted PR against correct branch (see CONTRIBUTING.md)
Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
Wrote tests
Updated relevant documentation (
docs/)Added entries in
PENDING.mdwith issue #rereviewed
Files changedin the github PR explorerFor Admin Use: