Provide better errors when failing to load storage Items

[webmaster128]

I have a set of contracts that use multiple Item<String>. Now I get the following error message:

failed to execute message; message index: 0: dispatch: submessages: alloc::string::String not found: execute wasm contract failed: invalid request

Unfortunately this does not help much to locate the issue. alloc::string::String is the type_name::<T> in

/// must_deserialize parses json bytes from storage (Option), returning NotFound error if no data present
pub(crate) fn must_deserialize<T: DeserializeOwned>(value: &Option<Vec<u8>>) -> StdResult<T> {
    match value {
        Some(vec) => from_slice(vec),
        None => Err(StdError::not_found(type_name::<T>())),
    }
}

If I wasn't a core maintainer, the error message would be completety unreadable for me.

The error expresses that a key was not found and is emitted before trying to deserialize into any type. I think the key should become the main information in the error message. The type could be there too but maybe as an optional hint.

In order to address that, I think we should add a new error type to cosmwasm-std and deprecate StdError::NotFound.

[ethanfrey]

Note that most contracts are built with a unique type included in only one Item or Map, so these error messages are often much easier to read. However, there are other cases where people include Uint128 or u64 in multiple Maps.

I would not deprecate this error, but maybe we could introduce new StoragePlusError, which wraps this one with more info. It would be API-breaking for all calling contracts, so we need to handle it carefully.

In order to address that, I think we should add a new error type to cosmwasm-std and deprecate StdError::NotFound.

Problem is the code that creates StdError::NotFound has no access to the Map/Item name, while the code that does have access is not in cosmwasm-std

[webmaster128]

maybe we could introduce new StoragePlusError

That would be great! I'm just afraid the contract developers then have to add both

pub enum ContractError {
    #[error("{0}")]
    /// this is needed so we can use `bucket.load(...)?` and have it auto-converted to the custom error
    Std(#[from] StdError),
    #[error("{0}")]
    StoragePlus(#[from] StoragePlusError),

But on the other hand, would that be bad?