Skip to content

Commit

Permalink
Auto merge of #9600 - danlrobertson:opts_parser_err, r=ecoal95
Browse files Browse the repository at this point in the history
Add informative error messages when parsing command line options returns an error

The background behind why I'm submitting this PR is slightly embarrassing. After running `./mach` with some servo options I got the following stack backtrace.

```
thread '<main>' panicked at 'called `Result::unwrap()` on an `Err` value: ParseIntError { kind: InvalidDigit }', ../src/libcore/result.rs:746
stack backtrace:
   1:     0x56459001b0b0 - sys::backtrace::tracing::imp::write::hb120982054a416e35nu
   2:     0x56459001e02b - panicking::default_handler::_$u7b$$u7b$closure$u7d$$u7d$::closure.42840
   3:     0x56459001dc96 - panicking::default_handler::h7ae2e4523ef4c187IFy
   4:     0x56459000760c - sys_common::unwind::begin_unwind_inner::h5fb19481d14902dbDgt
   5:     0x564590007cf8 - sys_common::unwind::begin_unwind_fmt::hd62d57279546b0f2Jft
   6:     0x56459001a701 - rust_begin_unwind
   7:     0x56459004c01f - panicking::panic_fmt::h27f7225e08792f40qYL
   8:     0x56458f393901 - result::unwrap_failed::h18215968003310890981
                        at ../src/libcore/macros.rs:29
   9:     0x56458f3937b9 - result::Result<T, E>::unwrap::h10319050269194353824
                        at ../src/libcore/result.rs:687
  10:     0x56458f383701 - opts::from_cmdline_args::h8ea8d8c87dc6ee726lg
                        at /home/drobertson/git/servo/components/util/opts.rs:599
  11:     0x56458d1f8cdf - main::h8517eb49d15e90fbNaa
                        at /home/drobertson/git/servo/components/servo/main.rs:53
  12:     0x56459001d8f4 - sys_common::unwind::try::try_fn::h9076850075504893162
  13:     0x56459001a68b - __rust_try
  14:     0x56459001d36e - rt::lang_start::h58a22f304b0c1e19Oxy
  15:     0x56458d2f4aa9 - main
  16:     0x7f370496360f - __libc_start_main
  17:     0x56458d1c04a8 - _start
  18:                0x0 - <unknown>
Servo exited with return value 101
```

I immediately opened up `gdb` and got to work only to realize that the error was due to a typo in an option that had been `unwrap`ed at [components/util/opts.rs:599](https://github.com/servo/servo/blob/master/components/util/opts.rs#L599). Perhaps some more informative error messages will help prevent some future face-palm moments like mine 😄

I couldn't think of a good way to add tests for this. Please let me know if I missed something. Comments and critiques are welcome!

<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.svg" height="40" alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/9600)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Feb 16, 2016
2 parents 8199bba + 4d38b82 commit 7aedb9c
Showing 1 changed file with 12 additions and 9 deletions.
21 changes: 12 additions & 9 deletions components/util/opts.rs
Expand Up @@ -587,31 +587,32 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
};

let tile_size: usize = match opt_match.opt_str("s") {
Some(tile_size_str) => tile_size_str.parse().unwrap(),
Some(tile_size_str) => tile_size_str.parse().expect("Error parsing option: -s"),
None => 512,
};

let device_pixels_per_px = opt_match.opt_str("device-pixel-ratio").map(|dppx_str|
dppx_str.parse().unwrap()
dppx_str.parse().expect("Error parsing option: --device-pixel-ratio")
);

let mut paint_threads: usize = match opt_match.opt_str("t") {
Some(paint_threads_str) => paint_threads_str.parse().unwrap(),
Some(paint_threads_str) => paint_threads_str.parse().expect("Error parsing option: -t"),
None => cmp::max(num_cpus::get() * 3 / 4, 1),
};

// If only the flag is present, default to a 5 second period for both profilers.
let time_profiler_period = opt_match.opt_default("p", "5").map(|period| {
period.parse().unwrap()
period.parse().expect("Error parsing option: -p")
});

let mem_profiler_period = opt_match.opt_default("m", "5").map(|period| {
period.parse().unwrap()
period.parse().expect("Error parsing option: -m")
});

let gpu_painting = !FORCE_CPU_PAINTING && opt_match.opt_present("g");

let mut layout_threads: usize = match opt_match.opt_str("y") {
Some(layout_threads_str) => layout_threads_str.parse().unwrap(),
Some(layout_threads_str) => layout_threads_str.parse().expect("Error parsing option: -y"),
None => cmp::max(num_cpus::get() * 3 / 4, 1),
};

Expand All @@ -625,16 +626,18 @@ pub fn from_cmdline_args(args: &[String]) -> ArgumentParsingResult {
}

let devtools_port = opt_match.opt_default("devtools", "6000").map(|port| {
port.parse().unwrap()
port.parse().expect("Error parsing option: --devtools")
});

let webdriver_port = opt_match.opt_default("webdriver", "7000").map(|port| {
port.parse().unwrap()
port.parse().expect("Error parsing option: --webdriver")
});

let initial_window_size = match opt_match.opt_str("resolution") {
Some(res_string) => {
let res: Vec<u32> = res_string.split('x').map(|r| r.parse().unwrap()).collect();
let res: Vec<u32> = res_string.split('x').map(|r| {
r.parse().expect("Error parsing option: --resolution")
}).collect();
Size2D::typed(res[0], res[1])
}
None => {
Expand Down

0 comments on commit 7aedb9c

Please sign in to comment.