zlacker

I think it's because unwrap() seems to unassuming at a glance. If it were or_panic() instead I think people would intuit it more as extremely dangerous. I understand that we're not dealing with newbies here, but everyone is still human and everything we do to reduce mistakes is a good thing.

>>Deflet+(OP)
.or_panic() would be a genuinely better name for .unwrap(). .unwrap() sounds a lot like .unbox(), whatcoulddagowrong?

>>Deflet+(OP)
I'm sure lots of bystanders are surprised to learn what .unwrap() does. But reading the post, I didn't get the impression that anyone at cloudflare was confused by unwrap's behaviour.

If you read the postmortem, they talk in depth about what the issue really was - which from memory is that their software statically allocated room for 20 rules or something. And their database query unexpected returned more than 20 items. Oops!

I can see the argument for renaming unwrap to unwrap_or_panic. But no alternate spelling of .unwrap() would have saved cloudflare from their buggy database code.

>>Deflet+(OP)
It's not unassuming. Rust is superior to many other languages for making this risky behaviour visually present in the code base.

You can go ahead and grep your codebase for this today, instead of waiting for an incident.

I'm a fairly new migrant from Java to C#, and when I do some kind of collection lookup, I still need to check whether the method will return a null, throw an exception, expect an out+variable, or worst of all, make up some kind of default. C#'s equivalent to unwrap seems to be '!' (or maybe .Val() or something?)

>>mrkeen+Rf
Whether the value is null (and under which conditions) is encoded into the nullability of return value. Unless you work with a project which went out of its way to disable NRTs (which I've sadly seen happen).

>>joseph+g3
Looking at that unwrap as a Result<T> handler, the arguable issue with the code was the lack of informative explanation in the unexpected case. Panicking from the ill-defined state was desired behaviour, but explicit is always better.

The argument to the contrary is that reading the error out-load showed “the config initializer failing to return a valid configuration”. A panic trace saying “config init failed” is a minor improvement.

If we’re gonna guess and point fingers, I think the configuration init should be doing its own panicking and logging when it blows up.

>>Deflet+(OP)
I don't think you can know what unwrap does and assume it is safe. Plus warnings about unwrap are very common in the Rust community, I even remember articles saying to never use it.

I have always been critical of the Rust hype but unwrap is completely fine. Is an escape hatch has legitimate uses. Some code is fine to just fail.

It is easy to spot during code review. I have never programmed Rust professional and even I would have asked about the unwrap in the cloudfare code if I had reviewed that. You can even enforce to not use unwrap at all through automatic tooling.

>>Deflet+(OP)
> I think it's because unwrap() seems to unassuming at a glance. If it were or_panic() instead I think people would intuit it more as extremely dangerous.

Anyone who has learned how to program Rust knows that unwrap() will panic if the thing you are unwrapping is Err/None. It's not unassuming at all. When the only person who could be tripped up by a method name is a complete newbie to the language, I don't think it's actually a problem.

Similarly, assert() isn't immediately obvious to a beginner that it will cause a panic. Heck, the term "panic" itself is non obvious to a beginner as something that will crash the program. Yet I don't hear anyone arguing that the panic! macro needs to be changed to crash_this_program. The fact of the matter is that a certain amount of jargon is inevitable in programming (and in my view this is a good thing, because it enables more concise communication amongst practitioners). Unwrap is no different than those other bits of jargon - perhaps non obvious when you are new, but completely obvious once you have learned it.

>>boness+9B
First, again, that’s not the issue. The bug was in their database code. Could this codebase be improved with error messages? Yes for sure. But that wouldn’t have prevented the outage.

Almost every codebase I’ve ever worked in, in every programming language, could use better human readable error messages. But they’re notoriously hard to figure out ahead of time. You can only write good error messages for error cases you’ve thought through. And most error cases only become apparent when you stub your toe on them for real. Then you wonder how you missed it in the first place.

In any case, this sort of thing has nothing to do with rust.