Make AssetLoader/Saver Error type bounds compatible with anyhow::Error

[rafalh]

Objective

• In Bevy 0.11 asset loaders used anyhow::Error for returning errors. In Bevy 0.12 AssetLoader (and AssetSaver) have associated Error type. Unfortunately it's type bounds does not allow anyhow::Error to be used despite migration guide claiming otherwise. This makes migration to 0.12 more challenging. Solve this by changing type bounds for associated Error type.
• Fix Can't use anyhow when writing custom AssetLoaders or AssetSavers #10350

Solution

Change associated Error type bounds to require Into<Box<dyn std::error::Error + Send + Sync + 'static>> to be implemented instead of std::error::Error + Send + Sync + 'static. Both anyhow::Error and errors generated by thiserror seems to be fine with such type bound.

---

Changelog

Fixed

• Fixed compatibility with anyhow::Error in AssetLoader and AssetSaver associated Error type

[rafalh]

Code below compiles so I assume it is compatible with pretty much all error types. It would be nice if we could get this change into Bevy 0.12.1 to make migration from 0.11 easier.

trait MyTrait {
    type Error: Into<Box<dyn std::error::Error + Send + Sync + 'static>>;
}

#[derive(Error, Debug)]
pub enum ThisErrorError {
    #[error("Foo")]
    _Foo,
}

#[derive(Debug)]
struct CustomError;
impl std::error::Error for CustomError {}
impl std::fmt::Display for CustomError {
    fn fmt(&self, _f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
        todo!()
    }
}

struct MyStructWithAnyhowError;
impl MyTrait for MyStructWithAnyhowError {
    type Error = anyhow::Error;
}

struct MyStructWithThiserrorError;
impl MyTrait for MyStructWithThiserrorError {
    type Error = ThisErrorError;
}

struct MyStructWithCustomError;
impl MyTrait for MyStructWithCustomError {
    type Error = CustomError;
}

[nicopap]

I'm not sure I understand this comment:

I also stepped onto this problem. I wanted to use Box<dyn std::error::Error + Send + Sync> as mentioned above but it does not work because T must be Sized for Box<T> to implement std::error::Error which is a little non-intuitive.

What are the implications for this PR?

to reviewers: the relevant std trait impl is: https://doc.rust-lang.org/stable/std/convert/trait.From.html#impl-From%3CE%3E-for-Box%3Cdyn+Error+%2B+Sync+%2B+Send+%2B+'a,+Global%3E

[nicopap]

This makes sense to me. Would this break compatibility with people using type Error = Box<dyn Error + Send + Sync> as error type?

[rafalh]

I'm not sure I understand this comment:

I also stepped onto this problem. I wanted to use Box<dyn std::error::Error + Send + Sync> as mentioned above but it does not work because T must be Sized for Box<T> to implement std::error::Error which is a little non-intuitive.

What are the implications for this PR?

I wrote this comment before preparing this PR and it is not directly related. It was about workaround proposed in #10350 that does not work (it did not involve anyhow::Error at all). I was trying to understand why it does not work and described my findings.

This makes sense to me. Would this break compatibility with people using type Error = Box<dyn Error + Send + Sync> as error type?

AFAIK there are no such people because Box<dyn Error + Send + Sync> does not implement std::error::Error (which is required by AssetLoader::Error) because of implicit Sized bound in https://doc.rust-lang.org/src/alloc/boxed.rs.html#2433 (dyn Error + Send + Sync is not Sized). It would work if impl<T: core::error::Error + ?Sized> core::error::Error for Box<T> existed.

[nicopap]

Yeah then I think we can include it in 0.12.1.