Skip to content

Commit

Permalink
feat: dedup error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes committed Dec 4, 2024
1 parent 3a1e76b commit cb1639f
Show file tree
Hide file tree
Showing 9 changed files with 74 additions and 68 deletions.
2 changes: 1 addition & 1 deletion crates/cast/bin/cmd/send.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub enum SendTxSubcommands {

impl SendTxArgs {
#[allow(unknown_lints, dependency_on_unit_never_type_fallback)]
pub async fn run(self) -> Result<(), eyre::Report> {
pub async fn run(self) -> eyre::Result<()> {
let Self {
eth,
to,
Expand Down
16 changes: 1 addition & 15 deletions crates/cheatcodes/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ impl Error {
}

impl Drop for Error {
#[inline]
fn drop(&mut self) {
if self.drop {
drop(unsafe { Box::<[u8]>::from_raw(self.data.cast_mut()) });
Expand All @@ -224,21 +223,18 @@ impl From<Cow<'static, str>> for Error {
}

impl From<String> for Error {
#[inline]
fn from(value: String) -> Self {
Self::new_string(value)
}
}

impl From<&'static str> for Error {
#[inline]
fn from(value: &'static str) -> Self {
Self::new_str(value)
}
}

impl From<Cow<'static, [u8]>> for Error {
#[inline]
fn from(value: Cow<'static, [u8]>) -> Self {
match value {
Cow::Borrowed(bytes) => Self::new_bytes(bytes),
Expand All @@ -248,21 +244,18 @@ impl From<Cow<'static, [u8]>> for Error {
}

impl From<&'static [u8]> for Error {
#[inline]
fn from(value: &'static [u8]) -> Self {
Self::new_bytes(value)
}
}

impl<const N: usize> From<&'static [u8; N]> for Error {
#[inline]
fn from(value: &'static [u8; N]) -> Self {
Self::new_bytes(value)
}
}

impl From<Vec<u8>> for Error {
#[inline]
fn from(value: Vec<u8>) -> Self {
Self::new_vec(value)
}
Expand All @@ -279,7 +272,6 @@ impl From<Bytes> for Error {
macro_rules! impl_from {
($($t:ty),* $(,)?) => {$(
impl From<$t> for Error {
#[inline]
fn from(value: $t) -> Self {
Self::display(value)
}
Expand Down Expand Up @@ -309,20 +301,14 @@ impl_from!(
);

impl<T: Into<BackendError>> From<EVMError<T>> for Error {
#[inline]
fn from(err: EVMError<T>) -> Self {
Self::display(BackendError::from(err))
}
}

impl From<eyre::Report> for Error {
#[inline]
fn from(err: eyre::Report) -> Self {
let mut chained_cause = String::new();
for cause in err.chain() {
chained_cause.push_str(format!(" {cause};").as_str());
}
Self::display(chained_cause)
Self::display(foundry_common::errors::display_chain(&err))
}
}

Expand Down
35 changes: 35 additions & 0 deletions crates/common/src/errors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,38 @@ pub use fs::FsPathError;

mod artifacts;
pub use artifacts::*;

/// Displays a chain of errors in a single line.
pub fn display_chain(error: &eyre::Report) -> String {
let mut causes = all_sources(error);
// Deduplicate the common pattern `msg1: msg2; msg2` -> `msg1: msg2`.
causes.dedup_by(|b, a| a.contains(b.as_str()));
causes.join("; ")
}

fn all_sources(err: &eyre::Report) -> Vec<String> {
err.chain().map(|cause| cause.to_string().trim().to_string()).collect()
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn dedups_contained() {
#[derive(thiserror::Error, Debug)]
#[error("my error: {0}")]
struct A(#[from] B);

#[derive(thiserror::Error, Debug)]
#[error("{0}")]
struct B(String);

let ee = eyre::Report::from(A(B("hello".into())));
assert_eq!(ee.chain().count(), 2, "{ee:?}");
let full = all_sources(&ee).join("; ");
assert_eq!(full, "my error: hello; hello");
let chained = display_chain(&ee);
assert_eq!(chained, "my error: hello");
}
}
2 changes: 1 addition & 1 deletion crates/evm/core/src/backend/cow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ impl<'a> CowBackend<'a> {
self.spec_id = env.handler_cfg.spec_id;
let mut evm = crate::utils::new_evm_with_inspector(self, env.clone(), inspector);

let res = evm.transact().wrap_err("backend: failed while inspecting")?;
let res = evm.transact().wrap_err("EVM error")?;

env.env = evm.context.evm.inner.env;

Expand Down
9 changes: 1 addition & 8 deletions crates/evm/core/src/backend/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use crate::{
utils::{configure_tx_env, configure_tx_req_env, new_evm_with_inspector},
InspectorExt,
};
use alloy_consensus::Transaction as TransactionTrait;
use alloy_genesis::GenesisAccount;
use alloy_network::{AnyRpcBlock, AnyTxEnvelope, TransactionResponse};
use alloy_primitives::{keccak256, uint, Address, TxKind, B256, U256};
Expand Down Expand Up @@ -771,7 +770,7 @@ impl Backend {
self.initialize(env);
let mut evm = crate::utils::new_evm_with_inspector(self, env.clone(), inspector);

let res = evm.transact().wrap_err("backend: failed while inspecting")?;
let res = evm.transact().wrap_err("EVM error")?;

env.env = evm.context.evm.inner.env;

Expand Down Expand Up @@ -1937,12 +1936,6 @@ fn commit_transaction(
persistent_accounts: &HashSet<Address>,
inspector: &mut dyn InspectorExt,
) -> eyre::Result<()> {
// TODO: Remove after https://github.com/foundry-rs/foundry/pull/9131
// if the tx has the blob_versioned_hashes field, we assume it's a Cancun block
if tx.blob_versioned_hashes().is_some() {
env.handler_cfg.spec_id = SpecId::CANCUN;
}

configure_tx_env(&mut env.env, tx);

let now = Instant::now();
Expand Down
7 changes: 4 additions & 3 deletions crates/evm/core/src/opts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use foundry_common::{provider::ProviderBuilder, ALCHEMY_FREE_TIER_CUPS};
use foundry_config::{Chain, Config, GasLimit};
use revm::primitives::{BlockEnv, CfgEnv, TxEnv};
use serde::{Deserialize, Serialize};
use std::fmt::Write;
use url::Url;

#[derive(Clone, Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -129,13 +130,13 @@ impl EvmOpts {
)
.await
.wrap_err_with(|| {
let mut err_msg = "Could not instantiate forked environment".to_string();
let mut msg = "Could not instantiate forked environment".to_string();
if let Ok(url) = Url::parse(fork_url) {
if let Some(provider) = url.host() {
err_msg.push_str(&format!(" with provider {provider}"));
write!(msg, " with provider {provider}").unwrap();
}
}
err_msg
msg
})
}

Expand Down
18 changes: 6 additions & 12 deletions crates/evm/evm/src/executors/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -708,8 +708,12 @@ pub enum EvmError {
#[error("{_0}")]
Skip(SkipReason),
/// Any other error.
#[error(transparent)]
Eyre(eyre::Error),
#[error("{}", foundry_common::errors::display_chain(.0))]
Eyre(
#[from]
#[source]
eyre::Report,
),
}

impl From<ExecutionErr> for EvmError {
Expand All @@ -724,16 +728,6 @@ impl From<alloy_sol_types::Error> for EvmError {
}
}

impl From<eyre::Error> for EvmError {
fn from(err: eyre::Report) -> Self {
let mut chained_cause = String::new();
for cause in err.chain() {
chained_cause.push_str(format!("{cause}; ").as_str());
}
Self::Eyre(eyre::format_err!("{chained_cause}"))
}
}

/// The result of a deployment.
#[derive(Debug)]
pub struct DeployResult {
Expand Down
27 changes: 0 additions & 27 deletions crates/forge/tests/cli/script.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2397,33 +2397,6 @@ Simulated On-chain Traces:
"#]]);
});

// Tests that chained errors are properly displayed.
// <https://github.com/foundry-rs/foundry/issues/9161>
forgetest_init!(
#[ignore]
should_display_evm_chained_error,
|prj, cmd| {
let script = prj
.add_source(
"Foo",
r#"
import "forge-std/Script.sol";
contract ContractScript is Script {
function run() public {
}
}
"#,
)
.unwrap();
cmd.arg("script").arg(script).args(["--fork-url", "https://public-node.testnet.rsk.co"]).assert_failure().stderr_eq(str![[r#"
Error: Failed to deploy script:
backend: failed while inspecting; header validation error: `prevrandao` not set; `prevrandao` not set;
"#]]);
}
);

forgetest_async!(should_detect_additional_contracts, |prj, cmd| {
let (_api, handle) = spawn(NodeConfig::test()).await;

Expand Down
26 changes: 25 additions & 1 deletion crates/forge/tests/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2659,7 +2659,7 @@ contract ForkTest is Test {
cmd.args(["test", "--mt", "test_fork_err_message"]).assert_failure().stdout_eq(str![[r#"
...
Ran 1 test for test/ForkTest.t.sol:ForkTest
[FAIL: vm.createSelectFork: Could not instantiate forked environment with provider eth-mainnet.g.alchemy.com;] test_fork_err_message() ([GAS])
[FAIL: vm.createSelectFork: Could not instantiate forked environment with provider eth-mainnet.g.alchemy.com] test_fork_err_message() ([GAS])
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; [ELAPSED]
...
Expand Down Expand Up @@ -2700,3 +2700,27 @@ Ran 1 test suite [ELAPSED]: 1 tests passed, 0 failed, 0 skipped (1 total tests)
"#]]);
});

// Tests that chained errors are properly displayed.
// <https://github.com/foundry-rs/foundry/issues/9161>
forgetest!(displays_chained_error, |prj, cmd| {
prj.add_test(
"Foo.t.sol",
r#"
contract ContractTest {
function test_anything(uint) public {}
}
"#,
)
.unwrap();

cmd.arg("test").arg("--gas-limit=100").assert_failure().stdout_eq(str![[r#"
...
Failing tests:
Encountered 1 failing test in test/Foo.t.sol:ContractTest
[FAIL: EVM error; transaction validation error: call gas cost exceeds the gas limit] setUp() ([GAS])
Encountered a total of 1 failing tests, 0 tests succeeded
"#]]);
});

0 comments on commit cb1639f

Please sign in to comment.