7 ms·
Agreed. I left out any commentary on `.unwrap()` from my original comment, but it’s an obvious example of something that should never have appeared in critical
by frumplestlatz 11mo ago
Agreed.
I left out any commentary on `.unwrap()` from my original comment, but it’s an obvious example of something that should never have appeared in critical code.
- echelon 11mo agoRust needs to get rid of .unwrap() and its kin. They're from pre-1.0 Rust, before many of the type system features and error handling syntax sugar were added. There's no reason to use them as the language provides lots of safer alternatives. If you do want to trigger a panic, you can, but I'd also ask - why? Alternatively, and perhaps even better, Rust needs a way to mark functions that can panic for any reason other than malloc failures. Any function that then calls a panicky function needs to be similarly marked. In doing this, we can statically be certain no such methods are called if we want to be rid of the behavior. Perhaps something like: panic fn my_panicky_function() { None.unwrap(); // NB: `unwrap()` is also marked `panic` in stdlib } fn my_safe_function() { // with a certain compiler or Crates flag, this would fail to compile // as my_safe_function isn't annotated as `panic` my_panicky_function() } The ideal future would be to have code that is 100% panic free.
- jacquesm 11mo agoI'd say the equivalent of Erlang's supervisor trees is what is needed but once you go that route you might as well use Erlang.
- ViewTrick1002 11mo agoOr just deploy containers with an orchestrator restarting them when failing? It is not like an Erlang service would be able to make progress with an invalid config either.
- zbentley 11mo agoI’m not sure that panic (speaking generally about the majority of its uses and the spirit of the law; obviously 100% of code does not obey this) is the equivalent of an Erlang process crash in most cases. Rather, I think unwrap()/panic are usually used in ways more similar to erlang:halt/1.
- jacquesm 11mo agoExactly, but that is kind of the point here. An Erlang 'halt' is something that most Erlang programmers would twig is not what you want in most cases, in most cases you want your process to crash and for the supervisor to restart it if the error is recoverable. What happened here is systemic: the config file contained an issue severe enough that it precluded the system from running in the first place and unfortunately that caused a runtime error when in fact that validation should have been separate from the actual use. This is where I see the problem with this particular outage. And that makes it an engineering issue much more than a language issue. Bad configuration files can and do happen, so you take that eventuality into account during systems design.
- ViewTrick1002 11mo agoI’ve been seeing you blazing this trail since the incident and it feels short sighted and reductive. Rust is built on forcing the developer to acknowledge the complexity of reality. Unwrap acknowledges said complexity with a perfectly valid decision. There are a few warts from early days like indexing and the ”as” operator where the easy path is doing the wrong thing. But unwraps or expects are where Rust shines. Throwing up your hands is a perfectly reasonable response. With your approach, what should Cloudflare have done? Return an error, log it and return a 500 result due to invalid config? They could fail open, but then that opens another enormous can of worms. There simply are no good options. The issue rests upstream where deployments and effects between disparate services needs to be mapped and managed. Which is a truly hard problem, rather than blaming the final piece throwing up its hand when given an invalid config.
- echelon 11mo ago> I’ve been seeing you blazing this trail since the incident and it feels a short sighted and reductive. Why is it inappropriate to be able to statically label the behavior? Maybe I don't want my failure behavior dictated by a downstream dependency or distracted engineer. The subject of how to fail is a big topic and is completely orthogonal to the topic of how can we know about this and shape our outcomes. I would rather the policy be encoded with first class tools rather than engineering guidelines and runbooks. Let me have some additional control at what looks like to me not a great expense. It doesn't feel "safe" to me to assume the engineer meant to do exactly this and all of the upstream systems accounted for it. I would rather the code explicitly declare this in a policy we can enforce, in an AST we can shallowly reason about.
- ViewTrick1002 11mo agoHow deep do you go? Being forced to label any function that allocates memory with ”panic”? Right now you all the instances where the code can panic are labeled. Grep for unwrap, panic, expect etc. In all my years of professional Rust development I’ve never seen a potential panic pass code review without a discussion. Unless it was trivial like trying to build an invalid Regex from a static string.
- 12_throw_away 11mo ago> There's no reason to use [panics] as the language provides lots of safer alternatives. Dunno ... I think runtime assertions and the ability to crash a misbehaving program are a pretty important part of the toolset. If rust required `Result`s to be wired up up and down the entire call tree for the privilege of using a runtime assertion, I think it would be a lot less popular, and probably less safe in practice. > Alternatively, and perhaps even better, Rust needs a way to mark functions that can panic for any reason other than malloc failures. I 100% agree that a mechanism to prove that code can or cannot panic would be great, but why would malloc be special here? Folks who are serious about preventing panics will generally use `no-std` in order to prevent malloc in the first place.
- echelon 11mo ago> I 100% agree that a mechanism to prove that code can or cannot panic would be great, but why would malloc be special here? Folks who are serious about preventing panics will generally use `no-std` in order to prevent malloc in the first place. In one of the domains I work in, a malloc failure and OOMkill are equivalent. We just restart the container. I've done all the memory pressure measurement ahead of time and reasonably understand how the system will behave under load. Ideally it should never happen because we pay attention to this and provision with lots of overhead capacity, failover, etc. We have slow spillover rather than instantaneous catastrophe. Then there's instrumentation, metrics, and alerting. A surprise bug in my code or a dependency that causes an unexpected panic might cause my application or cluster to restart in ways we cannot predict or monitor. And it can happen across hundreds of application instances all at once. There won't be advanced notice, and we won't have a smoking gun. We might waste hours looking for it. It could be as simple as ingesting a pubsub message and calling unwrap(). Imagine an critical service layer doing this all at once, which in turn kills downstream services, thundering herds of flailing services, etc. - now your entire company is on fire, everyone is being paged, and folks are just trying to make sense of it. The fact is that the type of bugs that might trigger a user-induced panic might be hidden for a long time and then strike immediately with millions of dollars of consequences. Maybe the team you implemented an RPC for six months ago changes their message protocol by flipping a flag. Or maybe you start publishing keys with encoded data center affinity bytes, but the schema changed, and the library that is supposed to handle routing did an unwrap() against a topology it doesn't understand - oops! Maybe the new version handles it, but you have older versions deployed that won't handle it gracefully. These failures tend to sneak up on you, then happen all at once, across the entire service, leaving you with no redundancy. If you ingest a message that causes every instance to death spiral, you're screwed. Then you've got to hope your logging can help you find it quickly. And maybe it's not a simple roll back to resolve. And we know how long Rust takes to build... The best tool for this surely can't be just a lint? In a supposedly "safe" language? And with no way to screen dependencies? Just because somebody's use case for Rust is okay with this behavior doesn't mean everyone's tolerates this. Distributed systems folks would greatly appreciate some control over this. All I'm asking for is tools to help us minimize the surface area for panics. We need as much control over this as we can get.
- burntsushi 11mo agoI'm on libs-api. We will never get rid of unwrap(). It is absolutely okay to use unwrap(). It's just an assertion. Assertions appear in critical code all the time, including the standard library. Just like it's okay to use `slice[i]`.
- echelon 11mo agoThis is the Hundred Billion Dollar unwrap() Bug. You can keep unwrap() and panics. I just want a static first class method to ensure it never winds up in our code or in the dependencies we consume. I have personally been involved in nearly a billion dollars of outages myself and am telling you there are simple things the language can do to help users purge their code of this. This is a Rust foot gun. A simple annotation and compiler flag to disallow would suffice. It needs to handle both my code and my dependencies. We can build it ourselves as a hack, but it will never be 100% correct. This is why I want it: https://news.ycombinator.com/item?id=46060907 https://news.ycombinator.com/item?id=46060907
- burntsushi 11mo agoYou said: > Rust needs to get rid of .unwrap() and its kin. Now you say: > You can keep unwrap() and panics. So which is it? > I just want a static first class method to ensure it never winds up in our code or in the dependencies we consume. Now this is absolutely a reasonable request. But it's not an easy one to provide depending on how you go about it. For example, I'd expect your suggestion in your other comment to be a non-starter because of the impact it will have on language complexity. But that doesn't mean there isn't a better way. (I just don't know what it is.) This is a classic motte and bailey. You come out with a bombastic claim like "remove unwrap and its ilk," but when confronted, you retreat to the far more reasonable, "I just want tools to detect and prevent panicking branches." If you had said the latter, I wouldn't have even responded to you. I wouldn't have even batted an eye. > This is the Hundred Billion Dollar unwrap() Bug. The Cloudflare bug wasn't even caused by unwrap(). unwrap() is just its manifestation. From a Cloudflare employee: > In this case the unwrap() was only a symptom of an already bad state causing an error that the service couldn't recover from. This would have been as much of an unrecoverable error if it was reported in any other way. The mechanisms needed to either prevent it or recover are much more nuanced than just whether it's an unwrap or Result.
- svieira 11mo agoAll that means is that the `Failure` bubbles up to the very top of `main` (in this scenario) because we're only caring about the happy path (because we can't conceive of what the unhappy path should be other than "crash") and then hits the `panic("Well, that's unexpected")` explicitly in Place B rather than Place A (the `.unwrap`). I'm not sure how that's _better_.
- jacquesm 11mo agoIt would not because it would be a compile time error rather than run time error which is a completely different beast if I understand the argument correctly.
- Dylan16807 11mo agoWhat would be a compile time error? The compiler rejecting unwrap? And then you fix that by bubbling the error case up, which fixes the compiler error and leaves you with a runtime error again. But one that's less ergonomic. You can't force a config file loaded at run time to be correct at compile time. You can only decide what you're going to do about the failure.
- jacquesm 11mo agoThe point they are - trying, apparently - making is that if you had a flag or an annotation that you could make to a function that you do not want that function to be built on top of anything that can 'unwrap' that you can rule out some of these cases of unexpected side effects.
- echelon 11mo agoNot really. Handler and middleware can handle this without much ceremony. The user gets to, and is informed of and encouraged to, choose. We also don't get surprised at runtime. It's in the AST and we know at compile time. The right API signature helps the engineer think about things and puts them in the correct headspace for systems thinking. If something is panicking under the hood, the thought probably doesn't even occur to them.
- bigstrat2003 11mo agoUnwrap is not only fine, it's a valuable part of the language. Getting rid of it would be a horrible change. What needs to happen is not using an assert (which is really what unwrap is) if an application can't afford to crash.
- antonvs 11mo agoAnd it's so easy to avoid, as well. #![deny(clippy::unwrap_used)] or cargo clippy -- -D clippy::unwrap_used Put that in your CI pipeline, and voila. Global crash averted.
- lenkite 11mo agoON HN, just a couple of years ago, a famous Rust programmer said that it is OK to use unwrap. Rustaceans supported this position. Cloudflare merely followed the community standard. Using unwrap() in Rust is Okay https://news.ycombinator.com/item?id=32385102 https://news.ycombinator.com/item?id=32385102 https://burntsushi.net/unwrap/ https://burntsushi.net/unwrap/
- deleted 11mo ago[deleted]
- burntsushi 11mo agoI already had a conversation with the GP specifically: https://news.ycombinator.com/item?id=45979127 https://news.ycombinator.com/item?id=45979127 They aren't presenting a coherent philosophy. And when asked for examples, or to engage directly with examples in my blog, they can't or won't do it. But yes, of course it's okay to use unwrap(). It's just an assertion. Assertions are fine.
- frumplestlatz 11mo agoResult declares a type-level invariant — an assertion enforced by the compiler, not runtime — that the operation can fail. Ignoring that is bypassing the type system. It means your types are either wrong, or your type system is incapable of modeling your true invariants. In the case of the cloudflare error, their types were wrong. That was an avoidable failure. They needed to fix their type-level invariants, not yolo the issue with `.unwrap()`. Your willful persistent lack of understanding doesn’t mean my philosophy is incoherent. Using `.unwrap()` is always an example of a failure to accurately model your invariants in the type system.
- burntsushi 11mo agoYour definition of "correct" is completely incoherent. Just because an invariant that could be modeled by a type system is not modeled by the type system in any given scenario does not make it incorrect. You can't engage with my examples and you provide none of your own. So continuing discussion with you is a waste of time.
- deleted 11mo ago[deleted]