From 251dd7ea2ca7ff2b79529cfeac6ee967dff2c25a Mon Sep 17 00:00:00 2001
From: printfn <printfn@users.noreply.github.com>
Date: Sun, 3 Nov 2024 07:10:49 +0000
Subject: [PATCH] Improve currency conversion error messages

---
 core/src/error.rs               | 12 +++---------
 core/src/lib.rs                 | 12 +++++++++++-
 core/src/units.rs               |  6 +++++-
 core/tests/integration_tests.rs | 25 +++++++++++++++++++++++++
 4 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/core/src/error.rs b/core/src/error.rs
index 236997a4..4f56408c 100644
--- a/core/src/error.rs
+++ b/core/src/error.rs
@@ -16,7 +16,7 @@ pub(crate) enum FendError {
 	ZeroToThePowerOfZero,
 	FactorialComplex,
 	DeserializationError,
-	Wrap(Box<dyn error::Error + Send + Sync + 'static>),
+	Wrap(String, Box<dyn error::Error + Send + Sync + 'static>),
 	NoExchangeRatesAvailable,
 	OutOfRange {
 		value: Box<dyn crate::format::DisplayDebug>,
@@ -227,7 +227,7 @@ impl fmt::Display for FendError {
 				)
 			}
 			Self::FormattingError(_) => write!(f, "error during formatting"),
-			Self::Wrap(e) => write!(f, "{e}"),
+			Self::Wrap(e, _) => write!(f, "{e}"),
 			Self::ExpectedADateLiteral => write!(f, "Expected a date literal, e.g. @1970-01-01"),
 			Self::NonExistentDate {
 				year,
@@ -251,7 +251,7 @@ impl error::Error for FendError {
 		match self {
 			Self::FormattingError(e) => Some(e),
 			Self::IoError(e) => Some(e),
-			Self::Wrap(e) => Some(e.as_ref()),
+			Self::Wrap(_, e) => Some(e.as_ref()),
 			_ => None,
 		}
 	}
@@ -269,10 +269,4 @@ impl From<io::Error> for FendError {
 	}
 }
 
-impl From<Box<dyn error::Error + Send + Sync + 'static>> for FendError {
-	fn from(e: Box<dyn error::Error + Send + Sync + 'static>) -> Self {
-		Self::Wrap(e)
-	}
-}
-
 pub(crate) use crate::interrupt::Interrupt;
diff --git a/core/src/lib.rs b/core/src/lib.rs
index 495b658e..64ff3b22 100644
--- a/core/src/lib.rs
+++ b/core/src/lib.rs
@@ -40,6 +40,8 @@ mod serialize;
 mod units;
 mod value;
 
+use std::error::Error;
+use std::fmt::Write;
 use std::sync::Arc;
 use std::{collections::HashMap, fmt, io};
 
@@ -436,7 +438,15 @@ fn evaluate_with_interrupt_internal(
 	}
 	let (result, is_unit, attrs) = match eval::evaluate_to_spans(input, None, context, int) {
 		Ok(value) => value,
-		Err(e) => return Err(e.to_string()),
+		Err(e) => {
+			let mut error: &dyn Error = &e;
+			let mut s = error.to_string();
+			while let Some(inner) = error.source() {
+				write!(&mut s, ": {inner}").unwrap();
+				error = inner;
+			}
+			return Err(s);
+		}
 	};
 	let mut plain_result = String::new();
 	for s in &result {
diff --git a/core/src/units.rs b/core/src/units.rs
index 784fc83c..18b4b9de 100644
--- a/core/src/units.rs
+++ b/core/src/units.rs
@@ -42,7 +42,11 @@ fn expr_unit<I: Interrupt>(
 		let Some(exchange_rate_fn) = &context.get_exchange_rate else {
 			return Err(FendError::NoExchangeRatesAvailable);
 		};
-		let one_base_in_currency = exchange_rate_fn.relative_to_base_currency(&singular)?;
+		let one_base_in_currency = exchange_rate_fn
+			.relative_to_base_currency(&singular)
+			.map_err(|e| {
+				FendError::Wrap(format!("failed to retrieve {singular} exchange rate"), e)
+			})?;
 		let value = evaluate_to_value(
 			format!("(1/{one_base_in_currency}) BASE_CURRENCY").as_str(),
 			None,
diff --git a/core/tests/integration_tests.rs b/core/tests/integration_tests.rs
index 67000d03..53bd1911 100644
--- a/core/tests/integration_tests.rs
+++ b/core/tests/integration_tests.rs
@@ -6086,3 +6086,28 @@ fn kilopond() {
 	test_eval("gf to N", "0.00980665 N");
 	expect_error("GF to N", Some("cannot convert from GF to N: units 'ampere^2 second^4 kilogram^-1 meter^-2' and 'kilogram meter / second^2' are incompatible"));
 }
+
+#[derive(Debug)]
+struct TestError(Box<dyn std::error::Error + Send + Sync + 'static>);
+
+impl std::fmt::Display for TestError {
+	fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+		write!(f, "my error")
+	}
+}
+
+impl std::error::Error for TestError {
+	fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+		Some(self.0.as_ref())
+	}
+}
+
+#[test]
+fn test_nested_exchange_rate_error() {
+	let mut context = Context::new();
+	context.set_exchange_rate_handler_v1(|_: &str| Err(TestError("inner error".into()).into()));
+	assert_eq!(
+		evaluate("usd to eur", &mut context).unwrap_err(),
+		"failed to retrieve EUR exchange rate: my error: inner error",
+	);
+}