Skip to content

Commit 4fd25c0

Browse files
authored
Fix using downcast with anyhow::Error (#12689)
* Fix using `downcast` with `anyhow::Error` Previously when converting `wasmtime::Error` into `anyhow::Error` it ended up breaking the `downcast` method. This is because `anyhow::Error::from_boxed` looks like it does not implement the `downcast` method which is how all errors were previously converted to `anyhow::Error`. This commit adds a new vtable method for specifically converting to `anyhow::Error` which enables using the typed construction methods of `anyhow` which preserves `downcast`-ness. * Add a test showcasing anyhow-source behavior
1 parent f6c69b0 commit 4fd25c0

File tree

4 files changed

+104
-1
lines changed

4 files changed

+104
-1
lines changed

crates/core/src/error/context.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -303,4 +303,12 @@ where
303303
}
304304
}
305305
}
306+
307+
#[cfg(feature = "anyhow")]
308+
fn ext_into_anyhow(mut self) -> anyhow::Error {
309+
match self.error.take() {
310+
Some(error) => anyhow::Error::from(error).context(self.context),
311+
None => anyhow::Error::msg(self),
312+
}
313+
}
306314
}

crates/core/src/error/error.rs

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ pub(crate) unsafe trait ErrorExt: Send + Sync + 'static {
6464
/// Take the backtrace from this error, if any.
6565
#[cfg(feature = "backtrace")]
6666
fn take_backtrace(&mut self) -> Option<Backtrace>;
67+
68+
/// Conversion into an `anyhow::Error`.
69+
#[cfg(feature = "anyhow")]
70+
fn ext_into_anyhow(self) -> anyhow::Error;
6771
}
6872

6973
/// Morally a `dyn ErrorExt` trait object that holds its own vtable.
@@ -542,7 +546,7 @@ impl From<Error> for Box<dyn core::error::Error + 'static> {
542546
impl From<Error> for anyhow::Error {
543547
#[inline]
544548
fn from(e: Error) -> Self {
545-
anyhow::Error::from_boxed(e.into_boxed_dyn_error())
549+
e.inner.into_anyhow()
546550
}
547551
}
548552

@@ -1241,6 +1245,11 @@ where
12411245
fn take_backtrace(&mut self) -> Option<Backtrace> {
12421246
None
12431247
}
1248+
1249+
#[cfg(feature = "anyhow")]
1250+
fn ext_into_anyhow(self) -> anyhow::Error {
1251+
anyhow::Error::new(self.0)
1252+
}
12441253
}
12451254

12461255
/// `ErrorExt` wrapper for types given to `Error::msg`.
@@ -1319,6 +1328,11 @@ where
13191328
fn take_backtrace(&mut self) -> Option<Backtrace> {
13201329
None
13211330
}
1331+
1332+
#[cfg(feature = "anyhow")]
1333+
fn ext_into_anyhow(self) -> anyhow::Error {
1334+
anyhow::Error::msg(self.0)
1335+
}
13221336
}
13231337

13241338
/// `ErrorExt` wrapper for `Box<dyn core::error::Error>`.
@@ -1374,6 +1388,11 @@ unsafe impl ErrorExt for BoxedError {
13741388
fn take_backtrace(&mut self) -> Option<Backtrace> {
13751389
None
13761390
}
1391+
1392+
#[cfg(feature = "anyhow")]
1393+
fn ext_into_anyhow(self) -> anyhow::Error {
1394+
anyhow::Error::from_boxed(self.0)
1395+
}
13771396
}
13781397

13791398
/// `ErrorExt` wrapper for `anyhow::Error`.
@@ -1430,6 +1449,11 @@ unsafe impl ErrorExt for AnyhowError {
14301449
fn take_backtrace(&mut self) -> Option<Backtrace> {
14311450
None
14321451
}
1452+
1453+
#[cfg(feature = "anyhow")]
1454+
fn ext_into_anyhow(self) -> anyhow::Error {
1455+
self.0
1456+
}
14331457
}
14341458

14351459
pub(crate) enum OomOrDynErrorRef<'a> {
@@ -1754,6 +1778,23 @@ impl OomOrDynError {
17541778
}
17551779
}
17561780

1781+
#[cfg(feature = "anyhow")]
1782+
pub(crate) fn into_anyhow(self) -> anyhow::Error {
1783+
if self.is_oom() {
1784+
// Safety: `self.is_oom()` is true.
1785+
anyhow::Error::from(unsafe { *self.unchecked_oom() })
1786+
} else {
1787+
debug_assert!(self.is_boxed_dyn_error());
1788+
// Safety: this is a boxed dyn error.
1789+
let ptr = unsafe { self.unchecked_into_dyn_error() };
1790+
// Safety: invariant of the type that the pointer is valid.
1791+
let vtable = unsafe { ptr.as_ref().vtable };
1792+
// Safety: the pointer is valid and the vtable is associated with
1793+
// this pointer's concrete error type.
1794+
unsafe { (vtable.into_anyhow)(ptr) }
1795+
}
1796+
}
1797+
17571798
/// Given that this is known to be an instance of the type associated with
17581799
/// the given `TypeId`, do an owning-downcast to that type, writing the
17591800
/// result through the given `ret_ptr`, and deallocating `self` along the

crates/core/src/error/vtable.rs

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,10 @@ pub(crate) struct Vtable {
4545
/// Upon successful return, a `T` will have been written to that memory
4646
/// block.
4747
pub(crate) downcast: unsafe fn(OwnedPtr<DynError>, TypeId, NonNull<u8>),
48+
49+
/// Conversion into `anyhow::Error` from `Box<Self>`.
50+
#[cfg(feature = "anyhow")]
51+
pub(crate) into_anyhow: unsafe fn(OwnedPtr<DynError>) -> anyhow::Error,
4852
}
4953

5054
impl Vtable {
@@ -63,6 +67,8 @@ impl Vtable {
6367
into_boxed_dyn_core_error: into_boxed_dyn_core_error::<E>,
6468
drop_and_deallocate: drop_and_deallocate::<E>,
6569
downcast: downcast::<E>,
70+
#[cfg(feature = "anyhow")]
71+
into_anyhow: into_anyhow::<E>,
6672
}
6773
}
6874
}
@@ -174,3 +180,14 @@ where
174180
}
175181
}
176182
}
183+
184+
#[cfg(feature = "anyhow")]
185+
unsafe fn into_anyhow<E>(error: OwnedPtr<DynError>) -> anyhow::Error
186+
where
187+
E: ErrorExt,
188+
{
189+
let error = error.cast::<ConcreteError<E>>();
190+
// Safety: implied by all vtable functions' safety contract.
191+
let error = unsafe { error.into_box() };
192+
error.error.ext_into_anyhow()
193+
}

crates/core/tests/tests.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -905,3 +905,40 @@ fn oom_requested_allocation_size() {
905905
let oom = OutOfMemory::new(usize::MAX);
906906
assert!(oom.requested_allocation_size() <= usize::try_from(isize::MAX).unwrap());
907907
}
908+
909+
#[test]
910+
#[cfg(feature = "anyhow")]
911+
fn anyhow_preserves_downcast() -> Result<()> {
912+
{
913+
let e: Error = TestError(1).into();
914+
assert!(e.downcast_ref::<TestError>().is_some());
915+
let e: anyhow::Error = e.into();
916+
assert!(e.downcast_ref::<TestError>().is_some());
917+
}
918+
{
919+
let e = Error::from(TestError(1)).context("hi");
920+
assert!(e.downcast_ref::<TestError>().is_some());
921+
assert!(e.downcast_ref::<&str>().is_some());
922+
let e: anyhow::Error = e.into();
923+
assert!(e.downcast_ref::<TestError>().is_some());
924+
assert!(e.downcast_ref::<&str>().is_some());
925+
}
926+
Ok(())
927+
}
928+
929+
#[test]
930+
#[cfg(feature = "anyhow")]
931+
fn anyhow_source_loses_downcast() -> Result<()> {
932+
let e: anyhow::Error = TestError(1).into();
933+
assert!(e.downcast_ref::<TestError>().is_some());
934+
935+
let e: Error = Error::from_anyhow(e);
936+
// FIXME: this should actually test for `is_some`
937+
assert!(e.downcast_ref::<TestError>().is_none());
938+
939+
// Even while the above is broken, when going back to `anyhow` we should
940+
// preserve the original error.
941+
let e: anyhow::Error = e.into();
942+
assert!(e.downcast_ref::<TestError>().is_some());
943+
Ok(())
944+
}

0 commit comments

Comments
 (0)