Skip to content

Commit

Permalink
Removed anyhow (bevyengine#10003)
Browse files Browse the repository at this point in the history
# Objective

- Fixes bevyengine#8140

## Solution

- Added Explicit Error Typing for `AssetLoader` and `AssetSaver`, which
were the last instances of `anyhow` in use across Bevy.

---

## Changelog

- Added an associated type `Error` to `AssetLoader` and `AssetSaver` for
use with the `load` and `save` methods respectively.
- Changed `ErasedAssetLoader` and `ErasedAssetSaver` `load` and `save`
methods to use `Box<dyn Error + Send + Sync + 'static>` to allow for
arbitrary `Error` types from the non-erased trait variants. Note the
strict requirements match the pre-existing requirements around
`anyhow::Error`.

## Migration Guide

- `anyhow` is no longer exported by `bevy_asset`; Add it to your own
project (if required).
- `AssetLoader` and `AssetSaver` have an associated type `Error`; Define
an appropriate error type (e.g., using `thiserror`), or use a pre-made
error type (e.g., `anyhow::Error`). Note that using `anyhow::Error` is a
drop-in replacement.
- `AssetLoaderError` has been removed; Define a new error type, or use
an alternative (e.g., `anyhow::Error`)
- All the first-party `AssetLoader`'s and `AssetSaver`'s now return
relevant (and narrow) error types instead of a single ambiguous type;
Match over the specific error type, or encapsulate (`Box<dyn>`,
`thiserror`, `anyhow`, etc.)

## Notes

A simpler PR to resolve this issue would simply define a Bevy `Error`
type defined as `Box<dyn std::error::Error + Send + Sync + 'static>`,
but I think this type of error handling should be discouraged when
possible. Since only 2 traits required the use of `anyhow`, it isn't a
substantive body of work to solidify these error types, and remove
`anyhow` entirely. End users are still encouraged to use `anyhow` if
that is their preferred error handling style. Arguably, adding the
`Error` associated type gives more freedom to end-users to decide
whether they want more or less explicit error handling (`anyhow` vs
`thiserror`).

As an aside, I didn't perform any testing on Android or WASM. CI passed
locally, but there may be mistakes for those platforms I missed.
  • Loading branch information
bushrat011899 authored and Ray Redondo committed Jan 9, 2024
1 parent 7ed48b1 commit 9cb30ce
Show file tree
Hide file tree
Showing 28 changed files with 226 additions and 123 deletions.
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,6 @@ bevy_dylib = { path = "crates/bevy_dylib", version = "0.12.0-dev", default-featu
bevy_internal = { path = "crates/bevy_internal", version = "0.12.0-dev", default-features = false }

[dev-dependencies]
anyhow = "1.0.4"
rand = "0.8.0"
ron = "0.8.0"
serde = { version = "1", features = ["derive"] }
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ bevy_reflect = { path = "../bevy_reflect", version = "0.12.0-dev" }
bevy_tasks = { path = "../bevy_tasks", version = "0.12.0-dev" }
bevy_utils = { path = "../bevy_utils", version = "0.12.0-dev" }

anyhow = "1.0"
async-broadcast = "0.5"
async-fs = "1.5"
async-lock = "2.8"
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/io/android.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::io::{
get_meta_path, AssetReader, AssetReaderError, AssetWatcher, EmptyPathStream, PathStream,
Reader, VecReader,
};
use anyhow::Result;
use bevy_log::error;
use bevy_utils::BoxedFuture;
use std::{ffi::CString, path::Path};
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/io/file/file_watcher.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::io::{AssetSourceEvent, AssetWatcher};
use anyhow::Result;
use bevy_log::error;
use bevy_utils::Duration;
use crossbeam_channel::Sender;
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/io/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use crate::io::{
get_meta_path, AssetReader, AssetReaderError, AssetWatcher, AssetWriter, AssetWriterError,
PathStream, Reader, Writer,
};
use anyhow::Result;
use async_fs::{read_dir, File};
use bevy_utils::BoxedFuture;
use futures_lite::StreamExt;
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/io/gated.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::io::{AssetReader, AssetReaderError, PathStream, Reader};
use anyhow::Result;
use bevy_utils::{BoxedFuture, HashMap};
use crossbeam_channel::{Receiver, Sender};
use parking_lot::RwLock;
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/io/memory.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::io::{AssetReader, AssetReaderError, PathStream, Reader};
use anyhow::Result;
use bevy_utils::{BoxedFuture, HashMap};
use futures_io::AsyncRead;
use futures_lite::{ready, Stream};
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/io/processor_gated.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::{
processor::{AssetProcessorData, ProcessStatus},
AssetPath,
};
use anyhow::Result;
use async_lock::RwLockReadGuardArc;
use bevy_log::trace;
use bevy_utils::BoxedFuture;
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_asset/src/io/wasm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ use crate::io::{
get_meta_path, AssetReader, AssetReaderError, AssetWatcher, EmptyPathStream, PathStream,
Reader, VecReader,
};
use anyhow::Result;
use bevy_log::error;
use bevy_utils::BoxedFuture;
use js_sys::{Uint8Array, JSON};
Expand Down
27 changes: 22 additions & 5 deletions crates/bevy_asset/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ pub use path::*;
pub use reflect::*;
pub use server::*;

pub use anyhow;
pub use bevy_utils::BoxedFuture;

use crate::{
Expand Down Expand Up @@ -428,8 +427,9 @@ mod tests {
Reader,
},
loader::{AssetLoader, LoadContext},
Asset, AssetApp, AssetEvent, AssetId, AssetPlugin, AssetProvider, AssetProviders,
AssetServer, Assets, DependencyLoadState, LoadState, RecursiveDependencyLoadState,
Asset, AssetApp, AssetEvent, AssetId, AssetPath, AssetPlugin, AssetProvider,
AssetProviders, AssetServer, Assets, DependencyLoadState, LoadState,
RecursiveDependencyLoadState,
};
use bevy_app::{App, Update};
use bevy_core::TaskPoolPlugin;
Expand All @@ -444,6 +444,7 @@ mod tests {
use futures_lite::AsyncReadExt;
use serde::{Deserialize, Serialize};
use std::path::Path;
use thiserror::Error;

#[derive(Asset, TypePath, Debug)]
pub struct CoolText {
Expand Down Expand Up @@ -471,24 +472,40 @@ mod tests {
#[derive(Default)]
struct CoolTextLoader;

#[derive(Error, Debug)]
enum CoolTextLoaderError {
#[error("Could not load dependency: {dependency}")]
CannotLoadDependency { dependency: AssetPath<'static> },
#[error("A RON error occurred during loading")]
RonSpannedError(#[from] ron::error::SpannedError),
#[error("An IO error occurred during loading")]
Io(#[from] std::io::Error),
}

impl AssetLoader for CoolTextLoader {
type Asset = CoolText;

type Settings = ();

type Error = CoolTextLoaderError;

fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a Self::Settings,
load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Self::Asset, anyhow::Error>> {
) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
let mut ron: CoolTextRon = ron::de::from_bytes(&bytes)?;
let mut embedded = String::new();
for dep in ron.embedded_dependencies {
let loaded = load_context.load_direct(&dep).await?;
let loaded = load_context.load_direct(&dep).await.map_err(|_| {
Self::Error::CannotLoadDependency {
dependency: dep.into(),
}
})?;
let cool = loaded.get::<CoolText>().unwrap();
embedded.push_str(&cool.text);
}
Expand Down
25 changes: 11 additions & 14 deletions crates/bevy_asset/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ pub trait AssetLoader: Send + Sync + 'static {
type Asset: crate::Asset;
/// The settings type used by this [`AssetLoader`].
type Settings: Settings + Default + Serialize + for<'a> Deserialize<'a>;
/// The type of [error](`std::error::Error`) which could be encountered by this loader.
type Error: std::error::Error + Send + Sync + 'static;
/// Asynchronously loads [`AssetLoader::Asset`] (and any other labeled assets) from the bytes provided by [`Reader`].
fn load<'a>(
&'a self,
reader: &'a mut Reader,
settings: &'a Self::Settings,
load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<Self::Asset, anyhow::Error>>;
) -> BoxedFuture<'a, Result<Self::Asset, Self::Error>>;

/// Returns a list of extensions supported by this asset loader, without the preceding dot.
fn extensions(&self) -> &[&str];
Expand All @@ -46,7 +48,10 @@ pub trait ErasedAssetLoader: Send + Sync + 'static {
reader: &'a mut Reader,
meta: Box<dyn AssetMetaDyn>,
load_context: LoadContext<'a>,
) -> BoxedFuture<'a, Result<ErasedLoadedAsset, AssetLoaderError>>;
) -> BoxedFuture<
'a,
Result<ErasedLoadedAsset, Box<dyn std::error::Error + Send + Sync + 'static>>,
>;

/// Returns a list of extensions supported by this asset loader, without the preceding dot.
fn extensions(&self) -> &[&str];
Expand All @@ -64,17 +69,6 @@ pub trait ErasedAssetLoader: Send + Sync + 'static {
fn asset_type_id(&self) -> TypeId;
}

/// An error encountered during [`AssetLoader::load`].
#[derive(Error, Debug)]
pub enum AssetLoaderError {
/// Any error that occurs during load.
#[error(transparent)]
Load(#[from] anyhow::Error),
/// A failure to deserialize metadata during load.
#[error(transparent)]
DeserializeMeta(#[from] DeserializeMetaError),
}

impl<L> ErasedAssetLoader for L
where
L: AssetLoader + Send + Sync,
Expand All @@ -85,7 +79,10 @@ where
reader: &'a mut Reader,
meta: Box<dyn AssetMetaDyn>,
mut load_context: LoadContext<'a>,
) -> BoxedFuture<'a, Result<ErasedLoadedAsset, AssetLoaderError>> {
) -> BoxedFuture<
'a,
Result<ErasedLoadedAsset, Box<dyn std::error::Error + Send + Sync + 'static>>,
> {
Box::pin(async move {
let settings = meta
.loader_settings()
Expand Down
3 changes: 2 additions & 1 deletion crates/bevy_asset/src/meta.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,12 +193,13 @@ impl VisitAssetDependencies for () {
impl AssetLoader for () {
type Asset = ();
type Settings = ();
type Error = std::io::Error;
fn load<'a>(
&'a self,
_reader: &'a mut crate::io::Reader,
_settings: &'a Self::Settings,
_load_context: &'a mut crate::LoadContext,
) -> bevy_utils::BoxedFuture<'a, Result<Self::Asset, anyhow::Error>> {
) -> bevy_utils::BoxedFuture<'a, Result<Self::Asset, Self::Error>> {
unreachable!();
}

Expand Down
25 changes: 11 additions & 14 deletions crates/bevy_asset/src/processor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use crate::{
get_asset_hash, get_full_asset_hash, AssetAction, AssetActionMinimal, AssetHash, AssetMeta,
AssetMetaDyn, AssetMetaMinimal, ProcessedInfo, ProcessedInfoMinimal,
},
AssetLoadError, AssetLoaderError, AssetPath, AssetServer, DeserializeMetaError,
LoadDirectError, MissingAssetLoaderForExtensionError, CANNOT_WATCH_ERROR_MESSAGE,
AssetLoadError, AssetPath, AssetServer, DeserializeMetaError,
MissingAssetLoaderForExtensionError, CANNOT_WATCH_ERROR_MESSAGE,
};
use bevy_ecs::prelude::*;
use bevy_log::{debug, error, trace, warn};
Expand Down Expand Up @@ -1130,20 +1130,17 @@ impl ProcessorAssetInfos {
Err(err) => {
error!("Failed to process asset {:?}: {:?}", asset_path, err);
// if this failed because a dependency could not be loaded, make sure it is reprocessed if that dependency is reprocessed
if let ProcessError::AssetLoadError(AssetLoadError::AssetLoaderError {
error: AssetLoaderError::Load(loader_error),
..
if let ProcessError::AssetLoadError(AssetLoadError::CannotLoadDependency {
path: dependency,
}) = err
{
if let Some(error) = loader_error.downcast_ref::<LoadDirectError>() {
let info = self.get_mut(&asset_path).expect("info should exist");
info.processed_info = Some(ProcessedInfo {
hash: AssetHash::default(),
full_hash: AssetHash::default(),
process_dependencies: vec![],
});
self.add_dependant(&error.dependency, asset_path.to_owned());
}
let info = self.get_mut(&asset_path).expect("info should exist");
info.processed_info = Some(ProcessedInfo {
hash: AssetHash::default(),
full_hash: AssetHash::default(),
process_dependencies: vec![],
});
self.add_dependant(&dependency, asset_path.to_owned());
}

let info = self.get_mut(&asset_path).expect("info should exist");
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_asset/src/processor/process.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub enum ProcessError {
#[error("The wrong meta type was passed into a processor. This is probably an internal implementation error.")]
WrongMetaType,
#[error("Encountered an error while saving the asset: {0}")]
AssetSaveError(anyhow::Error),
AssetSaveError(#[from] Box<dyn std::error::Error + Send + Sync + 'static>),
#[error("Assets without extensions are not supported.")]
ExtensionRequired,
}
Expand Down Expand Up @@ -122,7 +122,7 @@ impl<Loader: AssetLoader, Saver: AssetSaver<Asset = Loader::Asset>> Process
.saver
.save(writer, saved_asset, &settings.saver_settings)
.await
.map_err(ProcessError::AssetSaveError)?;
.map_err(|error| ProcessError::AssetSaveError(Box::new(error)))?;
Ok(output_settings)
})
}
Expand Down
11 changes: 8 additions & 3 deletions crates/bevy_asset/src/saver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,14 @@ use std::ops::Deref;
/// Saves an [`Asset`] of a given [`AssetSaver::Asset`] type. [`AssetSaver::OutputLoader`] will then be used to load the saved asset
/// in the final deployed application. The saver should produce asset bytes in a format that [`AssetSaver::OutputLoader`] can read.
pub trait AssetSaver: Send + Sync + 'static {
/// The top level [`Asset`] saved by this [`AssetSaver`].
type Asset: Asset;
/// The settings type used by this [`AssetSaver`].
type Settings: Settings + Default + Serialize + for<'a> Deserialize<'a>;
/// The type of [`AssetLoader`] used to load this [`Asset`]
type OutputLoader: AssetLoader;
/// The type of [error](`std::error::Error`) which could be encountered by this saver.
type Error: std::error::Error + Send + Sync + 'static;

/// Saves the given runtime [`Asset`] by writing it to a byte format using `writer`. The passed in `settings` can influence how the
/// `asset` is saved.
Expand All @@ -18,7 +23,7 @@ pub trait AssetSaver: Send + Sync + 'static {
writer: &'a mut Writer,
asset: SavedAsset<'a, Self::Asset>,
settings: &'a Self::Settings,
) -> BoxedFuture<'a, Result<<Self::OutputLoader as AssetLoader>::Settings, anyhow::Error>>;
) -> BoxedFuture<'a, Result<<Self::OutputLoader as AssetLoader>::Settings, Self::Error>>;
}

/// A type-erased dynamic variant of [`AssetSaver`] that allows callers to save assets without knowing the actual type of the [`AssetSaver`].
Expand All @@ -30,7 +35,7 @@ pub trait ErasedAssetSaver: Send + Sync + 'static {
writer: &'a mut Writer,
asset: &'a ErasedLoadedAsset,
settings: &'a dyn Settings,
) -> BoxedFuture<'a, Result<(), anyhow::Error>>;
) -> BoxedFuture<'a, Result<(), Box<dyn std::error::Error + Send + Sync + 'static>>>;

/// The type name of the [`AssetSaver`].
fn type_name(&self) -> &'static str;
Expand All @@ -42,7 +47,7 @@ impl<S: AssetSaver> ErasedAssetSaver for S {
writer: &'a mut Writer,
asset: &'a ErasedLoadedAsset,
settings: &'a dyn Settings,
) -> BoxedFuture<'a, Result<(), anyhow::Error>> {
) -> BoxedFuture<'a, Result<(), Box<dyn std::error::Error + Send + Sync + 'static>>> {
Box::pin(async move {
let settings = settings
.downcast_ref::<S::Settings>()
Expand Down
27 changes: 9 additions & 18 deletions crates/bevy_asset/src/server/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ mod info;
use crate::{
folder::LoadedFolder,
io::{AssetReader, AssetReaderError, AssetSourceEvent, AssetWatcher, Reader},
loader::{AssetLoader, AssetLoaderError, ErasedAssetLoader, LoadContext, LoadedAsset},
loader::{AssetLoader, ErasedAssetLoader, LoadContext, LoadedAsset},
meta::{
loader_settings_meta_transform, AssetActionMinimal, AssetMetaDyn, AssetMetaMinimal,
MetaTransform, Settings,
Expand Down Expand Up @@ -666,11 +666,9 @@ impl AssetServer {
}
};
let loader = self.get_asset_loader_with_type_name(&loader_name).await?;
let meta = loader.deserialize_meta(&meta_bytes).map_err(|e| {
AssetLoadError::AssetLoaderError {
let meta = loader.deserialize_meta(&meta_bytes).map_err(|_| {
AssetLoadError::CannotLoadDependency {
path: asset_path.clone().into_owned(),
loader: loader.type_name(),
error: AssetLoaderError::DeserializeMeta(e),
}
})?;

Expand Down Expand Up @@ -698,13 +696,10 @@ impl AssetServer {
let asset_path = asset_path.clone().into_owned();
let load_context =
LoadContext::new(self, asset_path.clone(), load_dependencies, populate_hashes);
loader.load(reader, meta, load_context).await.map_err(|e| {
AssetLoadError::AssetLoaderError {
loader: loader.type_name(),
path: asset_path,
error: e,
}
})
loader
.load(reader, meta, load_context)
.await
.map_err(|_| AssetLoadError::CannotLoadDependency { path: asset_path })
}
}

Expand Down Expand Up @@ -861,12 +856,8 @@ pub enum AssetLoadError {
CannotLoadProcessedAsset { path: AssetPath<'static> },
#[error("Asset '{path}' is configured to be ignored. It cannot be loaded.")]
CannotLoadIgnoredAsset { path: AssetPath<'static> },
#[error("Asset '{path}' encountered an error in {loader}: {error}")]
AssetLoaderError {
path: AssetPath<'static>,
loader: &'static str,
error: AssetLoaderError,
},
#[error("Asset '{path}' is a dependency. It cannot be loaded directly.")]
CannotLoadDependency { path: AssetPath<'static> },
}

/// An error that occurs when an [`AssetLoader`] is not registered for a given extension.
Expand Down
4 changes: 2 additions & 2 deletions crates/bevy_audio/src/audio_source.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use bevy_asset::{
anyhow::Error,
io::{AsyncReadExt, Reader},
Asset, AssetLoader, LoadContext,
};
Expand Down Expand Up @@ -42,13 +41,14 @@ pub struct AudioLoader;
impl AssetLoader for AudioLoader {
type Asset = AudioSource;
type Settings = ();
type Error = std::io::Error;

fn load<'a>(
&'a self,
reader: &'a mut Reader,
_settings: &'a Self::Settings,
_load_context: &'a mut LoadContext,
) -> BoxedFuture<'a, Result<AudioSource, Error>> {
) -> BoxedFuture<'a, Result<AudioSource, Self::Error>> {
Box::pin(async move {
let mut bytes = Vec::new();
reader.read_to_end(&mut bytes).await?;
Expand Down
Loading

0 comments on commit 9cb30ce

Please sign in to comment.