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.

Sign up for GitHub

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

Refactor: error messages to be concise and formatted correctly #40 #184

Closed
wants to merge 1 commit into from
Closed

Refactor: error messages to be concise and formatted correctly #40 #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
40 changes: 23 additions & 17 deletions sim-cli/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,17 @@ async fn main() -> anyhow::Result<()> {
);

if clients.contains_key(&node_info.pubkey) {
anyhow::bail!(LightningError::ValidationError(format!(
"duplicated node: {}.",
node_info.pubkey
)));
anyhow::bail!(
"Lightning Error\nCaused by: {}",
LightningError::ValidationError(format!("duplicated node: {}.", node_info.pubkey))
);
}

if alias_node_map.contains_key(&node_info.alias) {
anyhow::bail!(LightningError::ValidationError(format!(
"duplicated node: {}.",
node_info.alias
)));
anyhow::bail!(
"Lightning Error\nCaused by: {}",
LightningError::ValidationError(format!("duplicated node: {}.", node_info.alias))
);
}

clients.insert(node_info.pubkey, node);
Expand All @@ -143,21 +143,27 @@ async fn main() -> anyhow::Result<()> {
} {
source.clone()
} else {
anyhow::bail!(LightningError::ValidationError(format!(
"activity source {} not found in nodes.",
act.source
)));
anyhow::bail!(
"Lightning Error\nCaused by: {}",
LightningError::ValidationError(format!(
"activity source {} not found in nodes.",
act.source
))
);
};

let destination = match &act.destination {
NodeId::Alias(a) => {
if let Some(info) = alias_node_map.get(a) {
info.clone()
} else {
anyhow::bail!(LightningError::ValidationError(format!(
"unknown activity destination: {}.",
act.destination
)));
anyhow::bail!(
"Lightning Error\nCaused by: {}",
LightningError::ValidationError(format!(
"unknown activity destination: {}.",
act.destination
))
);
}
},
NodeId::PublicKey(pk) => {
Expand All @@ -174,7 +180,7 @@ async fn main() -> anyhow::Result<()> {
.map_err(|e| {
log::debug!("{}", e);
LightningError::ValidationError(format!(
"Destination node unknown or invalid: {}.",
"Lightning Error\nDestination node unknown or invalid: {}.",
pk,
))
})?
Expand Down
16 changes: 8 additions & 8 deletions sim-lib/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ pub struct ActivityDefinition {

#[derive(Debug, Error)]
pub enum SimulationError {
#[error("Lightning Error: {0:?}")]
#[error("Lightning Error")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this just swallow the actual error that happened? I still think we should surface that information, and then handle it in a prettier way in the cli if required?

LightningError(#[from] LightningError),
#[error("TaskError")]
TaskError,
Expand All @@ -195,19 +195,19 @@ pub enum SimulationError {

#[derive(Debug, Error)]
pub enum LightningError {
#[error("Node connection error: {0}")]
#[error("Node connection error\nCaused by: {0}")]
ConnectionError(String),
#[error("Get info error: {0}")]
#[error("Get info error\n{0}")]
GetInfoError(String),
#[error("Send payment error: {0}")]
#[error("Send payment error\n{0}")]
SendPaymentError(String),
#[error("Track payment error: {0}")]
#[error("Track payment error\n{0}")]
TrackPaymentError(String),
#[error("Invalid payment hash")]
InvalidPaymentHash,
#[error("Get node info error: {0}")]
#[error("Get node info error\n{0}")]
GetNodeInfoError(String),
#[error("Config validation failed: {0}")]
#[error("Config validation failed {0}")]
ValidationError(String),
#[error("Permanent error: {0:?}")]
PermanentError(String),
Expand Down Expand Up @@ -466,7 +466,7 @@ impl Simulation {
let node = node.lock().await;
if !node.get_info().features.supports_keysend() {
return Err(LightningError::ValidationError(format!(
"All nodes eligible for random activity generation must support keysend, {} does not",
"All nodes eligible for random activity generation must support keysend,\n{} does not",
node.get_info()
)));
}
Expand Down
2 changes: 1 addition & 1 deletion sim-lib/src/random_activity.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const SECONDS_PER_MONTH: u64 = HOURS_PER_MONTH * 60 * 60;

#[derive(Debug, Error)]
pub enum RandomActivityError {
#[error("Value error: {0}")]
#[error("Value error\nCaused by: {0}")]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we have Caused by here but not below?

ValueError(String),
#[error("InsufficientCapacity: {0}")]
InsufficientCapacity(String),
Expand Down