-
Notifications
You must be signed in to change notification settings - Fork 425
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
Convert all errors to use error-chain!{..} #17
Comments
There are various uses of I have to admit I can't figure out an alternative. I got the impression fn load_zone(zone_dir: &Path, zone: &ZoneConfig) -> Result<Authority, String> {
let zone_name: Name = zone.get_zone().expect("bad zone name"); should become fn load_zone(zone_dir: &Path, zone: &ZoneConfig) -> Result<Authority, String> {
let zone_name: Name = try!(zone.get_zone().chain_err("bad zone name")); but it doesn't compile, even with p.s. I'm commenting here rather than raising a new issue because I'm not sure whether this is as-designed or not. |
I'll try and answer this question as best I can. First on the usage of expect/unwrap/panic in named. This is a top level binary. I've only expected it to be run through the main() method in binary form. To me, this means that given that any error in loading config will end up causing the application to exit abnormally, is fine. For anyone trying to use this as I library, where unwraps/expects should all be guarded against, I never intended them to call into Now, we could clean this up in As to your issue with the |
OK, thanks for clarifying: panic!() from named should probably be cleaned up for nicer UX, though it's not critical. That's the way I look at it too. Here's hoping for time to look into it further. |
This should simplify error handling in Trust-DNS
The text was updated successfully, but these errors were encountered: