-
Notifications
You must be signed in to change notification settings - Fork 177
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
Improve error handling in anomac utils join-network
#1050
Conversation
apps/src/lib/client/utils.rs
Outdated
eprintln!( | ||
"The arg `--wasm-dir` cannot be an absolute path. It is \ | ||
nested inside the chain directory." | ||
); | ||
cli::safe_exit(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could be changed to returning something like Err("The arg --wasm-dir cannot be an absolute path. It is nested inside the chain directory.")
apps/src/lib/client/utils.rs
Outdated
}) | ||
.await | ||
.unwrap(); | ||
exit_if_error(config.write(&base_dir, &chain_id, true)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NB: having to call an exit_if_error
function here directly as we are running this block of code in another thread IIUC, though thinking it might be ok to run this block in the same thread as the rest of the function
.iter() | ||
.enumerate() | ||
.filter_map(|(index, peer)| | ||
// we do not add the validator in its own persistent peer list | ||
if index != ix { | ||
Some(peer.to_owned()) | ||
} else { | ||
None | ||
}) | ||
.collect(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated formatting change
cli::safe_exit(1); | ||
} | ||
|
||
// TODO: can these two exit_if_error helpers be combined? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can, maybe with map_err(AsRef::as_ref)
on the result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried some things out with map_err
though was getting compiler errors about returning a reference to an owned error, will have to come back to this
This PR won't be merged but may still push things here for discussion |
Some ideas for error handling, to discuss later on as part of #1049