Skip to content

start of error handling #134

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

Merged
merged 53 commits into from
Oct 31, 2016
Merged
Show file tree
Hide file tree
Changes from 28 commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
a28453e
start of error handling
steveklabnik Jul 14, 2016
4723782
move towards vectors
steveklabnik Aug 19, 2016
e7b5943
error handling take two
steveklabnik Aug 23, 2016
e35e0e1
Remove 'creating your own error types'
steveklabnik Aug 25, 2016
80c9725
remove second backtrace
steveklabnik Aug 25, 2016
730fb1f
Move explanation up as per @aturon
steveklabnik Aug 25, 2016
3634c81
address some feedback
steveklabnik Aug 25, 2016
ad0fcf3
It's not always possible to handle this at compile time
steveklabnik Aug 25, 2016
0b7bb54
some more small things
steveklabnik Aug 26, 2016
71d4ab9
disable non-nightly build
steveklabnik Aug 26, 2016
bd18fff
don't put my dir in output names
steveklabnik Aug 26, 2016
6f3cf5c
Rename files since we decided errors is chapter 9
carols10cents Aug 31, 2016
9057190
Wording tweaks
carols10cents Aug 31, 2016
3f9dd9d
Explain the code we're going to try a bit more before showing it
carols10cents Aug 31, 2016
e33f886
Technically, panicing isn't the *only* thing we can do here...
carols10cents Aug 31, 2016
b86cf13
Word wrap
carols10cents Aug 31, 2016
f9b690e
Calling panic should be rare; this makes a nice segue
carols10cents Sep 1, 2016
6f581c3
Small wording tweaks
carols10cents Aug 31, 2016
e8cba59
Explain the type parameters a little since this is the first time
carols10cents Sep 1, 2016
0f5a5e6
Leave some notes about ghosting code differences
carols10cents Sep 1, 2016
b2929bf
Use bash syntax highlighting for console output
carols10cents Sep 1, 2016
79bac99
Use parens when referring to fns inline; clarify some things
carols10cents Sep 1, 2016
b88aad0
Make these code examples build off each other
carols10cents Sep 1, 2016
7095fd1
TODO; added some ideas for continuing recoverable section
carols10cents Sep 1, 2016
cd7932f
tools -> features
carols10cents Sep 1, 2016
9154816
splits -> groups
carols10cents Sep 1, 2016
2d40c73
More edits after more readings forever
carols10cents Sep 14, 2016
48c6890
Add a section on deciding how to handle errors
carols10cents Sep 14, 2016
c1c40d5
Rework intro error handling section
carols10cents Oct 11, 2016
e10d974
Update reason why we can't test beta rn
carols10cents Oct 11, 2016
da9422b
Add a sidebar on unwind vs abort
carols10cents Oct 11, 2016
6d2a2c4
Remove distraction about literal 100
carols10cents Oct 11, 2016
e98eab8
Reworking Result examples til I hit the horrible Carrier error
carols10cents Oct 12, 2016
48bed57
Use `try!` for the illustration of calling it in `main`
carols10cents Oct 12, 2016
1eaa218
Finish rearranging the Result section
carols10cents Oct 12, 2016
a3dfedb
Add a discussion of matching on error kind
carols10cents Oct 12, 2016
34b346c
Fix section headers
carols10cents Oct 12, 2016
8f17a2a
Don't talk about `main` in the default recommendation
carols10cents Oct 12, 2016
68bf232
Remove not-quite-accurate parts of this section
carols10cents Oct 12, 2016
261056e
Start of a section on good `unwrap` usage`
carols10cents Oct 12, 2016
9cb2218
Remove a "this" that's not clear
carols10cents Oct 12, 2016
b7a9b8a
Flesh out constructor contract example
carols10cents Oct 13, 2016
c00340c
Small tweaks to wording on another readthrough
carols10cents Oct 17, 2016
6e7eb0e
Discuss that it's ok to `unwrap` if you know it can't fail
carols10cents Oct 17, 2016
211aec8
Add listing numbers for long or referred-to code
carols10cents Oct 17, 2016
0fc14df
Add a summary
carols10cents Oct 17, 2016
2162339
Talk a bit about the contract of Guess
carols10cents Oct 17, 2016
3a8c5ef
Title case titles
carols10cents Oct 18, 2016
fdc98f4
Remove redundant 'file'
carols10cents Oct 18, 2016
0cd5b91
Spelling
carols10cents Oct 18, 2016
ee82377
Whitespace in code
carols10cents Oct 18, 2016
234e758
Make notes since we now know 1.14 will have the question mark
carols10cents Oct 18, 2016
3f72a06
Forward reference to api docs
carols10cents Oct 18, 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
5 changes: 4 additions & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ language: rust
cache: cargo
rust:
- nightly
- stable
# - beta
# ^ to be changed once a new beta is cut that doesn't crash
# - stable
# ^ to be changed once question_mark becomes stable
before_script:
- (cargo install mdbook || true)
script:
Expand Down
5 changes: 4 additions & 1 deletion src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@
- [Strings]()
- [`HashMap<K, V>`]()

- [Error Handling]()
- [Error Handling](ch09-00-error-handling.md)
- [Unrecoverable errors with panic!](ch09-01-unrecoverable-errors-with-panic.md)
- [Recoverable errors with `Result<T, E>`](ch09-02-recoverable-errors-with-result.md)
- [To `panic!` or Not To `panic!`](ch09-03-to-panic-or-not-to-panic.md)
Copy link
Member Author

Choose a reason for hiding this comment

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

this title is in ... title case. The other two sub-titles aren't. Is that just because this is a Shakespeare reference? if so, I'm cool with that, just making sure the others don't need to also be title case

Copy link
Member

Choose a reason for hiding this comment

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

The others do need to be in title case. Good catch!


- [Lifetimes]()

Expand Down
23 changes: 23 additions & 0 deletions src/ch09-00-error-handling.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# Error Handling

Rust's laser-focus on safety spills over into a related area: error handling.
Copy link
Member

Choose a reason for hiding this comment

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

We should start moving away from "safety" and toward "reliability" or "robustness" for this purpose. Safety is a somewhat specialized term -- and is easily thought of as something that higher-level languages just have. What we really want to get at here is that Rust cares about building reliable software across the board.

Errors are a fact of life in software, so Rust has a number of features that you
can use to handle situations in which something bad happens. In many cases,
Rust requires you to acknowledge the possibility of an error occurring and take
some action in that situation. This makes your program safer and more robust by
eliminating the possibility of unexpected errors being discovered late in the
development process.
Copy link
Member

Choose a reason for hiding this comment

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

I think, more importantly, is the danger of only discovering possible errors after you've deployed.


Rust groups errors into two major kinds: errors that are recoverable, and
Copy link
Member

Choose a reason for hiding this comment

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

I still think we want to give some examples or additional intuition about this up front. E.g.:

  • Recoverable errors, like a file not being found, where it is usually reasonable to report something to the user and retry
  • Non-recoverable errors, like an "impossible" situation happening (trying to index out of bounds in an array), which are always symptoms of bugs, for which there's no meaningful recovery.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. I think at least a few examples will help the reader a lot in understanding those two terms. More examples:

  • Recoverable: "the world has an undesired state"
    • incorrect user input
    • no internet connection
  • Unrecoverable: "my program has an undesired state"
    • contract violations (although, I'm not quite sure if it's a good idea mentioning that here already)
    • Divide by zero

Also: maybe set "recoverable" and "unrecoverable" in italic font throughout the whole section?

Copy link
Member

Choose a reason for hiding this comment

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

Also: maybe set "recoverable" and "unrecoverable" in italic font throughout the whole section?

We usually set new terms in italics the first time we use them, when we define them. Then later uses we just use the words regularly.

Copy link
Member

Choose a reason for hiding this comment

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

Are we going with "unrecoverable" or "non-recoverable"?

errors that are not recoverable. What does it mean to "recover" from an
error? In the simplest sense, it relates to the answer of this question:

> If I call a function, and something bad happens, can I do something
> meaningful? Or should execution stop?

The technique that you use depends on the answer to this question. First,
we'll talk about `panic!`, Rust's way of signaling an unrecoverable error.
Then, we'll talk about `Result<T, E>`, the return type for functions that
may return an error you can recover from. Finally, we'll discuss considerations
to take into account when deciding whether to try to recover from an error or
to stop execution.
129 changes: 129 additions & 0 deletions src/ch09-01-unrecoverable-errors-with-panic.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
# Unrecoverable errors with panic!

Sometimes, bad things happen, and there's nothing that you can do about it. For
these cases, Rust has a macro, `panic!`. When this macro executes, your program
will print a failure message and then quit. The most common reason for this is
Copy link
Member

Choose a reason for hiding this comment

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

Hm, so I know there was some contention as to whether to discuss unwinding here, but the current text feels... misleading... especially given that we unwind by default. I'd like to see either a sidebar about unwinding, or (probably better) a forward reference to material on unwinding that can come later. (That could just be a section in this chapter? I don't think we need to delve into a lot of detail; we can point people at the std docs for catch_unwind etc.)

Copy link
Member

Choose a reason for hiding this comment

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

Briefly mentioning unwinding sounds good to me. However, I wouldn't point to catch_unwind. I already see some programmers trying to use it to simulate exceptions. I'd rather say we should keep catch_unwind kind of "hidden" ...

Copy link
Member

Choose a reason for hiding this comment

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

I'd like to see either a sidebar about unwinding, or (probably better) a forward reference to material on unwinding that can come later. (That could just be a section in this chapter? I don't think we need to delve into a lot of detail; we can point people at the std docs for catch_unwind etc.)

I'm not sure we have a great place to discuss unwinding in a later chapter... maybe in the chapter about unsafe rust or FFI? I feel like I see people talking about unwind vs abort a lot in those contexts, but I could be conflating things that aren't actually related.

I could see taking a condensed form of this section in this earlier version of the error handling chapter and making a sidebar on unwind vs abort that briefly explains the difference and points to the docs....

Which would be better, a sidebar here or a section in the later unsafe or FFI chapters?

Copy link
Member

Choose a reason for hiding this comment

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

I think, to start, a short sidebar is a good move.

We can see later on whether there's a logical place to give more details (e.g., talking about unwind safety) -- quite possibly in the FFI chapter.

when a bug of some kind has been detected, and it's not clear how to handle the
error.

Let's try it out with a simple program:

```rust,should_panic
fn main() {
panic!("crash and burn");
}
```

If you run it, you'll see something like this:

```bash
$ cargo run
Compiling panic v0.1.0 (file:///projects/panic)
Finished debug [unoptimized + debuginfo] target(s) in 0.25 secs
Running `target/debug/panic`
thread 'main' panicked at 'crash and burn', src/main.rs:2
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: Process didn't exit successfully: `target/debug/panic` (exit code: 101)
```

There are three lines of error message here. The first line shows our panic
message and the place in our source code where the panic occurred:
`src/main.rs`, line two.

But that only shows us the exact line that called `panic!`. That's not always
useful. Let's modify our example slightly to see what it's like when a `panic!`
call comes from code we call instead of from our code directly:

```rust,should_panic
fn main() {
let v = vec![1, 2, 3];

v[100];
}
```

We attempt to access the hundredth element of our vector, but it only has three
elements. In this situation, Rust will panic. Using `[]` is supposed to return
an element. If you pass `[]` an invalid index, though, there's no element that
Rust could return here that would be correct. In this case, we've typed in a
literal index of `100`, so in theory, Rust could examine our code at compile
Copy link
Member

Choose a reason for hiding this comment

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

I'd drop the aside about the literal index here. Feels like a distraction.

time and raise an error at that point. But if our program was different, say,
maybe reading the index from user input, it would not be possible to determine
at compile time if the index was in bounds.

Other languages like C will attempt to give you exactly what you asked for in
this situation, even though it isn't what you want: you'll get whatever is at
the location in memory that would correspond to that element in the vector,
even though the memory doesn't belong to the vector. This is called a *buffer
overread*, and can lead to security vulnerabilities if an attacker can
manipulate the index in such a way as to read data they shouldn't be allowed to
that is stored after the array.

In order to protect your program from this sort of vulnerability, if you try to
read an element at an index that doesn't exist, Rust will stop execution and
refuse to continue with an invalid value. Let's try it and see:

```bash
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if there is a global policy on this, but I don't think the bash syntax highlighting does any good. It's rather confusing IMO. (this affects other lines too)

Copy link
Member

Choose a reason for hiding this comment

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

Hm. I'm going to file a new issue on this... I'm mostly doing it so that we get a gray box around this block in the mdbook output, but we could probably tweak our CSS to get plain text blocks to have a gray box too.

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

@steveklabnik i think the bash syntax highlighting is highlighting as if these blocks were bash scripts, not bash shell output, which is what we're putting in these blocks. It changes color of words in a seemingly arbitrary way for our output :-/

$ cargo run
Compiling panic v0.1.0 (file:///projects/panic)
Finished debug [unoptimized + debuginfo] target(s) in 0.27 secs
Running `target/debug/panic`
thread 'main' panicked at 'index out of bounds: the len is 3 but the index is
100', ../src/libcollections/vec.rs:1265
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: Process didn't exit successfully: `target/debug/panic` (exit code: 101)
```

This points at a file we didn't write, `../src/libcollections/vec.rs`, line
1265. That's the implementation of `Vec<T>` in the standard library. While it's
easy to see in this short program where the error was, it would be nicer if we
could have Rust tell us what line in our program caused the error.

That's what the next line, the `note` is about. If we set the `RUST_BACKTRACE`
environment variable, we'll get a backtrace of exactly how the error happend.
Let's try that:

```bash
$ RUST_BACKTRACE=1 cargo run
Finished debug [unoptimized + debuginfo] target(s) in 0.0 secs
Running `target/debug/panic`
thread 'main' panicked at 'index out of bounds: the len is 3 but the index is
100', ../src/libcollections/vec.rs:1265
stack backtrace:
1: 0x560956150ae9 -
std::sys::backtrace::tracing::imp::write::h482d45d91246faa2
2: 0x56095615345c -
std::panicking::default_hook::_{{closure}}::h89158f66286b674e
3: 0x56095615291e - std::panicking::default_hook::h9e30d428ee3b0c43
4: 0x560956152f88 -
std::panicking::rust_panic_with_hook::h2224f33fb7bf2f4c
5: 0x560956152e22 - std::panicking::begin_panic::hcb11a4dc6d779ae5
6: 0x560956152d50 - std::panicking::begin_panic_fmt::h310416c62f3935b3
7: 0x560956152cd1 - rust_begin_unwind
8: 0x560956188a2f - core::panicking::panic_fmt::hc5789f4e80194729
9: 0x5609561889d3 -
core::panicking::panic_bounds_check::hb2d969c3cc11ed08
10: 0x56095614c075 - _<collections..vec..Vec<T> as
core..ops..Index<usize>>::index::hb9f10d3dadbe8101
at ../src/libcollections/vec.rs:1265
11: 0x56095614c134 - panic::main::h2d7d3751fb8705e2
at /projects/panic/src/main.rs:4
12: 0x56095615af46 - __rust_maybe_catch_panic
13: 0x560956152082 - std::rt::lang_start::h352a66f5026f54bd
14: 0x56095614c1b3 - main
15: 0x7f75b88ed72f - __libc_start_main
16: 0x56095614b3c8 - _start
17: 0x0 - <unknown>
error: Process didn't exit successfully: `target/debug/panic` (exit code: 101)
```

That's a lot of output! Line `11` there has the line in our project:
`src/main.rs` line four. The key to reading the backtrace is to start from the
top and read until we see files that we wrote: that's where the problem
originated. If we didn't want our program to panic here, this line is where we
would start investigating in order to figure out how we got to this location
with values that cause the panic.

Now that we've covered how to `panic!` to stop our code's execution and how to
debug a `panic!`, let's look at how to instead return and use recoverable
errors with `Result`.
220 changes: 220 additions & 0 deletions src/ch09-02-recoverable-errors-with-result.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,220 @@
# Recoverable errors with `Result<T, E>`

Most errors aren't so dire. Sometimes, when a function fails, it's for a reason
that we can easily interpret and respond to. As an example, maybe we are
making a request to a website, but it's down for maintenance. In this
situation, we'd like to wait and then try again. Terminating our process isn't
the right thing to do here.

In these cases, Rust's standard library provides an `enum` to use as the return
type of the function:

```rust
enum Result<T, E> {
Ok(T),
Err(E),
}
```

The `Ok` variant indicates a successful result, and `Err` indicates an
unsuccessful result. These two variants each contain one thing: in `Ok`'s case,
it's the successful return value. With `Err`, it's some value that represents
the error. The `T` and `E` are generic type parameters; we'll go into generics
in more detail in Chapter XX. What you need to know for right now is that the
`Result` type is defined such that it can have the same behavior for any type
`T` that is what we want to return in the success case, and any type `E` that
is what we want to return in the error case.

As an example, let's try opening a file:

```rust
use std::fs::File;

fn main() {
let f = File::open("hello.txt");
}
```

The `open` function returns a `Result`: there are many ways in which opening
a file can fail. For example, unless we created `hello.txt`, this file does
not yet exist. Before we can do anything with our `File`, we need to extract
it out of the result. Let's start with a basic tool: `match`. We've used it
to deal with enums previously.

<!-- I'll ghost everything except the match statement lines in the libreoffice file /Carol -->

```rust,should_panic
use std::fs::File;

fn main() {
let f = File::open("hello.txt");

let f = match f {
Ok(file) => file,
Err(error) => panic!("There was a problem opening the file: {:?}",
error),
};
}
```

If we see an `Ok`, we can return the inner `file` out of the `Ok` variant. If
we see `Err`, we have to decide what to do with it. The simplest thing is to
turn our error into a `panic!` instead, by calling the macro. And since we
haven't created that file yet, we'll see a message indicating as such when we
print the error value:

```bash
thread 'main' panicked at 'There was a problem opening the file: Error { repr:
Os { code: 2, message: "No such file or directory" } }', src/main.rs:8
```

This works okay. However, `match` can be a bit verbose, and it doesn't always
communicate intent well. The `Result<T, E>` type has many helper methods
defined on it to do various things. "Panic on an error result" is one of those
methods, and it's called `unwrap()`:

<!-- I'll ghost everything except `unwrap()` in the libreoffice file /Carol -->

```rust,should_panic
use std::fs::File;

fn main() {
let f = File::open("hello.txt").unwrap();
}
```

This has the same behavior as our previous example: If the call to `open()`
Copy link
Member

Choose a reason for hiding this comment

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

I know, expect() is explained below, but to say "same behavior" here is a bit confusing, as this code is missing the "There was a problem opening the file" error message.

returns `Ok`, return the value inside. If it's an `Err`, panic.

There's also another method, similar to `unwrap()`, but that lets us choose the
error message: `expect()`. Using `expect()` instead of `unwrap()` and providing
good error messages can convey your intent and make tracking down the source of
a panic easier. `expect()` looks like this:

<!-- I'll ghost everything except `expect()` in the libreoffice file /Carol -->

```rust,should_panic
use std::fs::File;

fn main() {
let f = File::open("hello.txt").expect("failed to open hello.txt");
Copy link
Member

Choose a reason for hiding this comment

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

Why a different error message here than above ("There was a problem opening the file: ")?

Copy link
Member

Choose a reason for hiding this comment

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

Mmmm probably copypasta! :) I'll make them be the same.

Copy link
Member

Choose a reason for hiding this comment

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

Actually, I think the reason is because we'd have to have format! in here to be able to do interpolation. I'll hardcode the other error message instead.

Copy link
Member

Choose a reason for hiding this comment

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

UGH but then you don't get the Err value....

}
```

This isn't the only way to deal with errors, however. This entire section is
supposed to be about recovering from errors, but we've gone back to panic. This
observation gets at an underlying truth: you can easily turn a recoverable
error into an unrecoverable one with `unwrap()` or `expect()`, but you can't
turn an unrecoverable `panic!` into a recoverable one. This is why good Rust
code chooses to make errors recoverable: you give your caller choices.

The Rust community has a love/hate relationship with `unwrap()` and `expect()`.
Copy link
Member

Choose a reason for hiding this comment

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

I'd also add that unwrap and expect are handy for early prototyping -- they leave very clear markers where an error is being ignored, which you can go back around and audit later when you want to improve the robustness of your code.

They're useful in tests since they will cause the test to fail if there's an
error anyplace you call them. In examples, you might not want to muddy the code
with proper error handling. But if you use them in a library, mis-using your
library can cause other people's programs to halt unexpectedly, and that's not
very user-friendly.

## Propagating errors with `?`

When writing a function, if you don't want to handle the error where you are,
you can return the error to the calling function. Within your function, that
would look like:

<!-- I'll ghost everything except `return Err(e)` in the libreoffice file /Carol -->

```rust,ignore
# use std::fs::File;
# fn foo() -> std::io::Result<()> {
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, why isn't foo shown here? It's new code -- we've shifted to a function context, and the return type is important.

Note also that you might want to avoid using the io::Result alias here, because it's likely to be confusing. I'd instead use the normal Result type and fill in the error component with io's error type.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure, I'll fix it!

let f = File::open("hello.txt");

let f = match f {
Ok(file) => file,
Err(e) => return Err(e),
};

# Ok(())
# }
```

This is a very common way of handling errors: propagate them upward until
you're ready to deal with them. This pattern is so common in Rust that there is
dedicated syntax for it: the question mark operator. We could have also written
the example like this:

<!-- I'll ghost everything except `?` in the libreoffice file /Carol -->

```rust,ignore
#![feature(question_mark)]

use std::fs::File;

fn main() {
let f = File::open("hello.txt")?;
}
```

The `?` operator at the end of the `open` call does the same thing as our
previous example: It will return the value inside an `Ok` to the binding `f`,
but will return early out of the whole function and give any `Err` value we get
to our caller.

There's one problem though; let's try compiling the example:

```rust,ignore
Compiling result v0.1.0 (file:///projects/result)
error[E0308]: mismatched types
--> src/main.rs:6:13
|
6 | let f = File::open("hello.txt")?;
| ^^^^^^^^^^^^^^^^^^^^^^^^ expected (), found enum
`std::result::Result`
|
= note: expected type `()`
= note: found type `std::result::Result<_, _>`

error: aborting due to previous error
```

What gives? The issue is that the `main()` function has a return type of `()`,
Copy link
Member

Choose a reason for hiding this comment

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

This is confusing -- it's very unclear whether your examples have shifted to talking about a helper function or in main.

In particular, if you're using return Err(e) above, you need to have already dealt with this point.

Copy link
Member

Choose a reason for hiding this comment

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

I agree. It's confusing to first talk about this foo() function (with the manually-match-err-return) and then, to show the short hand notation, switch to main().

Actually, I would already start with this process_file() function you mention further down. Or what about a slightly different function that actually returns a value on success (then we can avoid the explanation about the () return value, too). Maybe read_username_from_file() -> Result<String, io::Error> or something similar?

In that function, I would first show how to do it manually with match (just as you did). Next I would mention try!() (it has been around for a long time and will remain in many code bases for even longer) and how it simply expands to this hand written match. (It might be desirable to mention the From conversion here, but I'm afraid that it will be more confusing than helpful at this point). And after mentioning try!(), I would mention the special ? syntax.

Those three things are all in the non-main() function. Only now I would show this possible compiler error that occurs when using try!()/? in a function not returning Result<_, _>.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think we started the book using a lot of examples that started in a "not working" state and we walked through how to get them working, but this has been confusing a lot of people. We have been changing to showing the working cases first and then talk about cases that might not work-- I'll change this one too.

but the question mark operator is trying to return a `Result`. This doesn't
work. Instead of `main()`, let's create a function that returns a `Result` so
that we are allowed to use the question mark operator:

```rust
#![feature(question_mark)]

use std::fs::File;
use std::io;

pub fn process_file() -> Result<(), io::Error> {
let f = File::open("hello.txt")?;

// do some stuff with f

Ok(())
}
```

Since the `Result` type has two type parameters, we need to include them both
in our function signature. In this case, `File::open` returns `std::io::Error`,
so we will use it as our error type. But what about success? This function is
executed purely for its side effects; no value is returned when everything
works. Functions with no return type, as we just saw with `main()`, are the
same as returning the unit type, `()`. So we can use the unit type as the
return type here, too.

This leads us to the last line of the function, the slightly silly-looking
`Ok(())`. This is an `Ok()` with a `()` value inside.

Now we have the function `process_file` that uses the question mark operator and compiles successfully. We can call this function and handle the return value in any other function that returns a `Result` by also using the question mark operator there, and that would look like `process_file()?`. However, the `main` function still returns `()`, so we would still have to use a `match` statement, an `unwrap()`, or an `expect()` if we wanted to call `process_file` from `main`. Using `expect()` would look like this:
Copy link
Member

Choose a reason for hiding this comment

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

In general this section is pretty good. But one thing you don't illustrate is the error component of the Result type. In particular, it might be good to show an example where you match on it to discover that you're getting a file-not-found error, or something like that. And to emphasize that it's common to use an enum for the error type, so that you can signal multiple kinds of errors.


```rust,ignore
fn main() {
process_file().expect("There was a problem processing the file");
}
```

## Chaining Question Mark Operators

TODO
Copy link
Member

Choose a reason for hiding this comment

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

I'm still working on adding an example here along the lines of the one mitsuhiko quotes in discussions around ? that shows chaining, but only using stdlib of course. Thinking on it, will try to get one in tomorrow.

Loading