Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: dedup error messages #9481

Merged
merged 1 commit into from
Dec 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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::from(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
"#]]);
});
Loading