Skip to content

Commit

Permalink
all: properly propagate fmt.Errorf errors + use errors.New
Browse files Browse the repository at this point in the history
Changes usages of fmt.Errorf that tried to propagate
errors using format specifiers "%s" or "%v", and unnecessarily
invoked err.Error() to pass into "%s"
While here, replaced some instances of

    fmt.Errorf(string)

with

    errors.New(string)

Fixes #16536
  • Loading branch information
odeke-em committed Jun 15, 2023
1 parent 5385116 commit fe41652
Show file tree
Hide file tree
Showing 28 changed files with 75 additions and 68 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### Improvements

* (all) [#16537](https://github.com/cosmos/cosmos-sdk/pull/16537) Properly propagated fmt.Errorf errors + using errors.New where appropriate.
* (all) [#16497](https://github.com/cosmos/cosmos-sdk/pull/16497) Removed all exported vestiges of `sdk.MustSortJSON` and `sdk.SortJSON`.

### API Breaking Changes
Expand Down
10 changes: 5 additions & 5 deletions client/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,21 @@ func ReadFromClientConfig(ctx client.Context) (client.Context, error) {
// when config.toml does not exist create and init with default values
if _, err := os.Stat(configFilePath); os.IsNotExist(err) {
if err := os.MkdirAll(configPath, os.ModePerm); err != nil {
return ctx, fmt.Errorf("couldn't make client config: %v", err)
return ctx, fmt.Errorf("couldn't make client config: %w", err)
}

if ctx.ChainID != "" {
conf.ChainID = ctx.ChainID // chain-id will be written to the client.toml while initiating the chain.
}

if err := writeConfigToFile(configFilePath, conf); err != nil {
return ctx, fmt.Errorf("could not write client config to the file: %v", err)
return ctx, fmt.Errorf("could not write client config to the file: %w", err)
}
}

conf, err := getClientConfig(configPath, ctx.Viper)
if err != nil {
return ctx, fmt.Errorf("couldn't get client config: %v", err)
return ctx, fmt.Errorf("couldn't get client config: %w", err)
}
// we need to update KeyringDir field on Client Context first cause it is used in NewKeyringFromBackend
ctx = ctx.WithOutputFormat(conf.Output).
Expand All @@ -78,15 +78,15 @@ func ReadFromClientConfig(ctx client.Context) (client.Context, error) {

keyring, err := client.NewKeyringFromBackend(ctx, conf.KeyringBackend)
if err != nil {
return ctx, fmt.Errorf("couldn't get key ring: %v", err)
return ctx, fmt.Errorf("couldn't get key ring: %w", err)
}

ctx = ctx.WithKeyring(keyring)

// https://github.com/cosmos/cosmos-sdk/issues/8986
client, err := client.NewClientFromNode(conf.Node)
if err != nil {
return ctx, fmt.Errorf("couldn't get client from nodeURI: %v", err)
return ctx, fmt.Errorf("couldn't get client from nodeURI: %w", err)
}

ctx = ctx.WithNodeURI(conf.Node).
Expand Down
2 changes: 1 addition & 1 deletion client/keys/add.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,7 +311,7 @@ func printCreate(cmd *cobra.Command, k *keyring.Record, showMnemonic bool, mnemo
// print mnemonic unless requested not to.
if showMnemonic {
if _, err := fmt.Fprintf(cmd.ErrOrStderr(), "\n**Important** write this mnemonic phrase in a safe place.\nIt is the only way to recover your account if you ever forget your password.\n\n%s\n", mnemonic); err != nil {
return fmt.Errorf("failed to print mnemonic: %v", err)
return fmt.Errorf("failed to print mnemonic: %w", err)
}
}
case flags.OutputFormatJSON:
Expand Down
12 changes: 6 additions & 6 deletions client/keys/show.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {
if len(args) == 1 {
k, err = fetchKey(clientCtx.Keyring, args[0])
if err != nil {
return fmt.Errorf("%s is not a valid name or address: %v", args[0], err)
return fmt.Errorf("%s is not a valid name or address: %w", args[0], err)
}
} else {
pks := make([]cryptotypes.PubKey, len(args))
for i, keyref := range args {
k, err := fetchKey(clientCtx.Keyring, keyref)
if err != nil {
return fmt.Errorf("%s is not a valid name or address: %v", keyref, err)
return fmt.Errorf("%s is not a valid name or address: %w", keyref, err)
}
key, err := k.GetPubKey()
if err != nil {
Expand Down Expand Up @@ -142,15 +142,15 @@ func runShowCmd(cmd *cobra.Command, args []string) (err error) {

if isShowDevice {
if isShowPubKey {
return fmt.Errorf("the device flag (-d) can only be used for addresses not pubkeys")
return errors.New("the device flag (-d) can only be used for addresses not pubkeys")
}
if bechPrefix != "acc" {
return fmt.Errorf("the device flag (-d) can only be used for accounts")
return errors.New("the device flag (-d) can only be used for accounts")
}

// Override and show in the device
if k.GetType() != keyring.TypeLedger {
return fmt.Errorf("the device flag (-d) can only be used for accounts stored in devices")
return errors.New("the device flag (-d) can only be used for accounts stored in devices")
}

ledgerItem := k.GetLedger()
Expand Down Expand Up @@ -190,7 +190,7 @@ func fetchKey(kb keyring.Keyring, keyref string) (*keyring.Record, error) {

func validateMultisigThreshold(k, nKeys int) error {
if k <= 0 {
return fmt.Errorf("threshold must be a positive integer")
return errors.New("threshold must be a positive integer")
}
if nKeys < k {
return fmt.Errorf(
Expand Down
15 changes: 8 additions & 7 deletions crypto/armor.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package crypto
import (
"bytes"
"encoding/hex"
"errors"
"fmt"
"io"

Expand Down Expand Up @@ -104,7 +105,7 @@ func UnarmorInfoBytes(armorStr string) ([]byte, error) {
func UnarmorPubKeyBytes(armorStr string) (bz []byte, algo string, err error) {
bz, header, err := unarmorBytes(armorStr, blockTypePubKey)
if err != nil {
return nil, "", fmt.Errorf("couldn't unarmor bytes: %v", err)
return nil, "", fmt.Errorf("couldn't unarmor bytes: %w", err)
}

switch header[headerVersion] {
Expand All @@ -117,7 +118,7 @@ func UnarmorPubKeyBytes(armorStr string) (bz []byte, algo string, err error) {

return bz, header[headerType], err
case "":
return nil, "", fmt.Errorf("header's version field is empty")
return nil, "", errors.New("header's version field is empty")
default:
err = fmt.Errorf("unrecognized version: %v", header[headerVersion])
return nil, "", err
Expand Down Expand Up @@ -192,12 +193,12 @@ func UnarmorDecryptPrivKey(armorStr, passphrase string) (privKey cryptotypes.Pri
}

if header["salt"] == "" {
return privKey, "", fmt.Errorf("missing salt bytes")
return privKey, "", errors.New("missing salt bytes")
}

saltBytes, err := hex.DecodeString(header["salt"])
if err != nil {
return privKey, "", fmt.Errorf("error decoding salt: %v", err.Error())
return privKey, "", fmt.Errorf("error decoding salt: %w", err)
}

privKey, err = decryptPrivKey(saltBytes, encBytes, passphrase, header[kdfHeader])
Expand Down Expand Up @@ -261,15 +262,15 @@ func EncodeArmor(blockType string, headers map[string]string, data []byte) strin
buf := new(bytes.Buffer)
w, err := armor.Encode(buf, blockType, headers)
if err != nil {
panic(fmt.Errorf("could not encode ascii armor: %s", err))
panic(fmt.Errorf("could not encode ascii armor: %v", err))
}
_, err = w.Write(data)
if err != nil {
panic(fmt.Errorf("could not encode ascii armor: %s", err))
panic(fmt.Errorf("could not encode ascii armor: %v", err))
}
err = w.Close()
if err != nil {
panic(fmt.Errorf("could not encode ascii armor: %s", err))
panic(fmt.Errorf("could not encode ascii armor: %v", err))
}
return buf.String()
}
Expand Down
2 changes: 1 addition & 1 deletion crypto/ledger/ledger_mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func (mock LedgerSECP256K1Mock) GetAddressPubKeySECP256K1(derivationPath []uint3
// re-serialize in the 33-byte compressed format
cmp, err := secp.ParsePubKey(pk)
if err != nil {
return nil, "", fmt.Errorf("error parsing public key: %v", err)
return nil, "", fmt.Errorf("error parsing public key: %w", err)
}

compressedPublicKey := make([]byte, csecp256k1.PubKeySize)
Expand Down
14 changes: 7 additions & 7 deletions crypto/ledger/ledger_secp256k1.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func ShowAddress(path hd.BIP44Params, expectedPubKey types.PubKey, accountAddres
}

if !pubKey.Equals(expectedPubKey) {
return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
return errors.New("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
}

pubKey2, _, err := getPubKeyAddrSafe(device, path, accountAddressPrefix)
Expand All @@ -179,7 +179,7 @@ func ShowAddress(path hd.BIP44Params, expectedPubKey types.PubKey, accountAddres
}

if !pubKey2.Equals(expectedPubKey) {
return fmt.Errorf("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
return errors.New("the key's pubkey does not match with the one retrieved from Ledger. Check that the HD path and device are the correct ones")
}

return nil
Expand Down Expand Up @@ -274,7 +274,7 @@ func validateKey(device SECP256K1, pkl PrivKeyLedgerSecp256k1) error {

// verify this matches cached address
if !pub.Equals(pkl.CachedPubKey) {
return fmt.Errorf("cached key does not match retrieved key")
return errors.New("cached key does not match retrieved key")
}

return nil
Expand Down Expand Up @@ -316,13 +316,13 @@ func sign(device SECP256K1, pkl PrivKeyLedgerSecp256k1, msg []byte, p2 byte) ([]
func getPubKeyUnsafe(device SECP256K1, path hd.BIP44Params) (types.PubKey, error) {
publicKey, err := device.GetPublicKeySECP256K1(path.DerivationPath())
if err != nil {
return nil, fmt.Errorf("please open the %v app on the Ledger device - error: %v", options.appName, err)
return nil, fmt.Errorf("please open the %v app on the Ledger device - error: %w", options.appName, err)
}

// re-serialize in the 33-byte compressed format
cmp, err := secp.ParsePubKey(publicKey)
if err != nil {
return nil, fmt.Errorf("error parsing public key: %v", err)
return nil, fmt.Errorf("error parsing public key: %w", err)
}

compressedPublicKey := make([]byte, secp256k1.PubKeySize)
Expand All @@ -340,13 +340,13 @@ func getPubKeyUnsafe(device SECP256K1, path hd.BIP44Params) (types.PubKey, error
func getPubKeyAddrSafe(device SECP256K1, path hd.BIP44Params, hrp string) (types.PubKey, string, error) {
publicKey, addr, err := device.GetAddressPubKeySECP256K1(path.DerivationPath(), hrp)
if err != nil {
return nil, "", fmt.Errorf("%w: address rejected for path %s", err, path.String())
return nil, "", fmt.Errorf("%w: address rejected for path %s", err, path)
}

// re-serialize in the 33-byte compressed format
cmp, err := secp.ParsePubKey(publicKey)
if err != nil {
return nil, "", fmt.Errorf("error parsing public key: %v", err)
return nil, "", fmt.Errorf("error parsing public key: %w", err)
}

compressedPublicKey := make([]byte, secp256k1.PubKeySize)
Expand Down
3 changes: 2 additions & 1 deletion depinject/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package depinject

import (
"bytes"
stderrors "errors"
"fmt"
"reflect"

Expand Down Expand Up @@ -441,7 +442,7 @@ func (c *container) build(loc Location, outputs ...interface{}) error {
Outputs: nil,
Fn: func(values []reflect.Value) ([]reflect.Value, error) {
if len(values) != len(outputs) {
return nil, fmt.Errorf("internal error, unexpected number of values")
return nil, stderrors.New("internal error, unexpected number of values")
}

for i, output := range outputs {
Expand Down
8 changes: 4 additions & 4 deletions server/grpc/gogoreflection/serverreflection.go
Original file line number Diff line number Diff line change
Expand Up @@ -202,12 +202,12 @@ func (s *serverReflectionServer) fileDescForType(st reflect.Type) (*dpb.FileDesc
func decodeFileDesc(enc []byte) (*dpb.FileDescriptorProto, error) {
raw, err := decompress(enc)
if err != nil {
return nil, fmt.Errorf("failed to decompress enc: %v", err)
return nil, fmt.Errorf("failed to decompress enc: %w", err)
}

fd := new(dpb.FileDescriptorProto)
if err := proto.Unmarshal(raw, fd); err != nil {
return nil, fmt.Errorf("bad descriptor: %v", err)
return nil, fmt.Errorf("bad descriptor: %w", err)
}
return fd, nil
}
Expand All @@ -216,11 +216,11 @@ func decodeFileDesc(enc []byte) (*dpb.FileDescriptorProto, error) {
func decompress(b []byte) ([]byte, error) {
r, err := gzip.NewReader(bytes.NewReader(b))
if err != nil {
return nil, fmt.Errorf("bad gzipped descriptor: %v", err)
return nil, fmt.Errorf("bad gzipped descriptor: %w", err)
}
out, err := io.ReadAll(r)
if err != nil {
return nil, fmt.Errorf("bad gzipped descriptor: %v", err)
return nil, fmt.Errorf("bad gzipped descriptor: %w", err)
}
return out, nil
}
Expand Down
2 changes: 1 addition & 1 deletion server/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ func startStandAlone(svrCtx *Context, app types.Application, opts StartCmdOption
cmtApp := NewCometABCIWrapper(app)
svr, err := server.NewServer(addr, transport, cmtApp)
if err != nil {
return fmt.Errorf("error creating listener: %v", err)
return fmt.Errorf("error creating listener: %w", err)
}

svr.SetLogger(servercmtlog.CometLoggerWrapper{Logger: svrCtx.Logger.With("module", "abci-server")})
Expand Down
5 changes: 3 additions & 2 deletions tools/confix/cmd/diff.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cmd

import (
"errors"
"fmt"

"cosmossdk.io/tools/confix"
Expand All @@ -24,7 +25,7 @@ func DiffCommand() *cobra.Command {
case clientCtx.HomeDir != "":
filename = fmt.Sprintf("%s/config/app.toml", clientCtx.HomeDir)
default:
return fmt.Errorf("must provide a path to the app.toml file")
return errors.New("must provide a path to the app.toml file")
}

targetVersion := args[0]
Expand All @@ -39,7 +40,7 @@ func DiffCommand() *cobra.Command {

rawFile, err := confix.LoadConfig(filename)
if err != nil {
return fmt.Errorf("failed to load config: %v", err)
return fmt.Errorf("failed to load config: %w", err)
}

diff := confix.DiffValues(rawFile, targetVersionFile)
Expand Down
5 changes: 3 additions & 2 deletions tools/confix/cmd/migrate.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package cmd

import (
"context"
"errors"
"fmt"

"cosmossdk.io/tools/confix"
Expand Down Expand Up @@ -33,7 +34,7 @@ In case of any error in updating the file, no output is written.`,
case clientCtx.HomeDir != "":
filename = fmt.Sprintf("%s/config/app.toml", clientCtx.HomeDir)
default:
return fmt.Errorf("must provide a path to the app.toml file")
return errors.New("must provide a path to the app.toml file")
}

targetVersion := args[0]
Expand All @@ -44,7 +45,7 @@ In case of any error in updating the file, no output is written.`,

rawFile, err := confix.LoadConfig(filename)
if err != nil {
return fmt.Errorf("failed to load config: %v", err)
return fmt.Errorf("failed to load config: %w", err)
}

ctx := context.Background()
Expand Down
2 changes: 1 addition & 1 deletion tools/confix/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ func LoadLocalConfig(name string) (*tomledit.Document, error) {
func LoadConfig(path string) (*tomledit.Document, error) {
f, err := os.Open(path)
if err != nil {
return nil, fmt.Errorf("failed to open %q: %v", path, err)
return nil, fmt.Errorf("failed to open %q: %w", path, err)
}
defer f.Close()

Expand Down
8 changes: 4 additions & 4 deletions tools/confix/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,24 +35,24 @@ func Upgrade(ctx context.Context, plan transform.Plan, configPath, outputPath st

doc, err := LoadConfig(configPath)
if err != nil {
return fmt.Errorf("loading config: %v", err)
return fmt.Errorf("loading config: %w", err)
}

// transforms doc and reports whether it succeeded.
if err := plan.Apply(ctx, doc); err != nil {
return fmt.Errorf("updating %q: %v", configPath, err)
return fmt.Errorf("updating %q: %w", configPath, err)
}

var buf bytes.Buffer
if err := tomledit.Format(&buf, doc); err != nil {
return fmt.Errorf("formatting config: %v", err)
return fmt.Errorf("formatting config: %w", err)
}

// allow to skip validation
if !skipValidate {
// verify that file is valid after applying fixes
if err := CheckValid(configPath, buf.Bytes()); err != nil {
return fmt.Errorf("updated config is invalid: %v", err)
return fmt.Errorf("updated config is invalid: %w", err)
}
}

Expand Down
2 changes: 1 addition & 1 deletion tools/rosetta/client_online.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ func (c *Client) Balances(ctx context.Context, addr string, height *int64) ([]*r
func (c *Client) BlockByHash(ctx context.Context, hash string) (crgtypes.BlockResponse, error) {
bHash, err := hex.DecodeString(hash)
if err != nil {
return crgtypes.BlockResponse{}, fmt.Errorf("invalid block hash: %s", err)
return crgtypes.BlockResponse{}, fmt.Errorf("invalid block hash: %w", err)
}

block, err := c.tmRPC.BlockByHash(ctx, bHash)
Expand Down
2 changes: 1 addition & 1 deletion types/dec_coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -634,7 +634,7 @@ func ParseDecCoin(coinStr string) (coin DecCoin, err error) {
}

if err := ValidateDenom(denomStr); err != nil {
return DecCoin{}, fmt.Errorf("invalid denom cannot contain spaces: %s", err)
return DecCoin{}, fmt.Errorf("invalid denom cannot contain spaces: %w", err)
}

return NewDecCoinFromDec(denomStr, amount), nil
Expand Down
Loading

0 comments on commit fe41652

Please sign in to comment.