start of error handling #134

steveklabnik · 2016-07-14T20:56:21Z

I started on the error handling chapter. I feel... okay about it. I dunno.

I decided to go with an example that we start with a panic, and then convert to a result. I think this draws the connection a bit better than showing two totally different examples, but at the downside of showing function panic that probably shouldn't. Reasonable folks may differ on this tradeoff; I'm open to arguments.

I decided to stop here rather than go on to all the From stuff, because I wanted to make sure that this was on track first. Should we use a different example? Is the length of time spent talking about panic vs abort a waste of time right here? How should I end up demonstrating try!, just making another dummy function from main()?

aturon · 2016-07-22T22:01:09Z

cc me

sophiajt · 2016-07-25T23:21:52Z

src/ch07-01-error-handling.md

+# Error Handling
+
+Rust's laser-focus on safety spills over into a related area: error handling.
+Errors are a fact of life in software. Rust has a number of tools that you can


nit: do you mean "language features" where you say "tools"? For some reason I always think of tools as external things that you run on your code...

I mean "a thing you use to accomplish a task", a more general sense than you, I think.

carols10cents · 2016-07-27T00:34:58Z

Just added some commits with some small suggestions I couldn't help making while reading :) Other thoughts:

Unwind vs abort

I'm sure this will come as a shock, but I'm of the opinion that the detail about unwinding vs. aborting is too much for 07-02. The thing that sealed this opinion for me was this sentence:

Handing panics with unwinding is useful in many scenarios, but some applications would rather skip straight to the end, and 'abort'.

My mind wants to put a wikipedia-style [who?] annotation in there after "some applications" :) I'm sure those people know who they are, and you might be intending to have added a more concrete example later, but how does a beginning rustacean discern that they are in this scenario or not?

Is there a section planned for the book that would be about these scenarios (FFI? kernels? i don't even know) that we could move this to? Could we reference the nomicon instead?

assert!

I'm also questioning the inclusion of assert! here... For C folks, I worry that there's no indication of whether the assertions are run when optimizations are on (iirc they are NOT run in C but ARE in Rust by default). For Rubyish folks, I worry that normally assertions are only used in testing, but here they are in not-tests. Sooooo I think the section on assert! could either use these clarifications (which could be as simple as references to the chapters on optimizations and testing) or it should be taken out and not discussed until those other sections.

the example

I decided to go with an example that we start with a panic, and then convert to a result. I think this draws the connection a bit better than showing two totally different examples, but at the downside of showing function panic that probably shouldn't. Reasonable folks may differ on this tradeoff; I'm open to arguments.

What about framing it in more of an iterative software development story? "We got this requirement to make a function to compare numbers between 0 and 100, there's no validation in the UI right now aside from the type checking, so you can totally enter a number greater than 100, but the person asking for this function didn't tell us what the code should do in this case. For now, we're choosing to panic; we're waiting on an answer from them about what we should do, but this prevents us from proceeding with potentially invalid input"? Also sort of contrived, and not the only reason you might panic and then switch to recovering, but it is one reason.

This sentence:

It's kind of a silly function, but we need an example, so it works.

gives me the sense that you're not entirely happy with this example :) I'll think on this and see if I get any ideas for something else.

try!

How should I end up demonstrating try!, just making another dummy function from main()?

Yeah, I think it would be good to have main call a function that calls check_guess. Maybe that function calls check_guess multiple times, or calls multiple functions that return Result to show the early-return aspect of try!?

sophiajt · 2016-08-17T15:40:34Z

src/ch07-01-error-handling.md

+number of tools that you can use when something bad happens to handle the
+situation appropriately.
+
+Rust splits errors into two major kinds: errors that are recoverable and


nit: "groups" errors might read a little better.

steveklabnik · 2016-08-19T21:29:54Z

Okay, I've removed a bunch of the stuff about unwind vs abort, I've added in the start of a vector example.

We also wanted to talk about unwrap...

steveklabnik · 2016-08-23T18:55:54Z

Okay, so. I decided to totally re-write this. Here's a summary:

We now focus on showing off functions that already handle errors this way, rather than writing a ton of our own. This general structure is this:

panic!
vector indexing causing panic!
File::open returning Result
converting that into a panic with unwrap
propagating the error with ?.
connecting that to io::Error and io::Result as a way to talk about how to write libraries

that last part isn't there yet, I wanted to take the temperature on this approach first. One of the biggest changes here is that we don't talk about try! at all. ? is going to hit stable before the book is published, likely in 1.13, and so I think it makes sense to focus just on it. If they understand ? and see a try!, they'll have the tools to figure it out, and we can talk about the connection to ? in try!'s docs to help as well.

I also totally scrapped the details about how panic works. Maybe that's too much and we want to put a little bit of it back in, I dunno. I'm inclined to add that at the end, in its own "advanced" section.

steveklabnik · 2016-08-23T19:24:24Z

Oh, and this also means that Travis is gonna fail because nightly-only for now.

carols10cents · 2016-08-24T14:10:13Z

Nightly is failing too though, with some cyclic references: https://travis-ci.org/rust-lang/book/jobs/154542003#L366

carols10cents · 2016-08-24T14:11:34Z

src/ch07-02-unrecoverable-errors-with-panic.md

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


We actually haven't talked about vec! at this point, oops! Any reason this couldn't be an array?

this comes after the data structures chapter, right?

not the way we currently have them ordered, no reason we couldn't swap them though if you wanted

oh whoops! Yeah, I think we should swap them; this one should be the last of the three current chapters

I'm going to swap them in SUMMARY on master to remind us, then we can deal with the merge conflicts later :)

rebased to fix this

carols10cents · 2016-08-24T17:37:46Z

Overall, I love the new direction ❤️ I think it's just what the reader needs at this point in the book. 👏 👏 👏 👏 👏

aturon · 2016-08-24T17:56:06Z

src/ch07-02-unrecoverable-errors-with-panic.md

+```
+
+We attempt to access the hundredth element of our vector, but it only has three
+elements. In this situation, Rust will panic. Let's try it:


We should say why it's panicking here.

Ah, I see that you have that below. I would reorder it: start with the why and then delve into the mechanics.

steveklabnik · 2016-08-25T18:46:38Z

I just mentioned this to @aturon , but I believe the "write your own errors" chapter should be moved to the nanescent "advice for library authors" chapter, since we really need traits to talk about it.

carols10cents · 2016-08-25T19:27:34Z

I just mentioned this to @aturon , but I believe the "write your own errors" chapter should be moved to the nanescent "advice for library authors" chapter, since we really need traits to talk about it.

oh poo. Can't the reader just read all the chapters at once? It would be much more efficient that way ;)

steveklabnik · 2016-08-25T20:38:01Z

Okay, many of the nits should now be addressed, though not 100% of them yet.

carols10cents · 2016-10-17T20:41:57Z

Ok, I think I'm ready for another review, anyone have time for another look @steveklabnik @aturon @LukasKalbertodt @jonathandturner ? <3

steveklabnik · 2016-10-18T18:31:46Z

(I plan on reviewing later today)

On Mon, Oct 17, 2016 at 4:41 PM, Carol (Nichols || Goulding) <
notifications@github.com> wrote:

Ok, I think I'm ready for another review, anyone have time for another
look @steveklabnik https://github.com/steveklabnik @aturon
https://github.com/aturon @LukasKalbertodt
https://github.com/LukasKalbertodt @jonathandturner
https://github.com/jonathandturner ? <3

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#134 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AABsip2r9_4VGBJpPxTDwCsC9O1tziGHks5q092VgaJpZM4JM15O
.

steveklabnik

This is really, really great. I have some minor nits.

steveklabnik · 2016-10-18T20:43:57Z

.travis.yml

@@ -3,7 +3,10 @@ language: rust
 cache: cargo
 rust:
  - nightly
-  - stable
+# - beta
+# ^ to be changed once question_mark is in beta


rust-lang/rust#36995 means that this will be 1.14 🎊

steveklabnik · 2016-10-18T20:44:50Z

src/SUMMARY.md

+- [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)


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

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

steveklabnik · 2016-10-18T20:47:35Z

src/ch09-01-unrecoverable-errors-with-panic.md

+> up by the operating system. If you're in a situation where you need to make
+> the resulting binary as small as possible, you can switch from unwinding on
+> panic to aborting on panic by adding `panic = 'abort'` to the appropriate
+> `[profile]` sections in your `Cargo.toml` file.


"file" is redundant here.

steveklabnik · 2016-10-18T20:48:50Z

src/ch09-01-unrecoverable-errors-with-panic.md

+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


it's supposed to

steveklabnik · 2016-10-18T20:53:43Z

src/ch09-02-recoverable-errors-with-result.md

+They're very handy when prototyping, before you're ready to decide how to
+handle errors, and in that case they leave clear markers to look for when you
+are ready to make your program more robust. They're useful in tests since they
+will cause the test to fail if there's an error anyplace you call them. In


steveklabnik · 2016-10-18T20:55:04Z

src/ch09-02-recoverable-errors-with-result.md

+fn read_username_from_file() -> Result<String, io::Error> {
+    let mut f = try!(File::open("hello.txt"));
+    let mut s = String::new();
+    try!(f.read_to_string(&mut s));


i'd put whitespace both above and below this line

steveklabnik · 2016-10-18T20:55:36Z

src/ch09-02-recoverable-errors-with-result.md

+#
+fn read_username_from_file() -> Result<String, io::Error> {
+    let mut s = String::new();
+    File::open("hello.txt")?.read_to_string(&mut s)?;


same here, whitespace, above and below

steveklabnik · 2016-10-18T20:56:05Z

src/ch09-02-recoverable-errors-with-result.md

+<!-- NOTE: as of 2016-10-12, the error message when calling `?` in a function
+that doesn't return a result is confusing. `try!` isn't as bad, so I'm using
+that. When https://github.com/rust-lang/rust/issues/35946 is fixed, we can
+switch this example to use `?`. /Carol -->


I didn't know the error was any different between the two!

steveklabnik · 2016-10-18T20:57:44Z

src/ch09-03-to-panic-or-not-to-panic.md

+        if value < 1 || value > 100 {
+            panic!("Guess value must be between 1 and 100, got {}.", value);
+        }
+        Guess {


i'd put a newline above this

steveklabnik · 2016-10-18T20:58:12Z

src/ch09-03-to-panic-or-not-to-panic.md

+that would be a violation of the contract that `Guess::new` is relying on. This
+function needs to signal to the calling code that it has a bug somewhere
+leading to the contract violation. The conditions in which `Guess::new` might
+panic should be discussed in its public-facing API documentation.


do we want to mention the # Panics convention here?

(or maybe a forward reference to the documentation section of the building a library chapter)

Forward reference is good :)

carols10cents · 2016-10-18T22:38:42Z

Nits addressed! Thank you as always @steveklabnik :) Anyone else?

steveklabnik · 2016-10-19T00:41:33Z

ah yes, that'd make sense.

On Tue, Oct 18, 2016 at 6:26 PM, Carol (Nichols || Goulding) <
notifications@github.com> wrote:

@carols10cents commented on this pull request.

In src/ch09-01-unrecoverable-errors-with-panic.md
#134:

+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

@steveklabnik https://github.com/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 :-/

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#134, or mute the thread
https://github.com/notifications/unsubscribe-auth/AABsiv6gZcK5UFtfcXTjtRymbqSHItztks5q1UezgaJpZM4JM15O
.

carols10cents · 2016-10-31T15:52:16Z

Merging this and sending it to nostarch.

ch16-03 タイポ修正：ミューテック→ミューテックス

…ary-page rust-lang#134 add glossary page

sophiajt reviewed Jul 25, 2016
View reviewed changes

sophiajt reviewed Aug 17, 2016
View reviewed changes

steveklabnik force-pushed the error-handling branch from efa4088 to 146c07b Compare August 23, 2016 18:52

carols10cents reviewed Aug 24, 2016
View reviewed changes

aturon reviewed Aug 24, 2016
View reviewed changes

steveklabnik force-pushed the error-handling branch from a95c62d to efccc7a Compare August 25, 2016 20:35

carols10cents added 11 commits October 12, 2016 11:08

Add a discussion of matching on error kind

a3dfedb

Fix section headers

34b346c

Don't talk about main in the default recommendation

8f17a2a

Remove not-quite-accurate parts of this section

68bf232

Start of a section on good unwrap usage`

261056e

Remove a "this" that's not clear

9cb2218

Flesh out constructor contract example

b7a9b8a

Small tweaks to wording on another readthrough

c00340c

Discuss that it's ok to unwrap if you know it can't fail

6e7eb0e

Add listing numbers for long or referred-to code

211aec8

Add a summary

0fc14df

carols10cents approved these changes Oct 17, 2016

View reviewed changes

Talk a bit about the contract of Guess

2162339

steveklabnik commented Oct 18, 2016

View reviewed changes

carols10cents added 6 commits October 18, 2016 18:24

Title case titles

3a8c5ef

Remove redundant 'file'

fdc98f4

Spelling

0cd5b91

Whitespace in code

ee82377

Make notes since we now know 1.14 will have the question mark

234e758

Forward reference to api docs

3f72a06

carols10cents merged commit aabad34 into master Oct 31, 2016

carols10cents deleted the error-handling branch October 31, 2016 15:52

carols10cents mentioned this pull request Nov 8, 2016

Possible to have code block with gray background, no syntax highlighting, and not run with rustc? rust-lang/mdBook#179

Closed

AbrarNitk pushed a commit to FifthTry/rust-book that referenced this pull request Jun 2, 2021

Merge pull request rust-lang#134 from scivola/ch16-03-fix-typo

36184e6

ch16-03 タイポ修正：ミューテック→ミューテックス

AbrarNitk pushed a commit to FifthTry/rust-book that referenced this pull request Jun 2, 2021

Merge pull request rust-lang#137 from rinthel/rust-lang#134-add-gloss…

b207dcc

…ary-page rust-lang#134 add glossary page

start of error handling #134

start of error handling #134

Conversation

steveklabnik commented Jul 14, 2016

aturon commented Jul 22, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carols10cents commented Jul 27, 2016

Unwind vs abort

assert!

the example

try!

Choose a reason for hiding this comment

steveklabnik commented Aug 19, 2016

steveklabnik commented Aug 23, 2016 • edited Loading

steveklabnik commented Aug 23, 2016

carols10cents commented Aug 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carols10cents commented Aug 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

steveklabnik commented Aug 25, 2016

carols10cents commented Aug 25, 2016

steveklabnik commented Aug 25, 2016

carols10cents commented Oct 17, 2016

steveklabnik commented Oct 18, 2016

steveklabnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carols10cents commented Oct 18, 2016

steveklabnik commented Oct 19, 2016

@carols10cents commented on this pull request.

carols10cents commented Oct 31, 2016

steveklabnik commented Aug 23, 2016 •

edited

Loading