Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

all: ensure propagated errors in fmt.Errorf errors are wrapped with %w and not %s nor %v #16536

Closed
odeke-em opened this issue Jun 14, 2023 · 0 comments · Fixed by #16537
Closed
Labels
T:Bug Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.

Comments

@odeke-em
Copy link
Collaborator

Summary of Bug

I ran this rough grep pattern git grep 'return .*fmt.Errorf.\+".\+err' | grep -v "test\|%w"
which expoosed these unpropagated values

$ git grep 'return .*fmt.Errorf.\+".\+err' | grep -v "test\|%w"
api/cosmos/app/runtime/v1alpha1/module.pulsar.go:					return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: wrong wireType = %d for field OverrideStoreKeys", wireType)
api/cosmos/bank/module/v1/module.pulsar.go:					return protoiface.UnmarshalOutput{NoUnkeyedLiterals: input.NoUnkeyedLiterals, Flags: input.Flags}, fmt.Errorf("proto: wrong wireType = %d for field BlockedModuleAccountsOverride", wireType)
client/config/config.go:			return ctx, fmt.Errorf("couldn't make client config: %v", err)
client/config/config.go:			return ctx, fmt.Errorf("could not write client config to the file: %v", err)
client/config/config.go:		return ctx, fmt.Errorf("couldn't get client config: %v", err)
client/config/config.go:		return ctx, fmt.Errorf("couldn't get key ring: %v", err)
client/config/config.go:		return ctx, fmt.Errorf("couldn't get client from nodeURI: %v", err)
client/debug/main.go:						return fmt.Errorf("expected hex or bech32. Got errors: hex: %v, bech32 acc: %v, bech32 val: %v", err, err2, err3)
client/keys/add.go:				return fmt.Errorf("failed to print mnemonic: %v", err)
client/keys/show.go:			return fmt.Errorf("%s is not a valid name or address: %v", args[0], err)
client/keys/show.go:				return fmt.Errorf("%s is not a valid name or address: %v", keyref, err)
crypto/armor.go:		return nil, "", fmt.Errorf("couldn't unarmor bytes: %v", err)
crypto/armor.go:		return privKey, "", fmt.Errorf("error decoding salt: %v", err.Error())
crypto/ledger/ledger_mock.go:		return nil, "", fmt.Errorf("error parsing public key: %v", err)
crypto/ledger/ledger_secp256k1.go:		return nil, fmt.Errorf("please open the %v app on the Ledger device - error: %v", options.appName, err)
crypto/ledger/ledger_secp256k1.go:		return nil, fmt.Errorf("error parsing public key: %v", err)
crypto/ledger/ledger_secp256k1.go:		return nil, "", fmt.Errorf("error parsing public key: %v", err)
depinject/container.go:				return nil, fmt.Errorf("internal error, unexpected number of values")
docs/architecture/adr-011-generalize-genesis-accounts.md:            return fmt.Errorf("invalid account found in genesis state; address: %s, error: %s", addrStr, err.Error())
server/grpc/gogoreflection/serverreflection.go:		return nil, fmt.Errorf("failed to decompress enc: %v", err)
server/grpc/gogoreflection/serverreflection.go:		return nil, fmt.Errorf("bad descriptor: %v", err)
server/grpc/gogoreflection/serverreflection.go:		return nil, fmt.Errorf("bad gzipped descriptor: %v", err)
server/grpc/gogoreflection/serverreflection.go:		return nil, fmt.Errorf("bad gzipped descriptor: %v", err)
server/start.go:		return fmt.Errorf("error creating listener: %v", err)
tools/confix/cmd/diff.go:				return fmt.Errorf("failed to load config: %v", err)
tools/confix/cmd/migrate.go:				return fmt.Errorf("failed to load config: %v", err)
tools/confix/file.go:		return nil, fmt.Errorf("failed to open %q: %v", path, err)
tools/confix/upgrade.go:		return fmt.Errorf("loading config: %v", err)
tools/confix/upgrade.go:		return fmt.Errorf("updating %q: %v", configPath, err)
tools/confix/upgrade.go:		return fmt.Errorf("formatting config: %v", err)
tools/confix/upgrade.go:			return fmt.Errorf("updated config is invalid: %v", err)
tools/rosetta/client_online.go:		return crgtypes.BlockResponse{}, fmt.Errorf("invalid block hash: %s", err)
types/dec_coin.go:		return DecCoin{}, fmt.Errorf("invalid denom cannot contain spaces: %s", err)
x/auth/client/tx.go:		return fmt.Errorf("%s: %s", errors.ErrorInvalidSigner, name)
x/auth/client/tx.go:		return fmt.Errorf("%s: %s", errors.ErrorInvalidSigner, name)
x/genutil/client/cli/validate_genesis.go:				return fmt.Errorf("error unmarshalling genesis doc %s: %s", genesis, err.Error())
x/genutil/client/cli/validate_genesis.go:				return fmt.Errorf("error validating genesis file %s: %s", genesis, err.Error())
x/genutil/gentx.go:			return nil, fmt.Errorf("failed to decode GenTx '%s': %s", genTx, err)
x/genutil/gentx.go:			return nil, fmt.Errorf("failed to encode GenTx '%s': %s", genTx, err)
x/genutil/gentx.go:			return nil, fmt.Errorf("failed to execute DeliverTx for '%s': %s", genTx, err)
x/genutil/types/genesis_state.go:		return tx, fmt.Errorf("failed to decode gentx: %s, error: %s", genTx, err)
x/gov/client/cli/query.go:				return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
x/gov/client/cli/query.go:				return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
x/gov/client/cli/query.go:				return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
x/gov/client/cli/query.go:				return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
x/gov/client/cli/query.go:				return fmt.Errorf("failed to fetch proposal-id %d: %s", proposalID, err)
x/params/types/subspace.go:		return fmt.Errorf("invalid parameter value: %s", err)
x/staking/client/cli/query.go:				return fmt.Errorf("height argument provided must be a non-negative-integer: %v", err)
x/staking/client/cli/tx.go:					return fmt.Errorf("invalid new commission rate: %v", err)
x/staking/migrations/v5/store.go:			return fmt.Errorf("can't parse height from key %q to int64: %v", strHeight, err)
x/upgrade/abci.go:				return fmt.Errorf("unable to write upgrade info to filesystem: %s", err.Error())
x/upgrade/plan/info.go:		return nil, fmt.Errorf("could not parse plan info: %v", err)
x/upgrade/plan/info.go:			return fmt.Errorf("invalid url \"%s\" in binaries[%s]: %v", val, key, err)
x/upgrade/plan/info.go:			return fmt.Errorf("error downloading binary for os/arch %s: %v", osArch, err)

We got a whole bunch of unpropagated, please see https://go.dev/blog/go1.13-errors and most definitely we want to propagate the original error in our returns

/cc @elias-orijtech

@odeke-em odeke-em added T:Bug Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity. labels Jun 14, 2023
odeke-em added a commit that referenced this issue Jun 15, 2023
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:Bug Type: Code Hygiene General cleanup and restructuring of code to provide clarity, flexibility, and modularity.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant