Make AssetLoader/Saver Error type bounds compatible with anyhow::Error#10493
Make AssetLoader/Saver Error type bounds compatible with anyhow::Error#10493cart merged 1 commit intobevyengine:mainfrom
Conversation
|
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;
} |
rafalh
changed the title
anyhow::Error does not implement std::error::Error but converting to boxed std::error::Error is possible. At the same time this bound should work fine with custom error types like ones generated by thiserror. Note: after this change Bevy 0.12 migration guide claim that anyhow::Error works in this context becomes true. Fixes bevyengine#10350
rafalh
force-pushed
the
asset-loader-saver-error
branch
from
6f4d569 to
bee7d1a
Compare
nicopap
added
C-Feature
nicopap
added
P-Regression
|
I'm not sure I understand this comment:
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
left a comment
nicopap
left a comment
There was a problem hiding this comment.
This makes sense to me. Would this break compatibility with people using type Error = Box<dyn Error + Send + Sync> as error type?
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
AFAIK there are no such people because |
|
Yeah then I think we can include it in 0.12.1. |
alice-i-cecile
added
the
S-Ready-For-Final-Review
bevyengine#10493) # 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 bevyengine#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
#10493) # 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 #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
bevyengine#10493) # 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 bevyengine#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
5 participants
Objective
anyhow::Errorfor returning errors. In Bevy 0.12AssetLoader(andAssetSaver) have associatedErrortype. Unfortunately it's type bounds does not allowanyhow::Errorto be used despite migration guide claiming otherwise. This makes migration to 0.12 more challenging. Solve this by changing type bounds for associatedErrortype.anyhowwhen writing customAssetLoaders orAssetSavers #10350Solution
Change associated
Errortype bounds to requireInto<Box<dyn std::error::Error + Send + Sync + 'static>>to be implemented instead ofstd::error::Error + Send + Sync + 'static. Bothanyhow::Errorand errors generated bythiserrorseems to be fine with such type bound.Changelog
Fixed
anyhow::ErrorinAssetLoaderandAssetSaverassociatedErrortype