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

Completely overhaul error handling #358

Merged
merged 68 commits into from
Apr 25, 2016
Merged
Show file tree
Hide file tree
Changes from 60 commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
82212cb
Move notifications to their own modules
brson Apr 21, 2016
8e76c56
Add the rustup-error crate
brson Apr 22, 2016
4dd554c
Convert rustup-utils to easy_error
brson Apr 22, 2016
357f61b
Use chain_error in rustup utils to avoid throwing away errors
brson Apr 22, 2016
74129d4
Lots of refactoring for ergonomics
brson Apr 22, 2016
b95261f
Define rust-dist Error2 type with easy_error
brson Apr 22, 2016
d1756c5
Build ErrorChain in the easy_error macro
brson Apr 22, 2016
62f85e2
Remove the non-pub version of the macro. Bothersome to maintain
brson Apr 22, 2016
15278d9
Tweak the easy_error! syntax to make room for more declaration types
brson Apr 22, 2016
81f6da3
Specify the error chain name in the easy_error syntax
brson Apr 22, 2016
7cf11b8
Remove extra type parameter from ErrorChain
brson Apr 23, 2016
20b101f
Just move the ChainError trait into the macro
brson Apr 23, 2016
9617ef9
Convert rustup-dist to rustup-error
brson Apr 23, 2016
b742d8c
Introduce ForeignError
brson Apr 23, 2016
e6f3d2c
Remove unused from
brson Apr 23, 2016
7964a3c
Remove Implementation of StdError from error types
brson Apr 23, 2016
1c76430
Convert rustup to rustup-error
brson Apr 23, 2016
5830eb9
Remove explicit attributes from easy_error
brson Apr 23, 2016
6d479d1
Remove pub modifiers from easy_error syntax
brson Apr 23, 2016
f8c3144
Simplify macro syntax further
brson Apr 23, 2016
386cf30
Remove unused macro
brson Apr 23, 2016
20f00c7
Put from conversions into the macro
brson Apr 23, 2016
8457586
Port tests
brson Apr 23, 2016
ef99cfd
Move Result typedef into the macro
brson Apr 23, 2016
4bf2114
More macro syntax tweaking
brson Apr 23, 2016
9907aab
Rename Error::Install to Error::Dist
brson Apr 23, 2016
f309646
Swap type declaration order
brson Apr 23, 2016
5fa31ce
Fix some unix compile errors
brson Apr 23, 2016
78f9cb7
Rename ErrorChain to Error, Error to ErrorKind
brson Apr 23, 2016
e3ea815
from_links -> links
brson Apr 23, 2016
c632b2c
Remove unused variant
brson Apr 23, 2016
6b70c8a
Anything implementing Into can be chained
brson Apr 23, 2016
e472c4d
from_link -> link
brson Apr 23, 2016
2cd6175
Convert some errors to basic errors
brson Apr 23, 2016
c8983f4
chain_error -> chain_err
brson Apr 23, 2016
f301e2b
Add an 'err' function to the macro for initializing the error chain
brson Apr 23, 2016
948203a
Remove unused error variants
brson Apr 23, 2016
370dca5
Cleanup the macro
brson Apr 24, 2016
35bc8a9
Remove the err function in favor Into
brson Apr 24, 2016
8021be1
Replace trivial error types in rustup-dist
brson Apr 24, 2016
ca8baae
Give rustup-cli its own errors
brson Apr 24, 2016
55b6157
Add backtraces
brson Apr 24, 2016
9f13f4e
Remove the unchained method
brson Apr 24, 2016
7de5e05
Remove an Error::from
brson Apr 24, 2016
251a36c
wip
brson Apr 24, 2016
df4f117
Remove the 'chained' method
brson Apr 24, 2016
ce445cb
Fix some macro variable names
brson Apr 24, 2016
07d75ea
Whitespace
brson Apr 24, 2016
2b13273
Propagate backtraces
brson Apr 24, 2016
1cc995b
Write some docs
brson Apr 24, 2016
03ad55b
Add an iter method to Error to iterate over the error chain
brson Apr 24, 2016
c829831
Convert line endings
brson Apr 24, 2016
fabe758
Allow dead Windows-specific variants
brson Apr 24, 2016
4540b9a
Print the error chain and (optionally) backtrace on error
brson Apr 24, 2016
ea5a8fe
Rename inner method to 'kind'
brson Apr 24, 2016
e57aeb8
Put the Backtrace in an Arc
brson Apr 24, 2016
968c1c4
Simplify backtrace propagation
brson Apr 24, 2016
6a4bbef
Simplify foreign errors
brson Apr 25, 2016
a062ea3
More docs
brson Apr 25, 2016
d42d7dc
Fix some tests
brson Apr 25, 2016
7eb3be5
Remove some unused type inference hacks
brson Apr 25, 2016
c5745df
Remove stray Cargo.lock
brson Apr 25, 2016
1e0c68a
Typos
brson Apr 25, 2016
820ce07
Combine error chain and backtrace into a single field of Error
brson Apr 25, 2016
5d76f86
Small tweaks
brson Apr 25, 2016
d33e4a3
Rename rustup-error to error-chain
brson Apr 25, 2016
c04b430
Rename declare_errors to error_chain
brson Apr 25, 2016
6cefd93
Small doc improvements
brson Apr 25, 2016
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
45 changes: 45 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ build = "build.rs"
[dependencies]
rustup-dist = { path = "src/rustup-dist" }
rustup-utils = { path = "src/rustup-utils" }
rustup-error = { path = "src/rustup-error" }
clap = "2.2.4"
regex = "0.1.41"
openssl = "0.7.2"
Expand Down
48 changes: 41 additions & 7 deletions src/rustup-cli/common.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Just a dumping ground for cli stuff

use rustup::{Cfg, Result, Notification, Toolchain, Error, UpdateStatus};
use rustup::{self, Cfg, Notification, Toolchain, UpdateStatus};
use errors::*;
use rustup_utils::utils;
use rustup_utils::notify::NotificationLevel;
use self_update;
Expand Down Expand Up @@ -87,7 +88,8 @@ pub fn read_line() -> Result<String> {
let stdin = std::io::stdin();
let stdin = stdin.lock();
let mut lines = stdin.lines();
lines.next().and_then(|l| l.ok()).ok_or(Error::ReadStdin)
lines.next().and_then(|l| l.ok()).ok_or(
"unable to read from stdin for confirmation".into())
}

pub fn set_globals(verbose: bool) -> Result<Cfg> {
Expand All @@ -96,8 +98,8 @@ pub fn set_globals(verbose: bool) -> Result<Cfg> {

let download_tracker = RefCell::new(DownloadTracker::new());

Cfg::from_env(shared_ntfy!(move |n: Notification| {
if download_tracker.borrow_mut().handle_notification(&n) {
Ok(try!(Cfg::from_env(shared_ntfy!(move |n: Notification| {
if download_tracker.borrow_mut().handle_notification(&n) {
return;
}

Expand All @@ -117,16 +119,16 @@ pub fn set_globals(verbose: bool) -> Result<Cfg> {
err!("{}", n);
}
}
}))
}))))

}

pub fn show_channel_update(cfg: &Cfg, name: &str,
updated: Result<UpdateStatus>) -> Result<()> {
updated: rustup::Result<UpdateStatus>) -> Result<()> {
show_channel_updates(cfg, vec![(name.to_string(), updated)])
}

fn show_channel_updates(cfg: &Cfg, toolchains: Vec<(String, Result<UpdateStatus>)>) -> Result<()> {
fn show_channel_updates(cfg: &Cfg, toolchains: Vec<(String, rustup::Result<UpdateStatus>)>) -> Result<()> {
let data = toolchains.into_iter().map(|(name, result)| {
let ref toolchain = cfg.get_toolchain(&name, false).expect("");
let version = rustc_version(toolchain);
Expand Down Expand Up @@ -348,3 +350,35 @@ fn split_override<T: FromStr>(s: &str, separator: char) -> Option<(T, T)> {
}
})
}

pub fn report_error(e: &Error) {
err!("{}", e);

for e in e.iter().skip(1) {
info!("caused by: {}", e);
}

if show_backtrace() {
info!("backtrace:");
println!("");
println!("{:?}", e.backtrace());
} else {
}

fn show_backtrace() -> bool {
use std::env;
use std::ops::Deref;

if env::var("RUST_BACKTRACE").as_ref().map(Deref::deref) == Ok("1") {
return true;
}

for arg in env::args() {
if arg == "-v" || arg == "--verbose" {
return true;
}
}

return false;
}
}
47 changes: 47 additions & 0 deletions src/rustup-cli/errors.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
#![allow(dead_code)]

use std::path::PathBuf;

use rustup;
use rustup_dist::{self, temp};
use rustup_utils;

declare_errors! {
types {
Error, ErrorKind, ChainErr, Result;
}

links {
rustup::Error, rustup::ErrorKind, Rustup;
rustup_dist::Error, rustup_dist::ErrorKind, Dist;
rustup_utils::Error, rustup_utils::ErrorKind, Utils;
}

foreign_links {
temp::Error, Temp,
"temporary file error";
}

errors {
PermissionDenied {
description("permission denied")
}
ToolchainNotInstalled(t: String) {
description("toolchain is not installed")
display("toolchain '{}' is not installed", t)
}
InfiniteRecursion {
description("infinite recursion detected")
}
NoExeName {
description("couldn't determine self executable name")
}
NotSelfInstalled(p: PathBuf) {
description("rustup is not installed")
display("rustup is not installed at '{}'", p.display())
}
WindowsUninstallMadness {
description("failure during windows uninstall")
}
}
}
16 changes: 11 additions & 5 deletions src/rustup-cli/main.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
#![recursion_limit = "1024"]
Copy link
Member

Choose a reason for hiding this comment

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

Is this due to the macro recursing on itself?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.


extern crate rustup_dist;
#[macro_use]
extern crate rustup_utils;
#[macro_use]
extern crate rustup_error;

#[macro_use]
extern crate clap;
Expand Down Expand Up @@ -39,14 +43,15 @@ mod self_update;
mod tty;
mod job;
mod term2;
mod errors;

use std::env;
use std::path::PathBuf;
use rustup::{Error, Result};
use errors::*;

fn main() {
if let Err(e) = run_multirust() {
err!("{}", e);
if let Err(ref e) = run_multirust() {
common::report_error(e);
std::process::exit(1);
}
}
Expand All @@ -56,7 +61,7 @@ fn run_multirust() -> Result<()> {
let recursion_count = env::var("RUST_RECURSION_COUNT").ok()
.and_then(|s| s.parse().ok()).unwrap_or(0);
if recursion_count > 5 {
return Err(Error::InfiniteRecursion);
return Err(ErrorKind::InfiniteRecursion.into());
}

// Do various things to clean up past messes
Expand Down Expand Up @@ -111,7 +116,7 @@ fn run_multirust() -> Result<()> {
}
None => {
// Weird case. No arg0, or it's unparsable.
Err(Error::NoExeName)
Err(ErrorKind::NoExeName.into())
}
}
}
Expand Down Expand Up @@ -166,3 +171,4 @@ fn fix_windows_reg_key() {

#[cfg(not(windows))]
fn fix_windows_reg_key() { }

19 changes: 10 additions & 9 deletions src/rustup-cli/multirust_mode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ use cli;
use common::{self, confirm, set_globals,
show_channel_update, show_tool_versions,
update_all_channels};
use rustup::*;
use errors::*;
use rustup::{Cfg, Notification, command, utils, Toolchain};
use rustup_dist::manifest::Component;
use rustup_dist::dist::TargetTriple;
use self_update;
Expand Down Expand Up @@ -48,7 +49,7 @@ pub fn main() -> Result<()> {
("remove-target", Some(m)) => remove_target(&cfg, m),
("run", Some(m)) => run(&cfg, m),
("proxy", Some(m)) => proxy(&cfg, m),
("upgrade-data", Some(_)) => cfg.upgrade_data().map(|_| ()),
("upgrade-data", Some(_)) => Ok(try!(cfg.upgrade_data().map(|_| ()))),
("delete-data", Some(m)) => delete_data(&cfg, m),
("self", Some(c)) => {
match c.subcommand() {
Expand Down Expand Up @@ -79,14 +80,14 @@ fn run(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
let args = m.values_of("command").unwrap();
let args: Vec<_> = args.collect();
let cmd = try!(toolchain.create_command(args[0]));
command::run_command_for_dir(cmd, &args, &cfg)
Ok(try!(command::run_command_for_dir(cmd, &args, &cfg)))
}

fn proxy(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
let args = m.values_of("command").unwrap();
let args: Vec<_> = args.collect();
let cmd = try!(cfg.create_command_for_dir(&try!(utils::current_dir()), args[0]));
command::run_command_for_dir(cmd, &args, &cfg)
Ok(try!(command::run_command_for_dir(cmd, &args, &cfg)))
}

fn command_requires_metadata() -> Result<bool> {
Expand Down Expand Up @@ -120,11 +121,11 @@ fn self_update() -> Result<()> {
}

fn get_toolchain<'a>(cfg: &'a Cfg, m: &ArgMatches, create_parent: bool) -> Result<Toolchain<'a>> {
cfg.get_toolchain(m.value_of("toolchain").unwrap(), create_parent)
Ok(try!(cfg.get_toolchain(m.value_of("toolchain").unwrap(), create_parent)))
}

fn remove_toolchain_args(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
try!(get_toolchain(cfg, m, false)).remove()
Ok(try!(try!(get_toolchain(cfg, m, false)).remove()))
}

fn default_(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
Expand All @@ -133,7 +134,7 @@ fn default_(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
if !toolchain.is_custom() {
Some(try!(toolchain.install_from_dist_if_not_installed()))
} else if !toolchain.exists() {
return Err(Error::ToolchainNotInstalled(toolchain.name().to_string()));
return Err(ErrorKind::ToolchainNotInstalled(toolchain.name().to_string()).into());
} else {
None
}
Expand All @@ -158,7 +159,7 @@ fn update(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
if !toolchain.is_custom() {
Some(try!(toolchain.install_from_dist()))
} else if !toolchain.exists() {
return Err(Error::ToolchainNotInstalled(toolchain.name().to_string()));
return Err(ErrorKind::ToolchainNotInstalled(toolchain.name().to_string()).into());
} else {
None
}
Expand Down Expand Up @@ -224,7 +225,7 @@ fn doc_url(m: &ArgMatches) -> &'static str {
}

fn doc(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
cfg.open_docs_for_dir(&try!(utils::current_dir()), doc_url(m))
Ok(try!(cfg.open_docs_for_dir(&try!(utils::current_dir()), doc_url(m))))
}

fn which(cfg: &Cfg, m: &ArgMatches) -> Result<()> {
Expand Down
7 changes: 4 additions & 3 deletions src/rustup-cli/proxy_mode.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use common::set_globals;
use rustup::{Cfg, Result, Error};
use rustup::{Cfg};
use errors::*;
use rustup_utils::utils;
use rustup::command::run_command_for_dir;
use std::env;
Expand All @@ -15,7 +16,7 @@ pub fn main() -> Result<()> {
let arg0 = arg0.as_ref()
.and_then(|a| a.file_name())
.and_then(|a| a.to_str());
let ref arg0 = try!(arg0.ok_or(Error::NoExeName));
let ref arg0 = try!(arg0.ok_or(ErrorKind::NoExeName));

let cfg = try!(set_globals(false));
try!(cfg.check_metadata_version());
Expand All @@ -28,6 +29,6 @@ fn direct_proxy(cfg: &Cfg, arg0: &str) -> Result<()> {
let cmd = try!(cfg.create_command_for_dir(&try!(utils::current_dir()), arg0));
let args: Vec<_> = env::args_os().collect();

run_command_for_dir(cmd, &args, &cfg)
Ok(try!(run_command_for_dir(cmd, &args, &cfg)))
}

Loading