Update Error messages in all Contracts and Transactions to be more descriptive

[joshuahannan]

Issue to be solved

Error messages for panics in transactions and contracts are not very helpful to end-users.
They should be updated to be more descriptive and re-usable
We should also try to gear them to be able to be understood by regular users if possible so they can know how to fix them or what to say to whatever support person they are talking to

Suggest A Solution

Update 10/17/2024: We will hold off on making any more changes until onflow/flips#289 is implemented and use that string formatting from then on

Go through all the standard contracts, transactions, and scripts in every smart contract repo to update them with good error messages.

When applicable, contracts should have utility functions that return common error messages. For example, FungibleToken could have a utility function that returns a message that describes when a Vault of a particular type is not borrowable in the signer's or receiver's account. This way, transactions, can use this function to create the error message instead of having to hard-code it themselves which introduces the risk of human error with a typo.

Error messages could contain:

• Contract name, resource name, and function name if coming from a contract
  • Example: ExampleToken.Vault.deposit: ..."
• Description of literal error
• Description of what high-level reason is causing the error
• Suggestion for fixing it if applicable
• If referring to a type that a regular user might understand like TopShot.Collection, use the format "TopShot Collection" or "FungibleToken Vault" instead of using . to make it less confusing to regular users who might see it
• If including type values in the error message where the type is especially important or is a user-provided argument, use <> to contain the type
• If including any other value type, try to phrase the error message so that the the value does not need to be contained in parentheses or an = sign. For example:

        let withdrawRef = signer.storage.borrow<auth(NonFungibleToken.Withdraw) &{NonFungibleToken.Collection}>(
                from: self.collectionData.storagePath
            ) ?? panic("The signer does not store a "
                        .concat(contractName)
                        .concat(" Collection object at the path ")
                        .concat(self.collectionData.storagePath.toString())
                        .concat("The signer must initialize their account with this collection first!"))

• If you do need to use some sort of symbol to contain values, try to use parentheses. For example:

        let resolverRef = getAccount(contractAddress)
            .contracts.borrow<&{NonFungibleToken}>(name: contractName)
                ?? panic("Could not borrow NonFungibleToken reference to the contract. Make sure the provided contract name ("
                         .concat(contractName).concat(") and address (").concat(contractAddress.toString()).concat(") are correct!"))

These are all up for debate

We also will likely want to include pages in the docs sites for recommendations for error messages
We will also need to reach out to important partners to get them to update theirs as well

Repos to update:

• flow-nft
• flow-ft
• bridged-usdc
• flow-core-contracts
• nft-storefront
• nft-catalog
• hybrid-custody
• Rewards
• EVM Bridge
• contract-updater
• tutorials
• ft and nft guides
• Flow Wallet Transactions
• dapper repos
• FIND
• Flowty
• Increment

Potential Future Updates

Include Error codes, potentially per contract, that a user can look up in each project's documentation
Include errors in a struct that has additional metadata

References

https://doc.rust-lang.org/error_codes/E0015.html

[turbolent]

This is a great initiative and very helpful for everyone!

Just wanted to note that currently, repeated concat calls are a bit expensive (every call re-allocates, similar to other languages). However, onflow/flips#289 proposes a new language feature to allow for more efficient string construction, which is also more readable. The implementation of it in onflow/cadence#3585 is fairly close to being done. It might sense to wait for and use the feature when it is ready, or go ahead already with the error message changes and at least consider adopting the new feature once it is available to reduce the overhead.

[joshuahannan]

Good suggestion! I guess I missed seeing that FLIP when it was originally proposed. I'll definitely wait until that feature is implemented to do the rest of the changes, but the changes to the ledger transactions specifically will probably need to still happen sooner because a different initiative is dependent on them.