nit: panic with error #4741
nit: panic with error #4741
Conversation
crodriguezvega
requested review from
colin-axner,
AdityaSripal and
damiannolan
as code owners
|
Doing a quick search I noticed we are relatively inconsistent with panicking on strings vs errors, maybe we could turn them all into errors? Looks like a ~20+ panics using strings |
|
For the instances @chatton mentions: |
|
@DimitrisJim with next level grep-fu as usual :D |
crodriguezvega
added
the
type: code hygiene
crodriguezvega
requested review from
srdtrk,
charleenfei,
chatton and
DimitrisJim
as code owners
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| @@ -56,7 +56,7 @@ | |||
| func (h Height) Compare(other exported.Height) int64 { | |||
| height, ok := other.(Height) | |||
| if !ok { | |||
| panic(fmt.Sprintf("cannot compare against invalid height type: %T. expected height type: %T", other, h)) | |||
| panic(fmt.Errorf("cannot compare against invalid height type: %T. expected height type: %T", other, h)) | |||
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| @@ -178,7 +178,7 @@ | |||
| revision, err := strconv.ParseUint(splitStr[len(splitStr)-1], 10, 64) | |||
| // sanity check: error should always be nil since regex only allows numbers in last element | |||
| if err != nil { | |||
| panic(fmt.Sprintf("regex allowed non-number value as last split element for chainID: %s", chainID)) | |||
| panic(fmt.Errorf("regex allowed non-number value as last split element for chainID: %s", chainID)) | |||
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #4741 +/- ##
=======================================
Coverage 79.49% 79.49%
=======================================
Files 189 189
Lines 13285 13285
=======================================
Hits 10561 10561
Misses 2296 2296
Partials 428 428
|
| @@ -425,7 +426,7 @@ | |||
| store := ctx.KVStore(k.storeKey) | |||
| bz := store.Get([]byte(types.ParamsKey)) | |||
| if bz == nil { // only panic on unset params and not on empty params | |||
| panic("client params are not set in store") | |||
| panic(errors.New("client params are not set in store")) | |||
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| @@ -52,99 +54,99 @@ | |||
|
|
|||
| // ClientType panics! | |||
| func (ClientState) ClientType() string { | |||
| panic("legacy solo machine is deprecated!") | |||
| panic(errors.New("legacy solo machine is deprecated")) | |||
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| } | ||
|
|
||
| // Status panics! | ||
| func (ClientState) Status(_ sdk.Context, _ storetypes.KVStore, _ codec.BinaryCodec) exported.Status { | ||
| panic("legacy solo machine is deprecated!") | ||
| panic(errors.New("legacy solo machine is deprecated")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| } | ||
|
|
||
| // UpdateState panis! | ||
| func (*ClientState) UpdateState(_ sdk.Context, _ codec.BinaryCodec, _ storetypes.KVStore, _ exported.ClientMessage) []exported.Height { | ||
| panic("legacy solo machine is deprecated!") | ||
| panic(errors.New("legacy solo machine is deprecated")) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
Yeah, I am going to leave those two. They are in tests, I don't think we need to be that consistent. :) |
Description
closes: #XXXX
Commit Message / Changelog Entry
see the guidelines for commit messages. (view raw markdown for examples)
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/).godoccomments.Files changedin the Github PR explorer.Codecov Reportin the comment section below once CI passes.