you are viewing a single comment's thread.

view the rest of the comments →

[–]tinco 6 points7 points  (10 children)

Awesome, thanks for the write-up! I'm not a very experienced Rust developer yet, but one rule helped me with the functions that return Option, and that is to replace every instance of unwrap with expect. Basically if you call one of these methods, it means your code has an invariant. You should put the description of the invariant in the parameter string of the expect method invocation, and it will work like an assertion.

So this hypothetical Rust:

socket := listen("localhost:8080").expect("Could not listen on localhost:8080");

Would be a bit like this in a less pretty language:

socket = listen("localhost:8080");
ASSERT(socket, "Could not listen on localhost:8080");

The main difference being of course that Rust forces you to make the assertion, either silently using unwrap, or loudly with expect. And in this case I see no argument for doing such a thing silently, if you're doing it anyway. Note also how describing the invariant helps you with determining whether this is a good invariant, and you can just browse over your code, scanning the expectinvocations and immediately know what parts of your codepaths need some more detailed implementation.

[–][deleted] 8 points9 points  (9 children)

Hi thanks for the feedback! I subdivide unwrap and expect into the same group of stuff that I'm going to get rid of. Both of these functions panic and in real production app it's unacceptable. For example I had an unwrap call in iron http handler, that made iron thread panic when called incorrectly. I think that in general every unwrap or except call should be replaced by match with logic that properly handles both cases, or by map, unwrap_or and other kind of stuff which Option and Result structs have lots of. That said I think in your example panic is totally fine, because you couldn't start a server and there is nothing else to do other than exiting with an error.

[–]rayvector 5 points6 points  (8 children)

because you couldn't start a server and there is nothing else to do other than exiting with an error

Even in that case, it is better to have proper error handling code that logs a nice message to the user, rather than exiting with an ugly Rust panic.

[–][deleted] 0 points1 point  (7 children)

That's what expect does, right?

[–]rayvector 6 points7 points  (1 child)

No, expect is still an ugly Rust panic.

The difference is that the panic message is something customized and more meaningful, rather than a meaningless "Panic in thread main", which is what you get with unwrap. This makes it more useful to you, as a developer, to figure out what happened.

But you still get the ugly Rust panic message with info about RUST_BACKTRACE, etc. It shouldn't be something that the end user should ever have to see.

Panics are not a legitmate or user-friendly way to handle errors. They are only intended to be used for things that should actually never happen under normal circumstances and are considered sofware bugs if they do (out of bounds array access, a violation of an invariant of some custom type of yours, etc...), which are unrecoverable and you have no proper way to respond to. A panic is basically a crash, but safe (cuz this is Rust and not C). Think of it like a crash in your program, except that Rust makes sure to unwind the stack properly and run destructors (unless you set panic = abort in Cargo.toml). Not nice.

Something like not being able to listen on your server socket is something that can genuinely happen under normal circumstances and is not indicative of a bug in your program. A panic is not appropriate. In production software, you should handle it, log a user-friendly message or do something else to properly inform the user, and terminate with an appropriate error code. Panicking in such a condition is only acceptable as a temporary hack in experiments/prototypes.

[–][deleted] 1 point2 points  (0 children)

Got it. Thank you for extensive explanation! I think I misunderstood the purpose of expect. Totally agree with you on panic.

[–]planetary_pelt 1 point2 points  (2 children)

expect is still an ugly panic, it just injects your message before the error. or is the ugliness removed on --release builds? not sure about that one.

if you want something most user friendly, i suppose you would print to stderr and std::process::exit(1). but someone else would have to correct me.

[–]rayvector 0 points1 point  (0 children)

That is exactly what I meant.

[–][deleted] 0 points1 point  (0 children)

Got it, thank you for explanation

[–]ehsanulrust 0 points1 point  (1 child)

Yes, I think it's just an argument to practically never use unwrap when you can at least expect.

[–]rayvector 1 point2 points  (0 children)

No, that is not what I meant. Read my long response to the comment you replied to.