add "Obtain backtrace of complex error scenarios" example

[ludwigpacifici]

Issue #216

[ludwigpacifici]

Hello @budziq

Here is my attempt to show a "complex" backtrace with a relatively short code. Let me know if it is a good candidate for this issue.

Edit: the snippet generates an error on purpose and it looks like the CI is not happy about it.

[budziq]

Thanks! Few comments below.

I'll do a more thorough review tommorow

[budziq]

Thanks! Few comments below.

I'll do a more thorough review tommorow

[budziq]

please use wildcard version numbers

[budziq]

lets not use the quick_main! here as it will error out the application.
lets have a regular main in addition to run to showcase usage of
backtrace in order to iterate over the backtrace

[budziq]

we can substitute this fun tion with custom Display impl for Rgb

[ludwigpacifici]

I don't understand you comment. Can you give more details?

[ludwigpacifici]

custom Display impl for Rgb

Ah, I think I get it :-) Do you mean something like:

let color = Rgb{...};
println!("#{}", color); // Prints #66FFCC

rgb_to_hex will be replace by the Display impl for Rgb?

Edit: Will it make more sense to implement std::fmt::UpperHex?

[budziq]

Good point 👍

[budziq]

Ok, review is finished. Sorry for doing it in parts

[budziq]

Ok, review is finished. Sorry for doing it in parts

[budziq]

not sure about using both ok_or and chain_err at the same time One of these would suffice.

[ludwigpacifici]

I have to use ok_or and chain_err to make it work. I tried to be concise, maybe too much? Here is a more verbose version:

let mut reader = csv::Reader::from_reader(csv_data);
let mut it = reader.deserialize();
let option = it.nth(0);
if let Some(inner) = option {
    // Notice that we need to provide a type hint for automatic deserialization.
    let color: Rgb = inner.chain_err(|| "Cannot deserialize RGB color")?;
    Ok(color)
} else {
    Err(From::from("Cannot deserialize the first CSV record"))
}

Documentation I used: https://docs.rs/csv/1.0.0-beta.4/csv/index.html#example-with-serde

[budziq]

Nope, the original is very good. Honestly my bad I have made this review a too cursory one.

[ludwigpacifici]

No worries @budziq :-) It's good you question my rust code because I am beginner and cookbook PR teaches me a lot.

[budziq]

to bump the size of our backtrace we can use a file input. Provide an associate function from_path which would in turn call associated function from_reader (formerly read_rgb_from_csv)

[ludwigpacifici]

Not sure I understand this comment. from_path is already provided by the csv crate.

The snippet would look like:

    let color: Rgb = csv::ReaderBuilder::new()
        .from_path("color.csv")
        .chain_err(|| "Cannot read CSV file")?
        .deserialize()
        .nth(0)
        .ok_or("Cannot deserialize the first CSV record")?
        .chain_err(|| "Cannot deserialize RGB color")?;

Then it leads to a shorter error stack, if the file does not exist:

Error level - description
└> 0 - Cannot read CSV file
└> 1 - No such file or directory (os error 2)
└> 2 - No such file or directory (os error 2)

Am I missing something?

[budziq]

Again my bad for lack of diligence in the review.

[ludwigpacifici]

That's alright. Maybe the following snippet will make the example more Rust idiomatic?

impl Rgb {
    fn from_reader(csv_data: &[u8]) -> Result<Rgb> {
        // move code of read_rgb_from_csv here
    }
}

and in the run function, the line will be:

let rgb = Rgb::from_reader(csv.as_bytes()).chain_err(|| "Cannot read CSV data")?;

The error stack is unchanged. What do you think?

[budziq]

yep that's cool 👍 the comment slipped past me

[budziq]

I would start with mentioning that we have a complex error handling scenario and we present a backtrace from it.

[budziq]

Also I would present the example backtrace output after the code snippet.

[ludwigpacifici]

Isn't it redundant if the reader hit the play button?

[budziq]

We will be only able to (easily) simulate one error scenario (file not found). We might decide to show backtraces from other error scenarios too. But we will see how the example unfolds :)

[ludwigpacifici]

Thanks @budziq for your review. I am on a weekend, I expect to correct the PR beginning of the week.

[ludwigpacifici]

I updated the PR following most of your comments. Still open to discussion:

• usage of ok_or and chain_err
• usage of from_path

[budziq]

That is an exemplary work @ludwigpacifici !
Lets just make one of the two possible changes to make our CI/test runner happy and we are ready to merge!

[budziq]

That is an exemplary work @ludwigpacifici !
Lets just make one of the two possible changes to make our CI/test runner happy and we are ready to merge!

[budziq]

I'd just drop this line to make our CI happy or make the example no_run. We can always write a comment here mentioning that we would normally handle the errors or leave the app here with and error.

[ludwigpacifici]

The CI is trying to execute the recipes by default, meaning it checks the code compiles at least. If no_run is set, is the code still compiled?

I try to understand which is better no_run vs make the code return a success code.

[budziq]

no_run - compiles the code but does not run it
ignore - does not compile it at all

I'd probably go with leaving this example runnable but removing the ::std::process::exit(1); but I'll leave it up to you.

[ludwigpacifici]

Thanks for the listing.

I'd probably go with leaving this example runnable but removing the ::std::process::exit(1);

Yes, I'll do that

[ludwigpacifici]

CI is happy :-)

One question left on my side: #325 (comment), and waiting for your suggestions, if you have any.

[ludwigpacifici]

read_rgb_from_csv was moved to from_reader.

I noticed removing process::exit(1) make the CI happy, but when I click on the play button, the program output is empty (error backtrace is not displayed, but it is rendered as text just below the code snippet). Looks like the error output is redirected somewhere else. Should we keep this behavior or add a no_run instead?

[budziq]

Stderr is printed only if playground returns process failure

Let's keep the example runnable and possibly submit a bug report to mdbook (we can also modify it lolly in cookbook but I'd prefer to to avoid it)

[ludwigpacifici]

Alright @budziq. In that case, the PR looks fine on my side. I will raise an issue to mdbook. Should I cc you?

[budziq]

Thanks @ludwigpacifici!

Yep, please cc me on mdbook issue. I'll ad it to my ever expanding todo list ;).

[budziq]

or better yet please create the issue in cookbook for now untill we come up with reasonable solution to propose.

[ludwigpacifici]

Thanks for your time @budziq

I will put it on the ever expanding list of the rust-cookbook :-)