This is the multi-million dollar .unwrap() story. In a critical path of infrastructure serving a significant chunk of the internet, calling .unwrap() on a Result means you're saying "this can never fail, and if it does, crash the thread immediately."The Rust compiler forced them to acknowledge this could fail (that's what Result is for), but they explicitly chose to panic instead of handle it gracefully. This is textbook "parse, don't validate" anti-pattern.
I know, this is "Monday morning quarterbacking", but that's what you get for an outage this big that had me tied up for half a day.
I’ve led multiple incident responses at a FAANG, here’s my take. The fundamental problem here is not Rust or the coding error. The problem is:
1. Their bot management system is designed to push a configuration out to their entire network rapidly. This is necessary so they can rapidly respond to attacks, but it creates risk as compared to systems that roll out changes gradually.
2. Despite the elevated risk of system wide rapid config propagation, it took them 2 hours to identify the config as the proximate cause, and another hour to roll it back.
SOP for stuff breaking is you roll back to a known good state. If you roll out gradually and your canaries break, you have a clear signal to roll back. Here was a special case where they needed their system to rapidly propagate changes everywhere, which is a huge risk, but didn’t quite have the visibility and rapid rollback capability in place to match that risk.
While it’s certainly useful to examine the root cause in the code, you’re never going to have defect free code. Reliability isn’t just about avoiding bugs. It’s about understanding how to give yourself clear visibility into the relationship between changes and behavior and the rollback capability to quickly revert to a known good state.
Cloudflare has done an amazing job with availability for many years and their Rust code now powers 20% of internet traffic. Truly a great team.
How can you write the proxy without handling the config containing more than the maximum features limit you set yourself?
How can the database export query not have a limit set if there is a hard limit on number of features?
Why do they do non-critical changes in production before testing in a stage environment?
Why did they think this was a cyberattack and only after two hours realize it was the config file?
Why are they that afraid of a botnet? Does not leave me confident that they will handle the next Aisuru attack.
I'm migrating my customers off Cloudflare. I don't think they can swallow the next botnet attacks and everyone on Cloudflare go down with the ship, so it will be safer to not be behind Cloudflare when it hits.
Exactly. The only way this could happen in the first place was _because_ they failed at so many levels. And as a result, more layers of Swiss cheese will be added, and holes in existing ones will be patched. This process is the reason flying is so safe, and the reason why Cloudflare will be a little bit more resilient tomorrow than it was yesterday.
> Why do they do non-critical changes in production before testing in a stage environment?
I guess the noncritical change here was the change to the database? My experience has been a lot of teams do a poor job having a faithful replica of databases in stage environments to expose this type of issue.
In part because it is somewhere between really hard and impossible. Is your staging DB going to be as big? Seeing the same RPS as prod? Seeing the same scenarios?
Permissions stuff might be caught without a completely faithful replica, but there are always going to be attributes of the system that only exist in prod.
I know its easy to criticize what happened after the fact and having a clear(er) picture of all the moving parts and the timeline of events, but I think that while most of the people in the thread are pointing out either Rust-related or lack of configuration validation, what really grinds my gears is something that - in my opinion - is bad engineering.
Having an unprivileged application querying system.columns to infer the table layout is just bad; Not having a proper, well-defined table structure indicates sloppiness in the overall schema design, specially if it changes quickly. Considering specifically clickhouse, and even if this approach would be a good idea, the unprivileged way of doing it would be "DESCRIBE TABLE <name>", NOT iterating system.columns. The gist of it - sloppy design not even well implemented.
Having a critical application issuing ad-hoc commands to system.* tablespace instead of using a well-tested library is just amateurism, and again - bad engineering; IMO it is good practice to consider all system.* privileged applications and ensure their querying is completely separate from your application logic; Sometimes some system tables change, and fields are added and/or removed - not planning for this will basically make future compatibility a nightmare.
Not only the problematic query itself, but the whole context of this screams "lack of proper application design" and devs not knowing how to use the product and/or read the documentation. Granted, this is a bit "close to home" for me, because I use ClickHouse extensively (at a scale - I'm assuming - several orders of magnitude smaller than CloudFlare) and I have spent a lot of time designing specifically to avoid at least some of these kind of mistakes. But, if I can do it at my scale, why aren't they doing it?
On all the other issues, I thought they wanted to do the right thing at heart, but missed to make it fail safe. I can pass it as a problem of a journey to maturity or simply the fact that you can't get everything perfect. Maybe even a bit of sloppiness here and there.
The database issue screamed at me: lack of expertise. I don't use CH, but seeing someone to mess with a production system and they being surprised "Oh, it does that?", is really bad. And this is obviously not knowledge that is hard to achieve, buried deep in a manual or an edge case only discoverable by source code, it's bread and butter knowledge you should know.
What is confusing, that they didn't add this to their follow-up steps. With some benefit of doubt I'd assume they didn't want to put something very basic as a reason out there, just to protect the people behind it from widespread blame. But if that's not the case, then it's a general problem. Sadly it's not uncommon that components like databases are dealt with, on an low effort basis. Just a thing we plug in and works. But it's obviously not.
I don't think these are realistic requirements for any engineered system to be honest. Realistic is to have contingencies for such cases, which are simply errors.
But the case for Cloudflare here is complicated. Every engineer is very free to make a better system though.
What is not realistic? To do simple input validation on data that has the potential to break 20% of the internet? To not have a system in place to rollback to the latest known state when things crash?
Cloudflare builds a global scale system, not an iphone app. Please act like it.
Cloudflares success was simplicity to build a distributed system in different data centers around the world to be implemented by third party IT workers while Cloudflare were a few people. There are probably a lot of shitty iPhone apps that do less important work and are vastly more complex than the former Cloudflare server node configuration.
Every system has a non-reducible risk and no data rollback is trivial, especially for a CDN.
Yeah, I don't quite understand the people cutting Cloudflare massive slack. It's not about nailing blame on a single person or a team, it's about keeping a company that is THE closest thing to a public utility for the web accountable. They more or less did a Press Release with a call to action to buy or use their services at the end and everybody is going "Yep, that's totally fine. Who hasn't sent a bug to prod, amirite?".
It goes over my head why Cloudflare is HN's darling while others like Google, Microsoft and AWS don't usually enjoy the same treatment.
>It goes over my head why Cloudflare is HN's darling while others like Google, Microsoft and AWS don't usually enjoy the same treatment.
Do the others you mentioned provide such detailed outage reports, within 24 hours of an incident? I’ve never seen others share the actual code that related to the incident.
Or the CEO or CTO replying to comments here?
>Press Release
This is not press release, they always did these outage posts from the start of the company.
> Do the others you mentioned provide such detailed outage reports, within 24 hours of an incident? I’ve never seen others share the actual code that related to the incident.
The code sample might as well be COBOL for people not familiar with Rust and its error handling semantics.
> Or the CEO or CTO replying to comments here?
I've looked around the thread and I haven't seen the CTO here nor the CEO, probably I'm not familiar with their usernames and that's on me.
> This is not press release, they always did these outage posts from the start of the company.
My mistake calling them press releases. Newspapers and online publications also skim this outage report to inform their news stories.
I wasn't clear enough on my previous comment. I'd like all major players in the internet and web infrastructure to be held to higher standards. As it stands when it comes to them or the tech department of a retail store the retail store must answer to more laws when surface area of combined activities is took into account.
Yes, Cloudflare excels where others don't or barely bother and I too enjoyed the pretty graphs, diagrams and I've learned some nifty Rust tricks.
EDIT: I've removed some unwarranted snark from my comment which I apologize for.
> To do simple input validation on data that has the potential to break 20% of the internet?
There will always be bugs in code, even simple code, and sometimes those things don't get caught before they cause significant trouble.
The failing here was not having a quick rollback option, or having it and not hitting the button soon enough (even if they thought the problem was probably something else, I think my paranoia about my own code quality is such that I would have been rolling back much sooner just in case I was wrong about the “something else”).
Name me global, redundant systems that have not (yet) failed.
And if you used cloudflare to protect against botnet and now go off cloudflare... you are vulnerable and may experience more downtime if you cannot swallow the traffic.
I mean no service have 100% uptime - just that some have more nines than others.
As yourself more the question, is your service that important to need 99.999% uptime? Because i get the impression that people are so fixated on this uptime concept, that the idea of being down for a few hours is the most horrible issue in the world. To the point that they rather hand over control of their own system to a 3th party, then accept a downtime.
The fact that cloudflare can literally ready every bit of communication (as it sits between the client and your server) is already plenty bad. And yet, we accept this more easily, then a bit of downtime. We shall not ask about the prices for that service ;)
To me its nothing more then the whole "everybody on the cloud" issue, when most do not need the resource that cloud companies like AWS provide (and the bill), and yet, get totally tied down to this one service.
Not when you start pushing into the TB's range of monthly data... When you get that dreaded phone call from a CF rep, because the bill that is coming is no joke.
Its free as long as you really are small, not worth milking. The moment you can afford to run your own mini dc at your office, you start to enter the "well, hello there" for CF.
> The moment you can afford to run your own mini dc at your office, you start to enter the "well, hello there" for CF.
As someone who has (and is) runs (running) a DC with all the electrical/UPS, cooling, piping, HVAC+D stuff to deal with: it can be a lot of just time/overhead.
Especially if you don't have a number of folks in-house to deal with all that 'non-IT' equipment (I'm a bit strange in that I have an interest in both IT and HVAC-y stuff).
> There are many self-hosted alternatives to protect against botnet.
What would some good examples of those be? I think something like Anubis is mostly against bot scraping, not sure how you'd mitigate a DDoS attack well with self-hosted infra if you don't have a lot of resources?
On that note, what would be a good self-hosted WAF? I recall using mod_security with Apache and the OWASP ruleset, apparently the Nginx version worked a bit slower (e.g. https://www.litespeedtech.com/benchmarks/modsecurity-apache-... ), there was also the Coraza project but I haven't heard much about it https://coraza.io/ or maybe the people who say that running a WAF isn't strictly necessary also have a point (depending on the particular attack surface).
There is haproxy-protection, which I believe is the basis of Kiwiflare. Clients making new connections have to solve a proof-of-work challenge that take about 3 seconds of compute time.
Well if you self host DDoS protection service, that would be VERY expensive. You would need rent rack space along with a very fast internet connection at multiple data centers to host this service.
If you're buying transit, you'll have a hard time getting away with less than 10% commit, i.e. you'll have to pay for 10 Gbps of transit to have a 100 Gbps port, which will typically run into 4 digits USD / month. You'll need a few hundred Gbps of network and scrubbing capacity to handle common DDoS attacks using amplification from script kids with a 10 Gbps uplink server that allow spoofing, and probably on the order of 50+ Tbps to handle Aisuru.
If you're just renting servers instead, you have a few options that are effectively closer to a 1% commit, but better have a plan B for when your upstreams drop you if the incoming attack traffic starts disrupting other customers - see Neoprotect having to shut down their service last month.
We had better uptime with AWS WAF in us-east-1 than we've had in the last 1.5 years of Cloudflare.
I do like the flat cost of Cloudflare and feature set better but they have quite a few outages compared to other large vendors--especially with Access (their zero trust product)
I'd lump them into GitHub levels of reliability
We had a comparable but slightly higher quote from an Akamai VAR.
But at the same time, what value do they add if they:
* Took down the the customers sites due to their bug.
* Never protected against an attack that our infra could not have handled by itself.
* Don't think that they will be able to handle the "next big ddos" attack.
It's just an extra layer of complexity for us. I'm sure there are attacks that could help our customers with, that's why we're using them in the first place. But until the customers are hit with multiple ddos attacks that we can not handle ourself then it's just not worth it.
> • Took down the the customers sites due to their bug.
That is always a risk with using a 3rd party service, or even adding extra locally managed moving parts. We use them in DayJob, and despite this huge issue and the number of much smaller ones we've experienced over the last few years their reliability has been pretty darn good (at least as good as the Azure infrastructure we have their services sat in front of).
> • Never protected against an attack that our infra could not have handled by itself.
But what about the next one… Obviously this is a question sensitive to many factors in our risk profiles and attitudes to that risk, there is no one right answer to the “but is it worth it?” question here.
On a slightly facetious point: if something malicious does happen to your infrastructure, that it does not cope well with, you won't have the “everyone else is down too” shield :) [only slightly facetious because while some of our clients are asking for a full report including justification for continued use of CF and any other 3rd parties, which is their right both morally and as written in our contracts, most, especially those who had locally managed services affected, have taken the “yeah, half our other stuff was affected to, what can you do?” viewpoint].
> • Don't think that they will be able to handle the "next big ddos" attack.
It is a war of attrition. At some point a new technique, or just a new botnet significantly larger than those seen before, will come along that they might not be able to deflect quickly. I'd be concerned if they were conceited enough not to be concerned about that possibility. Any new player is likely to practise on smaller targets first before directly attacking CF (in fact I assume that it is rather rare that CF is attacked directly) or a large enough segment of their clients to cause them specific issues. Could your infrastructure do any better if you happen to be chosen as one of those earlier targets?
Again, I don't know your risk profile so can say which is the right answer, if there even is an easy one other than “not thinking about it at all” being a truly wrong answer. Also DDoS protection is not the only service many use CF for, so those need to be considered too if you aren't using them for that one thing.
I agree. I think the comments about how "it is fine, because so many things had to fail" do not apply in this case.
It's not that many things had to fail, it's that many things that are obvious haven't been done. It would be a valid excuse if many "exotic" scenarios would have to align, not when it's obvious error cases that weren't handled and changes have not been tested.
While having wrong first assumptions is just how things work when you try to analyze the issue[1], not testing changes before production is just stupidity and nothing else.
The story would be different if eg. multiple unlikely, hard to track things happened at once without anyone making a clearly linkable event, something that would also happen in staging. Most of the things mentioned could essentially statically checked. This is the prime example of what you want as any tech person, because it's not hard to prevent compared to a lot of scenarios where you deal with balancing likelihoods of scenarios, timings, etc.
You don't think someone is a great plumber, because they forgot their tools and missed that big hole in the pipe and also rang at the wrong door, because all these things failed. You think someone is a good plumber if they said they would have to go back to fetch a bulky specialized tool, because this is the rare case in which they need it, but they could also do this other thing in this spcific case. They are great plumbers if they tell you how this happened in first place and how to fix it. They are great plumbers if they manage to fix something outside of their usual scope.
Here pretty much all of the things that you pay them for failed. At a large scale.
I am sure this has there are reasons which we don't now about, and I hope that CloudFlare can fix them. Be it management focusing on the wrong things, be it developers not being in the wrong position or annoyed enough to care or something else entirely. However, not doing these things is (likely) a sign that currently they are not in the state of creating reliable systems - at least none reliable enough for what they are doing. It would be perfectly fine if they ran a web shop or something, but if as experienced many other companies rely on you being up or their stuff fails, then maybe you should not run a company with products like "Always Online".
[1] And should make you adapt the process of analyzing issues. Eg. making sure config changes are "very loud" in monitoring. It's one of the most easily tracked thing that can go wrong, and can relatively easily be mapped to a point in time compared to many other things.
Does their ring based rollout really truly have to be 0->100% in a few seconds?
I don’t really buy this requirement. At least make it configurable with a more reasonable default for “routine” changes. E.g. ramping to 100% over 1 hour.
As long as that ramp rate is configurable, you can retain the ability to respond fast to attacks by setting the ramp time to a few seconds if you truly think it’s needed in that moment.
The configuration file is updated every five minutes, so clearly they have some past experience where they’ve decided an hour is too long. That said, even a roll out over five minutes can be helpful.
This was not about DDoS defense but the Bot Management feature, which is a paid Enterprise-only feature not enabled by default to block automated requests regardless of whether an attack is going on.
Bots can also cause a DoS/DDoS. We use the feature to restrict certain AI scraper tools by user agent that adversly impact performance (they have a tendency to hammer "export all the data" endpoints much more than regular users do)
It would still fail if you were unluckily on the new proxy (it's not very clear why if the feature was not enabled, indeed):
> Unrelated to this incident, we were and are currently migrating our customer traffic to a new version of our proxy service, internally known as FL2. Both versions were affected by the issue, although the impact observed was different.
> Customers deployed on the new FL2 proxy engine, observed HTTP 5xx errors. Customers on our old proxy engine, known as FL, did not see errors, but bot scores were not generated correctly, resulting in all traffic receiving a bot score of zero. Customers that had rules deployed to block bots would have seen large numbers of false positives. Customers who were not using our bot score in their rules did not see any impact.
Maybe, but in that case maybe have some special casing logic to detect that yes indeed we're under a massive DDOS at this very moment, do a rapid rollout of this thing that will mitigate said DDOS. Otherwise use the default slower one?
Of course, this is all so easy to say after the fact..
> Their bot management system is designed to push a configuration out to their entire network rapidly.
Once every 5m is not "rapidly". It isn't uncommon for configuration systems to do it every few seconds [0].
> While it’s certainly useful to examine the root cause in the code.
Believe the issue is as much an output from a periodic run (clickhouse query) caused by (on the surface, an unrelated change) causing this failure. That is, the system that validated the configuration (FL2) was different to the one that generated it (ML Bot Management DB).
Ideally, it is the system that vends a complex configuration that also vends & tests the library to consume it, or the system that consumes it, does so as if it was "tasting" the configuration first before devouring it unconditionally [1].
Of course, as with all distributed system failures, this is all easier said and done in hindsight.
Isn't rapidly more of how long it takes to get from A to Z rather than how often it is performed? You can push out a configuration update every fortnight but if it goes through all of your global servers in three seconds, I'd call it quite rapid.
I don't understand why they didn't validate and sanitize the new config file revision.
If bad(whatever that reason is) throw an error and revert back to previous version. You don't need to take down the whole internet for that.
Same as for almost every bug I think: the dev in question hadn't considered that the input could be bad in the way that it turned out to be. Maybe they were new, or maybe they hadn't slept much because of a newborn baby, or maybe they thought it was a reasonable assumption that there would never be more than 200 ML features in the array in question. I don't think this developer will ever make the same mistake again at least.
Let those who have never written a bug before cast the first stone.
> Maybe they were new, or maybe they hadn't slept much because of a newborn baby
Reminds me of House of Dynamite, the movie about nuclear apocalypse that really revolves around these very human factors. This outage is a perfect example of why relying on anything humans have built is risky, which includes the entire nuclear apparatus. “I don’t understand why X wasn’t built in such a way that wouldn’t mean we live in an underground bunker now” is the sentence that comes to mind.
I don't think this is an error originating from a single human. At CF scale I'd expect that multiple humans saw that code and gave it a pass.
Rust or not, but an experienced dev could have seen this can lead to issues. Panicking without restoring a healthy state is just not an option in this case. They *know* that.
I guess you are right, likely a social issue, but certainly not a single exhausted parent.
> I don't understand why they didn't validate and sanitize the new config file revision.
The new config file was not (AIUI) invalid (syntax-wise) but rather too big:
> […] That feature file, in turn, doubled in size. The larger-than-expected feature file was then propagated to all the machines that make up our network.
> The software running on these machines to route traffic across our network reads this feature file to keep our Bot Management system up to date with ever changing threats. The software had a limit on the size of the feature file that was below its doubled size. That caused the software to fail.
Exactly the right take. Even when you want to have rapid changes on your infra, do it at least by region. You can start with the region where the least amount of users are impacted and if everything is fine, there is no elevated number of crashes for example, you can move forward. It was a standard practice at $RANDOM_FAANG when we had such deployments.
Thank you. I am sympathetic to CF’s need to deploy these configs globally fast and don’t think slowing down their DDoS mitigation is necessarily a good trade off. What I am saying is this presents a bigger reliability risk and needs correspondingly fine crafted observability around such config changes and a rollback runbook. Greater risk -> greater attention.
But the rapid deployment mechanism for bot features wasn’t where the bug was introduced.
In fact, the root bug (faulty assumption?) was in one or more SQL catalog queries that were presumably written some time ago.
(Interestingly the analysis doesn’t go into how these erroneous queries made it into production OR whether the assumption was “to spec” and it’s the security principal change work that was faulty. Seems more likely to be the former.)
It was a change to the database that is used to generate a bot management config file. That file was the proximate cause for the panics. The kind of observability that would have helped here is “panics are elevated and here are the binary and config changes that preceded it,” along with a rollback runbook for it all.
Generally I would say we as an industry are more nonchalant about config changes vs binary changes. Where an org might have great processes and systems in place for binary rollouts, the whole fleet could be reading config from a database in a much more lax fashion. Those systems are quite risky actually.
I am genuinely curious (albeit skeptical!) how anyone like Cloudflare could make that kind of feedback loop work at scale.
Even only in CF’s “critical path” there must be dozens of interconnected services and systems. How do you close the loop between an observed panic at the edge and a database configuration change N systems upstream?
I've also led a team of Incident Commanders at a FAANG.
If this was a routine config change, I could see how it could take 2 hours to start the mediation plan. However they should have dashboards that correlate config setting changes with 500 errors (or equivalent). It gets difficult when you have many of of these going out at the same time and they are slowly rolled out.
The root cause document is mostly for high level and the public. The details on this specific outage will be in a internal document with many action items, some of them maybe quarter long projects including fixing this specific bug and maybe some linter/monitor to prevent it from happening again.
In a productive way, this view also shifts the focus to improving the system (visibility etc), empowering the team, rather than focusing on the code which broke (probably strikes fear in the individuals, to do anything!)
The "coding error" is a somewhat deliberate choice to fail eagerly that is usually safe but doesn't align with the need to do something (propagation of the configuration file) without failing.
I'm sure that there are misapplied guidelines to do that instead of being nice to incoming bot management configuration files, and someone might have been scolded (or worse) for proposing or attempting to handle them more safely.
I would say that whilst this is a good top down view, that `.unwrap()` should have been caught at code-review and not allowed. Clippy rule could have saved a lot of money.
That and why the hell wasn't their alerting showing up colossal amount of panics in their bot manager thing?
Yes the lack of observability is really the disturbing bit here. You have panics in a bunch of your core infrastructure, you would expect there to be a big red banner on the dashboard that people look at when they first start troubleshooting an incident.
This is also a pretty good example why having stack traces by default is great. That error could have been immediately understood just from a stack trace and a basic exception message.
You can write the safest code in the world, but if you're shipping config changes globally every few minutes without a robust rollback plan or telemetry that pinpoints when things go sideways, you're flying blind
Partial disagree. There should be lints against 'unwrap's. An 'expect' at least forces you to write down why you are so certain it can't fail. An unwrap is not just hubris, it's also laziness, and has no place in sensitive code.
And yes, there is a lint you can use against slicing ('indexing_slicing') and it's absolutely wild that it's not on by default in clippy.
I use unwrap a lot, and my most frequent target is unwrapping the result of Mutex::lock. Most applications have no reasonable way to recover from lock poisoning, so if I were forced to write a match for each such use site to handle the error case, the handler would have no choice but to just call panic anyway. Which is equivalent to unwrap, but much more verbose.
Perhaps it needs a scarier name, like "assume_ok".
I use locks a lot too, and I always return a Result from lock access. Sometimes an anyhow::Result, but still something to pass up to the caller.
This lets me do logging at minimum. Sometimes I can gracefully degrade. I try to be elegant in failure as possible, but not to the point where I wouldn't be able to detect errors or would enter a bad state.
That said, I am totally fine with your use case in your application. You're probably making sane choices for your problem. It should be on each organization to decide what the appropriate level of granularity is for each solution.
My worry is that this runtime panic behavior has unwittingly seeped into library code that is beyond our ability and scope to observe. Or that an organization sets a policy, but that the tools don't allow for rigid enforcement.
Pretty much - the time spent ruling out the hypothesis that it was a cyberattack would have been time spent investigating the uptick in deliberately written error logs, since you would expect alerts to be triggered if those exceed a threshold.
I imagine it would also require less time debugging a panic. That kind of breadcrumb trail in your logs is a gift to the future engineer and also customers who see a shorter period of downtime.
I actually have to do this for programs that runs in bare metal. You can't afford to have nondeterministic panic like this. If things really gone wrong you'd have a watchdog and health checker to verify the state of program.
There's a crate that prevents linking panic symbol in the final stage of the executable generation, forcing it to be undefined symbol, so while it is hard to find out where the panic is, it effectively requires me to inspect throughout the code to find out. Sometimes I have to disassemble the object file to see this
It is just 2 different layers. Of course the code is also a problem, if it is in fact as the GP describes it. You are taking the higher level view, which is the second layer of dealing with not only this specific mistake, but also other mistakes, that can be related to arbitrary code paths.
Both are important, and I am pretty sure, that someone is gonna fix that line of code pretty soon.
The bot is efficient. This is by design. It will push out mistakes just as efficiently as it pushes out good changes. Good or bad... the plane of control is unchanged.
This is the danger of automated control systems. If they get hacked or somehow push out bad things (CloudStrike), they will have complete control and be very efficient.
It seems people have a blind spot for unwrap, perhaps because it's so often used in example code. In production code an unwrap or expect should be reviewed exactly like a panic.
It's not necessarily invalid to use unwrap in production code if you would just call panic anyway. But just like every unsafe block needs a SAFETY comment, every unwrap in production code needs an INFALLIBILITY comment. clippy::unwrap_used can enforce this.
Yes? Funnily enough, I don't often use indexed access in Rust. Either I'm looping over elements of a data structure (in which case I use iterators), or I'm using an untrusted index value (in which case I explicitly handle the error case). In the rare case where I'm using an index value that I can guarantee is never invalid (e.g. graph traversal where the indices are never exposed outside the scope of the traversal), then I create a safe wrapper around the unsafe access and document the invariant.
If that's the case then hats off. What you're describing is definitely not what I've seen in practice. In fact, I don't think I've ever seen a crate or production codebase that documents infallibility of every single slice access. Even security-critical cryptography crates that passed audits don't do that. Personally, I found it quite hard to avoid indexing for graph-heavy code, so I'm always on the lookout for interesting ways to enforce access safety. If you have some code to share that would be very interesting.
My rule of thumb is that unchecked access is okay in scenarios where both the array/map and the indices/keys are private implementation details of a function or struct, since an invariant is easy to manually verify when it is tightly scoped as such. I've seen it used it in:
* Graph/tree traversal functions that take a visitor function as a parameter
> I don't think I've ever seen a crate or production codebase that documents infallibility of every single slice access.
The smoltcp crate typically uses runtime checks to ensure slice accesses made by the library do not cause a panic. It's not exactly equivalent to GP's assertion, since it doesn't cover "every single slice access", but it at least covers slice accesses triggered by the library's public API. (i.e. none of the public API functions should cause a panic, assuming that the runtime validation after the most recent mutation succeeds).
I think this goes against the Rust goals in terms of performance. Good for safe code, of course, but usually Rust users like to have compile time safety to making runtime safety checks unnecessary.
Sure, these days I'm mostly working on a few compilers. Let's say I want to make a fixed-size SSA IR. Each instruction has an opcode and two operands (which are essentially pointers to other instructions). The IR is populated in one phase, and then lowered in the next. During lowering I run a few peephole and code motion optimizations on the IR, and then do regalloc + asm codegen. During that pass the IR is mutated and indices are invalidated/updated. The important thing is that this phase is extremely performance-critical.
One normal "trick" is phantom typing. You create a type representing indices and have a small, well-audited portion of unsafe code handling creation/unpacking, where the rest of the code is completely safe.
The details depend a lot on what you're doing and how you're doing it. Does the graph grow? Shrink? Do you have more than one? Do you care about programmer error types other than panic/UB?
Suppose, e.g., that your graph doesn't change sizes, you only have one, and you only care about panics/UB. Then you can get away with:
1. A dedicated index type, unique to that graph (shadow / strong-typedef / wrap / whatever), corresponding to whichever index type you're natively using to index nodes.
2. Some mechanism for generating such indices. E.g., during graph population phase you have a method which returns the next custom index or None if none exist. You generated the IR with those custom indexes, so you know (assuming that one critical function is correct) that they're able to appropriately index anywhere in your graph.
3. You have some unsafe code somewhere which blindly trusts those indices when you start actually indexing into your array(s) of node information. However, since the very existence of such an index is proof that you're allowed to access the data, that access is safe.
Techniques vary from language to language and depending on your exact goals. GhostCell [0] in Rust is one way of relegating literally all of the unsafe code to a well-vetted library, and it uses tagged types (via lifetimes), so you can also do away with the "only one graph" limitation. It's been awhile since I've looked at it, but resizes might also be safe pretty trivially (or might not be).
The general principle though is to structure your problem in such a way that a very small amount of code (so that you can more easily prove it correct) can provide promises that are enforceable purely via the type system (so that if the critical code is correct then so is everything else).
That's trivial by itself (e.g., just rely on option-returning .get operators), so the rest of the trick is to find a cheap place in your code which can provide stronger guarantees. For many problems, initialization is the perfect place (e.g., you can bounds-check on init and then not worry about it again) (e.g., if even bounds-checking on initialization is too slow then you can still use the opportunity at initialization to write out a proof of why some invariant holds and then blindly/unsafely assert it to be true, but you then immediately pack that hard-won information into a dedicated type so that the only place you ever have to think about it is on initialization).
I do use a combination of newtyped indices + singleton arenas for data structures that only grow (like the AST). But for the IR, being able to remove nodes from the graph is very important. So phantom typing wouldn't work in that case.
Usually you'd want to write almost all your slice or other container iterations with iterators, in a functional style.
For the 5% of cases that are too complex for standard iterators? I never bother justifying why my indexes are correct, but I don't see why not.
You very rarely need SAFETY comments in Rust because almost all the code you write is safe in the first place. The language also gives you the tool to avoid manual iteration (not just for safety, but because it lets the compiler eliminate bounds checks), so it would actually be quite viable to write these comments, since you only need them when you're doing something unusual.
I didn't restate the context from the code we're discussing: it must not panic. If you don't care if the code panics, then go ahead and unwrap/expect/index, because that conforms to your chosen error handling scheme. This is fine for lots of things like CLI tools or isolated subprocesses, and makes review a lot easier.
So: first, identify code that cannot be allowed to panic. Within that code, yes, in the rare case that you use [i], you need to at least try to justify why you think it'll be in bounds. But it would be better not to.
There are a couple of attempts at getting the compiler to prove that code can't panic (e.g., the no-panic crate).
What about memory allocation - how will you stop that from panicking ? `Vec::resize` will always panic in Rust. And this is just one example out of thousands in the Rust stdlib.
Unless the language addresses no-panic in its governing design or allows try-catch, not sure how you go about this.
That is slowly being addressed, but meanwhile it’s likely you have a reliable upper bound on how much heap your service needs, so it’s a much smaller worry. There are also techniques like up-front or static allocation if you want to make more certain.
This is ridiculous. We're probably going to start seeing more of these. This was just the first, big highly visible instance.
We should have a name for this similar to "my code just NPE'd". I suggest "unwrapped", as in, "My Rust app just unwrapped a present."
I think we should start advocating for the deprecation and eventual removal of the unwrap/expect family of methods. There's no reason engineers shouldn't be handling Options and Results gracefully, either passing the state to the caller or turning to a success or fail path. Not doing this is just laziness.
Indexing is comparatively rare given the existence of iterators, IMO. If your goal is to avoid any potential for panicking, I think you'd have a harder time with arithmetic overflow.
Your pair of posts is very interesting to me. Can you share with me: What is your programming environment such that you are "fine with allocation failures"? I'm not doubting you, but for me, if I am doing systems programming with C or C++, my program is doomed if a malloc fails! When I saw your post, I immediately thought: Am I doing it wrong? If I get a NULL back from malloc(), I just terminate with an error message.
I mean, yeah, if I am using a library, as an user of this library, I would like to be able to handle the error myself. Having the library decide to panic, for example, is the opposite of it.
If I can't allocate memory, I'm typically okay with the program terminating.
I don't want dependencies deciding to unwrap() or expect() some bullshit and that causing my entire program to crash because I didn't anticipate or handle the panic.
Code should be written, to the largest extent possible, to mitigate errors using Result<>. This is just laziness.
I want checks in the language to safeguard against lazy Rust developers. I don't want their code in my dependency tree, and I want static guarantees against this.
edit: I just searched unwrap() usage on Github, and I'm now kind of worried/angry:
Something that allows me to tag annotate a function (or my whole crate) as "no panic", and get a compile error if the function or anything it calls has a reachable panic.
This will allow it to work with many unmodified crates, as long as constant propagation can prove that any panics are unreachable. This approach will also allow crates to provide panicking and non panicking versions of their API (which many already do).
Yes, I want that. I also want to be able to (1) statically apply a badge on every crate that makes and meets these guarantees (including transitively with that crate's own dependencies) so I can search crates.io for stronger guarantees and (2) annotate my Cargo.toml to not import crates that violate this, so time isn't wasted compiling - we know it'll fail in advance.
On the subject of this, I want more ability to filter out crates in our Cargo.toml. Such as a max dependency depth. Or a frozen set of dependencies that is guaranteed not to change so audits are easier. (Obviously we could vendor the code in and be in charge of our own destiny, but this feels like something we can let crate authors police.)
I think the most common solution at the moment is dtolnay's no_panic [0]. That has a bunch of caveats, though, and the ergonomics leave something to be desired, so a first-party solution would probably be preferable.
I would be fine just getting rid of unwrap(), expect(), etc. That's still a net win.
Look at how many lazy cases of this there are in Rust code [1].
Some of these are no doubt tested (albeit impossible to statically guarantee), but a lot of it looks like sloppiness or not leaning on the language's strong error handling features.
It's disappointing to see. We've had so much of this creep into the language that eventually it caused a major stop-the-world outage. This is unlikely to be the last time we see it.
I don't write Rust so I don't really know, but from someone else's description here it sounds similar to `fromJust` in Haskell which is a common newbie footgun. I think you're right that this is a case of not using the language properly, though I know I was seduced into the idea that Haskell is safe by default when I was first learning, which isn't quite true — the safety features are opt-in.
A language DX feature I quite like is when dangerous things are labelled as such. IIRC, some examples of this are `accursedUnutterablePerformIO` in Haskell, and `DO_NOT_USE_OR_YOU_WILL_BE_FIRED_EXPERIMENTAL_CREATE_ROOT_CONTAINERS` in React.js.
I would be in favor of renaming unwrap() and its family to `unwrap_do_not_use_or_you_will_break_the_internet()`
I still think we should remove them outright or make production code fail to compile without a flag allowing them. And we also need tools to start cleaning up our dependency tree of this mess.
For iteration, yes. But there's other cases, like any time you have to deal with lots of linked data structures. If you need high performance, chances are that you'll have to use an index+arena strategy. They're also common in mathematical codebases.
Yes, I always thought it was wrong to use unwrap in examples. I know, people want to keep examples simple, but it trains developers to use unwrap() as they see that everywhere.
Yes, there are places where it's ok as that blog post explains so well: https://burntsushi.net/unwrap/
But most devs IMHO don't have the time to make the call correctly most of the time... so it's just better to do something better, like handle the error and try to recover, or if impossible, at least do `expect("damn it, how did this happen")`.
There is a prevailing mentality that LLMs make it easy to become productive in new languages, if you are already proficient in one. That's perhaps true until you suddenly bump up against the need to go beyond your superficial understanding of the new language and its idiosyncrasies. These little collisions with reality occur until one of them sparks an issue of this magnitude.
In theory, experienced human code reviewers can course correct newer LLM-guided devs work before it blows up. In practice, reviewers are already stretched thin and submitters absolute to now rapidly generate more and more code to review makes that exhaustion effect way worse. It becomes less likely they spot something small but obvious amongst the haystack of LLM generated code bailing there way.
> There is a prevailing mentality that LLMs make it easy to become productive in new languages, if you are already proficient in one.
Yes, and: I've found this to be mostly true, if you make sure you take the time to deeply understand what the code is doing. When I asked an LLM to do something for me in Javascript, then I said, "What if X happens, wouldn't that cause Y? Would it be better to restructure it like so and so to make it more robust?" The LLM immediately improves it.
Any experienced programmer who was taking the time to review this code, on learning that unwrap() has a "panic" inside, would certainly change it. But as you say, reviewers are already stretched thin.
Dunno, I think the alternatives have their own pretty significant downsides. All would require front loading more in-depth understanding of error handling and some would just be quite a bit more verbose.
IMO making unwrap a clippy lint (or perhaps a warning) would be a decent start. Or maybe renaming unwrap.
This strikes me as a culture issue more than one of language.
A tenet of systems code is that every possible error must be handled explicitly and exhaustively close to the point of occurrence. It doesn’t matter if it is Rust, C, etc. Knowing how to write systems code is unrelated to knowing a systems language. Rust is a systems language but most people coming into Rust have no systems code experience and are “holding it wrong”. It has been a recurring theme I’ve seen with Rust development in a systems context.
C is pretty broken as a language but one of the things going for it is that it has a strong systems code culture surrounding it that remembers e.g. why we do all of this extra error handling work. Rust really needs systems code practice to be more strongly visible in the culture around the language.
Unwrap _is_ explicitly handling an error at the point of occurrence. You have explicitly decided to panic, which is sometimes a valid choice. I use it (on startup only) when server configs are missing or invalid or in CLI tools when the options aren't valid. Crashing a pod on startup before it goes Ready is a valid pattern in k8s and generally won't cause an outage because the previous pod will continue working.
> at least do `expect("damn it, how did this happen")`
That gives you the same behavior as unwrap with a less useful error message though. In theory you can write useful messages, but in practice (and your example) expect is rarely better than unwrap in modern rust
I disagree with that characterization. Using unwrap() like you suggest in your blog post is an intentional, well-thought-out choice. Using unwrap() the way Cloudflare did it is, with hindsight, a bad choice, that doesn't utilize the language's design features.
Note that they're not criticizing the language. I read "Rust developers" in this context as developers using Rust, not those who develop the language and ecosystem. (In particular they were not criticizing you.)
I think it's reasonable to question the use of unwrap() in this context. Taking a cue from your blog post^ under runtime invariant violations, I don't think this use matches any of your cases. They assumed the size of a config file is small, it wasn't, so the internet crashed.
Echelon's comment was "We shouldn't be using unwrap() or expect() at all. [...] unwrap(), expect(), bad math, etc. - this is all caused by lazy Rust developers". Even in my most generous interpretation I can't see how that is anything except a rejection of all unwraps (and equivalent constructs like expect()).
I fully agree with burntsushi that echelon is taking an extreme and arguably wrong stance. His sentiment becomes more and more correct as Rust continues to evolve ways to avoid unwrap as an ergonomic shortcut, but I don't think we are quite there yet for general use. There absolutely is code that should never panic, but that involves tradeoffs and design choices that aren't true for every project (or even the majority of them)
> We shouldn't be using unwrap() or expect() at all.
So the context of their comment is not some specific nuanced example. They made a blanket statement.
> Note that they're not criticizing the language. I read "Rust developers" in this context as developers using Rust, not those who develop the language and ecosystem.
I have the same interpretation.
> I think it's reasonable to question the use of unwrap() in this context. Taking a cue from your blog post^ under runtime invariant violations, I don't think this use matches any of your cases. They assumed the size of a config file is small, it wasn't, so the internet crashed.
Yes? I didn't say it wasn't reasonable to question the use of unwrap() here. I don't think we really have enough information to know whether it was inappropriate or not.
unwrap() is all about nuance. I hope my blog post conveyed that. Because unwrap() is a manifestation of an assertion on a runtime invariant. A runtime invariant can be arbitrarily complicated. So saying things like, "we shouldn't be using unwrap() or expect() at all" is an extreme position to carve out that is also way too generalized.
I stand by what I said. They are factually mistaken in their characterization of the use of unwrap()/expect() in general.
> So the context of their comment is not some specific nuanced example. They made a blanket statement.
That is their opinion, I disagree with it, but I don't think it's an insulting or invalid opinion to have. There are codebases that ban nulls in other languages too.
> They are factually mistaken in their characterization of the use of unwrap()/expect() in general.
It's an opinion about a stylistic choice. I don't see what fact there is here that could be mistaken.
I'm finding this exchange frustrating, and now we're going in circles. I'll say this one last time in as clear language as I can. They said this:
> unwrap(), expect(), bad math, etc. - this is all caused by lazy Rust developers or Rust developers not utilizing the language's design features.
The factually incorrect part of this is the statement that use of `unwrap()`, `expect()` and so on is caused by X or Y, where X is "lazy Rust developers" and Y is "Rust developers not utilizing the language's design features." But there are, factually, other causes than X or Y for use of `unwrap()`, `expect()` and so on. So stating that it is all caused by X or Y is factually incorrect. Moreover, X is 100% insulting when applied to any one specific individual. Y can be insulting when applied to any one specific individual.
Now this:
> We shouldn't be using unwrap() or expect() at all.
That's an opinion. It isn't factually incorrect. And it isn't insulting.
I'm sorry I'm frustrating you. It was not my intention. For what it's worth, I use ripgrep every day, and it's made my life appreciably better. (Same goes for Astral products.) Thank you for that, and I wish your day improves.
> unwrap(), expect(), bad math, etc. - this is all caused by lazy Rust developers or Rust developers not utilizing the language's design features
I just read that line as shorthand for large outages caused by misuse of unwrap(), expect(), bad math etc. - all caused by...
That's also an opinion, by my reading.
I assumed we were talking specifically about misuses, not all uses of unwrap(), or all bad bugs. Anyway, I think we're ultimately saying the same thing. It's ironic in its own way.
I have to disagree that unwrap is ever OK. If you have to use unwrap, your types do not match your problem. Fix them. You have encoded invariants in your types that do not match reality.
Change your API boundary, surface the discrepancy between your requirements and the potential failing case at the edges where it can be handled.
If you need the value, you need to handle the case that it’s not available explicitly. You need to define your error path(s)
This is a failure caused by lazy Rust programming and not relying on the language's design features.
It's a shame this code can even be written. It is surprising and escapes the expected safety of the language.
I'm terrified of some dependency using unwrap() or expect() and crashing for something entirely outside of my control.
We should have an opt-in strict Cargo.toml declaration that forbids compilation of any crate that uses entirely preventable panics. The only panics I'll accept are those relating to memory allocation.
This is one of the sharpest edges in the language, and it needs to be smoothed away.
`slice[i]` is also a hole in the type system, but at least it’s generally relying on a local invariant, immediate to the surrounding context, that does not require lying about invariants across your API surface.
The blog post doesn’t address the issue, it simply pretends it’s not a real problem.
Also from the post: “If we were to steelman advocates in favor of this style of coding, then I think the argument is probably best limited to certain high reliability domains. I personally don’t have a ton of experience in said domains …”
`slice[i]` is just sugar for `slice.get(i).unwrap()`. And whether it's a "local" invariant or not is orthogonal. And `unwrap()` does not "require lying about invariants across your API surface."
> The blog post doesn’t address the issue, it simply pretends it’s not a real problem.
It very explicitly addresses it! It even gives real examples.
> Also from the post: “If we were to steelman advocates in favor of this style of coding, then I think the argument is probably best limited to certain high reliability domains. I personally don’t have a ton of experience in said domains …”
>
> Enough said.
Ad hominem... I don't have experience working on, e.g., medical devices upon which someone's life depends. So the point of that sentence is to say, "yes, I acknowledge this advice may not apply there." You also cherry picked that quote and left off the context, which is relevant here.
And note that you said:
> I have to disagree that unwrap is ever OK.
That's an extreme position. It isn't caveated to only apply to certain contexts.
> `slice[i]` is just sugar for `slice.get(i).unwrap()`. And whether it's a "local" invariant or not is orthogonal. And `unwrap()` does not "require lying about invariants across your API surface."
It's not orthogonal. `Result` isn't a local invariant, and yes, `.unwrap()` does require lying. If your code depends on an API that can fail, and you cannot handle that failure locally (`.unwrap()` is not handling it), then your type signature needs to express that you can fail -- and you need to raise an error on that failure.
> That's an extreme position. It isn't caveated to only apply to certain contexts.
No, it's a principled position. Correct code doesn't `.unwrap()`, but code that hides failure cases -- or foists invariant enforcement onto programmers remembering not to screw up -- does.
I've built and worked on ridiculously complex code bases without a single instance of `.unwrap()` or the local language equivalent; it's just not necessary. This is just liked the unchecked exception debate in Java -- complex explanations for a very simple goal of avoiding the thought, time, and effort to accurately model a system's invariants.
> No, it's a principled position. Correct code doesn't `.unwrap()`, but code that hides failure cases -- or foists invariant enforcement onto programmers remembering not to screw up -- does.
I don't think you understand what an internal runtime invariant is. Either way, I don't know of any widespread libraries (in any language) that follow this "principled" position. That makes it de facto extreme.
> I've built and worked on ridiculously complex code bases without a single instance of `.unwrap()` or the local language equivalent; it's just not necessary.
Show me. If you're using `slice[i]`, then you're using `unwrap()`. It introduces a panicking branch.
> If your code depends on an API that can fail, and you cannot handle that failure locally (`.unwrap()` is not handling it), then your type signature needs to express that you can fail -- and you need to raise an error on that failure.
You use `unwrap()` when you know the failure cannot happen.
I note you haven't engaged with any of the examples I provided in the blog.
> You use `unwrap()` when you know the failure cannot happen.
That’s an invariant meant to be expressed by your type system — and it is.
You’ve failed to model your invariants in your API — and thus the type system — if you ever reach a point where an engineer has to manually assess and assert whether “cannot” applies.
> If you have to use unwrap, your types do not match your problem
The problem starts with Rust stdlib. It panics on allocation failure. You expect Rust programmers to look at stdlib and not imitate it?
Sure, you can try to taboo unwrap(), but 1) it won't work, and 2) it'll contort program design in places where failure really is a logic bug, not a runtime failure, and for which unwrap() is actually appropriate.
The real solution is to go back in time, bonk the Rust designers over the head with a cluebat, and have them ship a language that makes error propagation the default and syntactically marks infallible cleanup paths --- like C++ with noexcept.
Of course it will. I've built enormous systems, including an entire compiler, without once relying on the local language equivalent of `.unwrap()`.
> 2) it'll contort program design in places where failure really is a logic bug, not a runtime failure, and for which unwrap() is actually appropriate.
That's a failure to model invariants in your API correctly.
> ... have them ship a language that makes error propagation the default and syntactically marks infallible cleanup paths --- like C++ with noexcept.
Unchecked exceptions aren't a solution. They're a way to avoid taking the thought, time, and effort to model failure paths, and instead leave that inherent unaddressed complexity until a runtime failure surprises users. Like just happened to Cloudflare.
It's the same blind spot people have to Java's checked exceptions. People commonly resort to Pokemon exception handling and either blindly ignoring or rethrowing as a runtime exception. When Rust got popular, I was a bit confused by people talking about how great Result it's essentially a checked exception without a stack trace.
"Checked Exceptions Are Actually Good" gang, rise up! :p
I think adoption would have played out very different if there had only been some more syntactic-sugar. For example, an easy syntax for saying: "In this method, any (checked) DeepException e that bubbles up should immediately be replaced by a new (checked) MylayerException(e) that contains the original one as a cause.
We might still get lazy programmers making systems where every damn thing goes into a generic MylayerException, but that mess would still be way easier to fix later than a hundred scattered RuntimeExceptions.
Exception handling would be better than what we're seeing here.
The problem is that any non-trivial software is composition, and encapsulation means most errors aren't recoverable.
We just need easy ways to propagate exceptions out to the appropriate reliability boundary, ie. the transaction/ request/ config loading, and fail it sensibly, with an easily diagnosable message and without crashing the whole process.
C# or unchecked Java exceptions are actually fairly close to ideal for this.
The correct paradigm is "prefer throw to catch" -- requiring devs to check every ret-val just created thousands of opportunities for mistakes to be made.
By contrast, a reliable C# or Java version might have just 3 catch clauses and handle errors arising below sensibly without any developer effort.
I'm with you! Checked exceptions are actually good and the hate for them is super short sighted. The exact same criticisms levied at checked exceptions apply to static typing in general, but people acknowledge the great value static types have for preventing errors at compile time. Checked exceptions have that same value, but are dunked on for some reason.
1. in most cases they don't want to handle `InterruptedException` or `IOException` and yet need to bubble them up. In that case the code is very verbose.
2. it makes lambdas and functions incompatible. So eg: if you're passing a function to forEach, you're forced to wrap it in runtime exception.
3. Due to (1) and (2), most people become lazy and do `throws Exception` which negates most advantages of having exceptions in the first place.
In line-of-business apps (where Java is used the most), an uncaught exception is not a big deal. It will bubble up and gets handled somewhere far up the stack (eg: the server logger) without disrupting other parts of the application. This reduces the utility of having every function throw InterruptedException / IOException when those hardly ever happen.
Java checked exceptions suffer from a lack of generic exception types ("throws T", where T can be e.g. "Exception", "Exception1|Exception2", or "never") This would also require union types and a bottom type.
Without generics, higher order functions are very hard to use.
In my experience, it actually is a big deal, leaving a wake of indeterminant state behind after stack unrolling. The app then fails with heisenbugs later, raising more exceptions that get ignored, compounding the problem.
People just shrug off that unreliability as an unavoidable cost of doing business.
Yeah, in both cases it's a layering situation, where it's the duty of your code to decide what layers of abstraction need to be be bridged, and to execute on that decision. Translating/wrapping exception-types from deeper functions is the same as translating/wrapping return-types the same places.
I think it comes down to a psychological or use-case issue: People hate thinking about errors and handling them, because it's that hard stuff that always consumes more time than we'd like to think. Not just digitally, but in physical machines too. It's also easier to put off "for later."
Checked exceptions in theory were good, but Java simply did not add facilities to handle or support them well in many APIs. Even the new API's in Java - Streams, etc do not support checked exceptions.
There is also the problem that they decided to make all references nullable, so `NullPointerException`s could appear everywhere. This "forced" them to introduce the escape hatch of `RuntimeException`, which of course was way overused immediately, normalizing it.
It's a lot lighter: a stack trace takes a lot of overhead to generate; a result has no overhead for a failure. The overhead (panic) only comes once the failure can't be handled. (Most books on Java/C# don't explain that throwing exceptions has high performance overhead.)
Exceptions force a panic on all errors, which is why they're supposed to be used in "exceptional" situations. To avoid exceptions when an error is expected, (eof, broken socket, file not found,) you either have to use an unnatural return type or accept the performance penalty of the panic that happens when you "throw."
In Rust, the stack trace happens at panic (unwrap), which is when the error isn't handled. IE, it's not when the file isn't found, it's when the error isn't handled.
Exceptions do not force panic at all. In most practical situations, an exception unhandled close to where it was thrown will eventually get logged. It's kind of a "local" panic, if you will, that will terminate the specific function, but the rest of the program will remain unaffected. For example, a web server might throw an exception while processing a specific HTTP request, but other HTTP requests are unaffected.
Throwing an exception does not necessarily mean that your program is suddenly in an unsupported state, and therefore does not require terminating the entire program.
> Throwing an exception does not necessarily mean that your program is suddenly in an unsupported state, and therefore does not require terminating the entire program.
That's not what a panic means. Take a read through Go's panic / resume mechanism; it's similar to exceptions, but the semantics (with multiple return values) make it clear that panic is for exceptional situations. (IE, panic isn't for "file not found," but instead it's for when code isn't written to handle "file not found.")
Sure, but the same is true of any error handling strategy.
When you work with exceptions, the key is to assume that every line can throw unless proven otherwise, which in practice means almost all lines of code can throw. Once you adopt that mental model, things get easier.
Explicit error handling strategies allow you to not worry about all the code paths that explicitly cannot throw -- which is a lot of them. It makes life a lot easier in the non-throwing case, and doesn't complicate life any more in the throwing case as compared to exception-based error handling.
It also makes errors part of the API contract, which is where they belong, because they are.
It can and that optimization has existed for a while.
Actually it can also just turn off the collection of stack traces entirely for throw sites that are being hit all the time. But most Java code doesn't need this because code only throws exceptions for exceptional situations.
> it's essentially a checked exception without a stack trace
In theory, theory and practice are the same. In practice...
You can't throw a checked exception in a stream, this fact actually underlines the key difference between an exception and a Result: Result is in return position and exceptions are a sort of side effect that has its own control flow. Because of that, once your method throws an Exception or you are writing code in a try block that catches an exception, you become blind to further exceptions of that type, even if you might be able to or required to fix those errors. Results are required to be handled individually and you get syntactic sugar to easily back propagate.
It is trivial to include a stack trace, but stack traces are really only useful for identifying where something occurred, and generally what is superior is attaching context as you back propagate which trivially occurs with judicious use of custom error types with From impls. Doing this means that the error message uniquely defines the origin and paths it passed through without intermediate unimportant stack noise. With exceptions you would always need to catch each exception and rethrow a new exception containing the old to add contextual information, then to avoid catching to much you need variables that will be initialized inside the try block defined outside of the try block. So stack traces are basically only useful when you are doing Pokemon exception handling.
> When Rust got popular, I was a bit confused by people talking about how great Result it's essentially a checked exception without a stack trace.
It's not a checked exception without a stack trace.
Rust doesn't have Java's checked or unchecked exception semantics at the moment. Panics are more like Java's Errors (e.g. OOM error). Results are just error codes on steroids.
checked exceptions failed because when used properly they fossilize method signatures. they're fine if your code will never be changed and they're fine when you control 100% of users of the throwing code. if you're distributing a library... no bueno.
That’s just not true. They required that you use hierarchical exception types and define your own library exception type that you declare at the boundary.
The same is required for any principled error handling.
That's kind of what I'm saying with the blind spot comment. The words "unwrap" and "expect" should be just as much a scary red flag as the word "panic", but for some reason it seems a lot of people don't see them that way.
Even in lowly Java, they later added to Optional the orElseThrow() method since the name of the get() method did not connote the impact of unwrapping an empty Optional.
I've found both methods very useful. I'm using `get()` when I've checked that the value is present and I don't expect any exceptions. I'm using `orElseThrow()` when I actually expect that value can be absent and throwing is fine. Something like
if (userOpt.isPresent()) {
var user = userOpt.get();
var accountOpt = accountRepository.selectAccountOpt(user.getId());
var account = accountOpt.orElseThrow();
}
Idea checks it by default and highlights if I've used `get()` without previous check. It's not forced at compiler level, but it's good enough for me.
The `unsafe` keyword means something specific in Rust, and panicking isn't unsafe by Rust's definition. Sometimes avoiding partial functions just isn't feasible, and an unwrap (or whatever you want to call the method) is a way of providing a (runtime-checked) proof to the compiler that the function is actually total.
unwrap() should effectively work as a Result<> where the user must manually invoke a panic in the failure branch. Make special syntax if a match and panic is too much boilerplate.
This is like an implicit null pointer exception that cannot be statically guarded against.
I want a way to statically block any crates doing this from my dependency chain.
That would require an effects system[0] like Koka's[1]. Then one could not only express the absence of panics but also allocations, infinite loops and various other undesirable effects within some call-trees.
This is a desirable feature, but an enormous undertaking.
Same thing that would happen if it did a match statement and panicked. The problem is the panic, not the unwrap.
I don’t think you can ever completely eliminate panics, because there are always going to be some assumptions in code that will be surprisingly violated, because bugs exist. What if the heap allocator discovers the heap is corrupted? What if you reference memory that’s paged out and the disk is offline? (That one’s probably not turned into a panic, but it’s the same principle.)
Not sure what you're saying with the "work as a Result<>" part...unwrap is a method on Result. I think you're just saying the unwrap/expect methods should be eliminated?
Than they are going to write None | Err => yolo() that has the same impact. It is not the syntax or the semantic meaning is the problem here but the fact that there is no monitoring around the elevated error counts after a deployment.
Software engineers tend to get stuck in software problems and thinking that everything should be fixed in code. In reality there are many things outside of the code that you can do to operate unreliable components safely.
Exactly. People are very hung up on "unwrap" but even if it wasn't there at all, you will have devs just manually writing the match. Or, even more likely, using a trivial 'unwrap!" macro.
There's also an assumption here that if the unwrap wasn't there, the caller would have handled the error properly. But if this isn't part of some common library at CF, then chances are the caller is the same person who wrote the panicking function in the first place. So if a new error variant they introduced was returned they'd probably still abort the thread either by panicking at that point or breaking out of the thread's processing loop.
Really not! This is a huge faceplant for writing things in Rust. If they had been writing their code in Java/Kotlin instead of Rust, this outage either wouldn't have happened at all (a failure to load a new config would have been caught by a defensive exception handler), or would have been resolved in minutes instead of hours.
The most useful thing exceptions give you is not static compile time checking, it's the stack trace, error message, causal chain and ability to catch errors at the right level of abstraction. Rust's panics give you none of that.
Look at the error message Cloudflare's engineers were faced with:
thread fl2_worker_thread panicked: called Result::unwrap() on an Err value
That's useless, barely better than "segmentation fault". No wonder it took so long to track down what was happening.
A proxy stack written in a managed language with exceptions would have given an error message like this:
com.cloudflare.proxy.botfeatures.TooManyFeaturesException: 200 > 60
at com.cloudflare.proxy.botfeatures.FeatureLoader(FeatureLoader.java:123)
at ...
and so on. It'd have been immediately apparent what went wrong. The bad configs could have been rolled back in minutes instead of hours.
In the past I've been able to diagnose production problems based on stack traces so many times I was been expecting an outage like this ever since the trend away from providing exceptions in new languages in the 2010s. A decade ago I wrote a defense of the feature and I hope we can now have a proper discussion about adding exceptions back to languages that need them (primarily Go and Rust):
That has nothing to do with exceptions, just the ability to unwind the stack. Rust can certainly give you a backtrace on panics; you don’t even have to write a handler to get it. I would find it hard to believe Cloudflare’s services aren’t configured to do it. I suspect they just didn’t put the entire message in the post.
tldr: Capturing a backtrace can be a quite expensive runtime operation, so the environment variables allow either forcibly disabling this runtime performance hit or allow selectively enabling it in some programs.
It's one of the problems with using result types. You don't distinguish between genuinely exceptional events and things that are expected to happen often on hot paths, so the runtime doesn't know how much data to collect.
panic is the exceptional event. It so happens that rust doesn't print a stacktrace in release unless configured to do so.
Similarly, capturing a stack trace in a error type (within a Result for example) is perfectly possible. But this is a choice left to the programmer, because capturing a trace is not cheap.
There's clearly a big gap in how things are done in practice. You wouldn't see anyone call System.exit in a managed language if a data file was bigger than expected. You'd always get an exception.
I used to be an SRE at Google. Back then we also had big outages caused by bad data files pushed to prod. It's a common enough issue so I really sympathize with Cloudflare, it's not nice to be on call for issues like that. But Google's prod environments always generated stack traces for every kind of failure, including CHECK failures (panics) in C++. You could also reflect the stack traces of every thread via HTTP. I used to diagnose bugs in production under time pressure quite regularly using just these tools. You always need detailed diagnostics.
Languages shouldn't have panics, tbh, it's a primitive concept. It so rarely makes sense to handle errors that way. I know there's a whole body of Rust/Go lore claiming panics are fine, but it's not a good move and is one of the reasons I've stayed away from Go over the years and wouldn't use Rust for anything higher than low level embedded components or operating system code that has to export a C ABI. You always want diagnostics and recoverable errors; this kind of micro-optimization doesn't make sense outside of extremely constrained embedded environments that very few of us work in.
An uncaught exception in C++ or an uncaught panic in Rust terminates the program. The unwinding is the same mechanism. I think the implementation is what comes with LLVM, but I haven't checked.
I was also a Google SRE, and I liked the stacktrace facilities so much that I got permission to open source a library inspired from it: https://github.com/bombela/backward-cpp (I know I am not doing a great job maintaining it)
At Uber I implemented a similar stackrace introspection for RPC tasks via HTTP for Go services.
You can also catch a Go panic. Which we did in our RPC library at Uber.
It would be great for all of that to somehow come ready made though. A sort of flag "this program is a service, turn on all the good diagnostics, here is my main loop".
Alternatively you can look at actually innovative programming languages to peek at the next 20 years of innovation.
I am not sure that watching the trendy forefront successfully reach the 1990s and discuss how unwrapping Option is potentially dangerous really warm my heart. I can’t wait for the complete meltdown when they discover effect systems in 2040.
To be more serious, this kind of incident is yet another reminder that software development remains miles away from proper engineering and even key providers like Cloudfare utterly fail at proper risk management.
Celebrating because there is now one popular language using static analysis for memory safety feels to me like being happy we now teach people to swim before a transatlantic boat crossing while we refuse to actually install life boats.
To me the situation has barely changed. The industry has been refusing to put in place strong reliability practices for decades, keeps significantly under investing in tools mitigating errors outside of a few fields where safety was already taken seriously before software was a thing and keeps hiding behind the excuse that we need to move fast and safety is too complex and costly while regulation remains extremely lenient.
I mean this Cloudfare outage probably cost millions of dollars of damage in aggregate between lost revenue and lost productivity. How much of that will they actually have to pay?
Let's try to make effect systems happen quicker than that.
> I mean this Cloudfare outage probably cost millions of dollars of damage in aggregate between lost revenue and lost productivity. How much of that will they actually have to pay?
Probably nothing, because most paying customers of cloudflare are probably signing away their rights to sue Cloudflare for damages by being down for a while when they purchase Cloudflare's services (maybe some customers have SLAs with monetary values attached, I dunno). I honestly have a hard time suggesting that those customers are individually wrong to do so - Cloudflare isn't down that often, and whatever amount it cost any individual customer by being down today might be more than offset by the DDOS protection they're buying.
Anyway if you want Cloudflare regulated to prevent this, name the specific regulations you want to see. Should it be illegal under US law to use `unwrap` in Rust code? Should it be illegal for any single internet services company to have more than X number of customers? A lot of the internet also breaks when AWS goes down because many people like to use AWS, so maybe they should be included in this regulatory framework too.
> I honestly have a hard time suggesting that those customers are individually wrong to do so - Cloudflare isn't down that often, and whatever amount it cost any individual customer by being down today might be more than offset by the DDOS protection they're buying.
We have collectively agreed to a world where software service providers have no incentive to be reliable as they are shielded from the consequences of their mistakes and somehow we see it as acceptable that software have a ton of issues and defects. The side effect is that research on actually lowering the cost of safety has little return on investment. It doesn't have be so.
> Anyway if you want Cloudflare regulated to prevent this, name the specific regulations you want to see.
I want software provider to be liable for the damage they cause and minimum quality regulation on par with an actual engineering discipline. I have always been astounded that nearly all software licences start with extremely broad limitation of liability provisions and people somehow feel fine with it. Try to extend that to any other product you regularly use in your life and see how that makes you fell.
How to do proper testing, formal methods and resilient design have been known for decades. I would personnaly be more than okay with let's move less fast and stop breaking things.
> I want software provider to be liable for the damage they cause and minimum quality regulation on par with an actual engineering discipline. I have always been astounded that nearly all software licences start with extremely broad limitation of liability provisions and people somehow feel fine with it. Try to extend that to any other product you regularly use in your life and see how that makes you fell.
So do you want to make it illegal to punish GNU GPL licensed software because that license has a warranty disclaimer? Do you want to make it illegal for a company like Cloudflare to use open source licensed software with similar warranty disclaimers, or for the SLA agreements and penalties for violating them that they make with their own paying customers to be legally unenforceable? What if I just have a personal website and I break the javascript on it because I was careless, how should that be legally treated?
I'm not against research into more reliable software or using better engineering techniques that result in more reliable software. What I'm concerned about is the regulatory regime - in other words, what software it is or is not legal to write or sell for money - and how to properly incentivize software service providers to use techniques that result in more reliable software without causing a bunch of bad second order effects.
You can't go out in the middle of your city, build a shoddy bridge, say you wave all responsibilities and then wash your hands with the consequences when it predictably breaks. Why can you do that with pieces of software?
Limiting the scope of liability waivers is not the same things as censoring what software can be produced. It's just ensuring that everyone actually take responsibility for the things they distribute.
As I said previously, the current situation doesn't make sense to me. People have been brainwashed in believing that the way software is released currently, half finished and crippled with bugs, is somehow normal and acceptable. It absolutely doesn't have to be this way.
It'a beyond shameful that the average developers today is blissfully unaware of anything related to producing actually secure pieces of software. I am pretty sure I can walk into more than 90% of development shops today and no one there will know what formal methods are. With some luck, they might have some static analysers running, probably from a random provider and be happy with the crappy percentages that it outputs.
It's not about research. It's about a field which entirely refuses to become mature despite being pivotal to the modern economy. And why would it? Software products somehow get a free pass for the shit they push on everyone.
We are in the classical "market for lemons" trap where negative externalities are not priced in and investing in security will just get you to lose against companies that don't care. Every major incidents remind us we need out. The market has already showed it won't self correct. It's a classical case where regulatory intervention is necessary and legitimate.
The shift is already happening by the way. The EU product liability directive was adopted in 2024 and the transition period ends in December 2026. The US "National Cybersecurity Strategy" signals intend to review the status quo. It's coming faster that people realise.
I find myself in the odd position of agreeing with you both.
That we’re even having this discussion is a major step forward. That we’re still having this discussion is a depressing testament to how slow slowly the mainstream has adopted better ideas.
> I can’t wait for the complete meltdown when they discover effect systems in 2040
Zig is undergoing this meltdown. Shame it's not memory safe. You can only get so far in developing programming wisdom before Eternal September kicks in and we're back to re-learning all the lessons of history as punishment for the youthful hubris that plagues this profession.
It's not about whether you should ban unwrap() in production. You shouldn't. Some errors are logic bugs beyond which a program can't reasonably continue. The problem is that the language makes it too easy for junior developers (and AI!) to ignore non-logic-bug problems with unwrap().
Programmers early in their careers will do practically anything to avoid having to think about errors and they get angry when you tell them about it.
> In production code an unwrap or expect should be reviewed exactly like a panic.
An unwrap should never make it to production IMHO. It's fine while prototyping, but once the project gets closer to production it's necessary to just grep `uncheck` in your code and replace those that can happen with a proper error management and replace those that cannot happen with `expect`, with a clear justification of why they cannot happen unless there's a bug somewhere else.
I would say, sure, if you feel the same way about panic calls making to production. In other words, review all of them the same way. Because writing unwrap/expect is exactly the same as writing “if error, panic”.
I don't understand your point: panic! is akin to expect: you think about it consciously, use it explicitly and you write down a panic message explaining its rational.
It should be. If you aren’t treating it exactly the same as panic and expect, that’s what I’m calling the “blind spot”. And why should you have to make up a message every time when the backtrace is going to tell you what was wrong?
> And why should you have to make up a message every time when the backtrace is going to tell you what was wrong?
The message isn't really here to be displayed during a crash (since the crash should never happen in the first place), it's here to communicate the invariant in the code, to the developer reading and modifying it later on.
Isn't the point of this article that pieces of infrastructure don't go down to root causes, but due to bad combinations of components that are correct individually? After reading "engineering a safer world", I find root cause analysis rather reductionistic, because it wasn't just an unwrap, it was that the payload was larger than normal, because of a query that didn't select by database, because a clickhouse made more databases visible. Hard to say "it was just due to an unwrap" imo. Especially in terms of how to fix an issue going forwards. I think the article lists a lot of good ideas, that aren't just "don't unwrap", like enabling more global kill switches for features, or eliminating the ability for core dumps or other error reports to overwhelm system resources.
You're right. A good postmortem/root cause analysis would START from "unwrap" and continue from there.
You might start with a basic timeline of what happened, then you'd start exploring: why did this change affect so many customers (this would be a line of questioning to find a potential root cause), why did it take so long to discover or recover (this might be multiple lines of questioning), etc.
> This is the multi-million dollar .unwrap() story.
That's too semantic IMHO. The failure mode was "enforced invariant stopped being true". If they'd written explicit code to fail the request when that happened, the end result would have been exactly the same.
> If the `.unwrap()` was replaced with `.expect("Feature config is too large!")` it would certainly make the outage shorter.
It wouldn't, not meaningfully. The outage was caused by change in how they processed the queries. They had no way to observe the changes, nor canaries to see that change is killing them. Plus, they would still need to manually feed and restart services that ingested bad configs.
`expect` would shave a few minutes; you would still spend hours figuring out and fixing it.
Granted, using expect is better, but it's not a silver bullet.
A billion alerts in DD/Sentry/whatever saying the exact problem that coincide with the exact graph of failures would probably be helpful if someone looked at them.
In general for unexpected errors like these the internal function should log the error, and I assume it was either logged or they can quickly deduce reason based on the line number.
I'm not sure if this is serious or not, but to take it at face value: the value of this sort of thing in Rust is not that it prevents crashes altogether but rather that it prevents _implicit_ failures. It forces a programmer to make the explicit choice of whether to crash.
There's lots of useful code where `unwrap()` makes sense. On my team, we first try to avoid it (and there are many patterns where you can do this). But when you can't, we leave a comment explaining why it's safe.
The language semantics do not surface `unwrap` usage or make any guarantees. It should be limited to use in `unsafe` blocks.
> There's lots of useful code where `unwrap()` makes sense. On my team, we first try to avoid it (and there are many patterns where you can do this). But when you can't, we leave a comment explaining why it's safe.
I would prefer the boiler plate of a match / if-else / if let, etc. to call attention to it. If you absolutely must explicitly panic. Or better - just return an error Result.
It doesn't matter how smart your engineers are. A bad unwrap can sneak in through refactors, changing business logic, changing preconditions, new data, etc.
Restricting unwrap to unsafe blocks adds negative value to the language. It won't prevent unwrap mistakes (people who play fast and loose with it today will just switch to "foo = unsafe { bar.unwrap() };" instead). And it'll muddy the purpose of unsafe by adding in a use that has nothing to do with memory safety. It's not a good idea.
That would be a fairly significant expansion of what `unsafe` means in Rust, to put it lightly. Not to mention that I think doing so would not really accomplish anything; marking unwrap() `unsafe` would not "surface `unwrap` usage" or "make any guarantees", as it's perfectly fine for safe functions to contain `unsafe` blocks with zero indication of such in the function signature and.
> fairly significant expansion of what `unsafe` means in Rust
I want an expansion of panic free behavior. We'll never get all the way there due to allocations etc., but this is the class of error the language is intended to fix.
This turned into a null pointer, which is exactly what Rust is supposed to quench.
I'll go as far as saying I would like to statically guarantee none of my dependencies use the unwrap() methods. We should be able to design libraries that provably avoid panics to the greatest extent possible.
Sure, and I'd hardly be one to disagree that a first-party method to guarantee no panics would be nice, but marking unwrap() `unsafe` is definitely not an effective way to go about it.
> but this is the class of error the language is intended to fix.
Is it? I certainly don't see any memory safety problems here.
> This turned into a null pointer, which is exactly what Rust is supposed to quench.
There's some subtlety here - Rust is intended to eliminate UB due to null pointer dereferences. I don't think Rust was ever intended to eliminate panics. A panic may still be undesirable in some circumstances, but a panic is not the same thing as unrestricted UB.
> We should be able to design libraries that provably avoid panics to the greatest extent possible.
Yes, this would be nice indeed. But again, marking unwrap() `unsafe` is not an effective way to do so.
dtolnay's no_panic is the best we have right now IIRC, and there are some prover-style tools in an experimental stage which can accomplish something similar. I don't think either of those are polished enough for first-party adoption, though.
> The language says "safe" on the tin. It advertises safety.
Rust advertises memory safety (and other closely related things, like no UB, data race safety, etc.). I don't think it's made any promises about hard guarantees of other kinds of safety.
Rust has grown beyond its original design as a "memory safe" language. People are using this as an HTTP/RPC server programming language now. WASM serverless jobs, etc. Rust has found itself deployed in a lot of unexpected places.
These folks are not choosing Rust for the memory safety guarantees. They're choosing Rust for being a fast language with a nice type system that produces "safe" code.
Rust is widely known for producing relatively defect-free code on account of its strong type system and ergonomics. Safety beyond memory safety.
Unwrap(), expect(), and their kin are a direct affront to this.
There are only two uses cases for these: (1) developer laziness, (2) the engineer spent time proving the method couldn't fail, but unfortunately they're not using language design features that allow this to be represented in the AST with static guarantees.
In both of these cases, the engineer should instead choose to (1) pass the Result<T,E> or Option<T> to the caller and let the caller decide what to do, (2) do the same, but change the type to be more appropriate to the caller, (3) handle it locally so the caller doesn't have to deal with it, (4) silently turn it into a success. That's it. That's idiomatic Rust.
I'm now panicked (hah) that some dependency of mine will unwrap something and panic at runtime. That's entirely invisible to users. It's extremely dangerous.
Today a billion people saw the result of this laziness. It won't be the last time. And hopefully it never happens in safety-critical applications like aircraft. But the language has no say in this because it isn't taking a stand against this unreasonably sharp edge yet. Hopefully it will. It's a (relatively) easy fix.
>> This is the multi-million dollar .unwrap() story.
> That's too semantic IMHO. The failure mode was "enforced invariant stopped being true". If they'd written explicit code to fail the request when that happened, the end result would have been exactly the same.
Problem is, the enclosing function (`fetch_features`) returns a `Result`, so the `unwrap` on line #82 only serves as a shortcut a developer took due to assuming `features.append_with_names` would never fail. Instead, the routine likely should have worked within `Result`.
> Instead, the routine likely should have worked within `Result`.
But it's a fatal error. It doesn't matter whether it's implicit or explicit, the result is the same.
Maybe you're saying "it's better to be explicit", as a broad generalization I don't disagree with that.
But that has nothing to do with the actual bug here, which was that the invariant failed. How they choose to implement checking and failing the invariant in the semantics of the chosen language is irrelevant.
>> Problem is, the enclosing function (`fetch_features`) returns a `Result`, so the `unwrap` on line #82 only serves as a shortcut a developer took due to assuming `features.append_with_names` would never fail. Instead, the routine likely should have worked within `Result`.
> But it's a fatal error. It doesn't matter whether it's implicit or explicit, the result is the same.
I agree it is an error, but disagree that it should be a fatal error at that location. The reason being is the method defining the offending `unwrap` construct produces a `Result`, which is fully capable of representing any error `features.append_with_names` could produce.
> But that has nothing to do with the actual bug here, which was that the invariant failed.
The bug is by invoking `unwrap` the process crashed. To the degree that Cloudfare had a massive outage.
Had the logic been such that a `Result` representing this error condition activated an alternate workflow to handle the error (perhaps by logging it, emitting a notification event alerting SRE's, transitioning into a failure mode, or all of these options), then a global outage might have been averted.
Which makes:
> How they choose to implement checking and failing the invariant in the semantics of the chosen language is irrelevant.
Of course it depends on the situation. But I don't see how you could think that in this case, crashing is better than stale config.
Crashing on a config update is usually only done if it could cause data corruption if the configs aren't in sync. That's obviously not the case here since the updates (although distributed in real time) are not coupled between hosts. Such systems usually are replicated state machines where config is totally ordered relative to other commands. Example: database schema and write operations (even here the way many databases are operated they don't strongly couple the two).
I'm not completely sure I agree. I mean, I do agree about the .unwrap() culture being a bug trap. But I don't think this example qualifies.
The root cause here was that a file was mildly corrupt (with duplicate entries, I guess). And there was a validation check elsewhere that said "THIS FILE IS TOO BIG".
But if that's a validation failure, well, failing is correct? What wasn't correct was that the failure reached production. What should have happened is that the validation should have been a unified thing and whatever generated the file should have flagged it before it entered production.
And that's not an issue with function return value API management. The software that should have bailed was somewhere else entirely, and even there an unwrap explosion (in a smoke test or pre-release pass or whatever) would have been fine.
It sounds to me like there was validation, but the system wasn't designed for the validation to ever fail - at which point crashing is the only remaining option. You've essentially turned it into an assertion error rather than a parsing/validation error.
Ideally every validation should have a well-defined failure path. In the case of a config file rotation, validation failure of the new config could mean keeping the old config and logging a high-priority error message. In the case of malformed user-provided data, it might mean dropping the request and maybe logging it for security analysis reasons. In the case of "pi suddenly equals 4" checks the most logical approach might be to intentionally crash, as there's obviously something seriously wrong and application state has corrupted in such a way that any attempt to continue is only going to make things worse.
But in all cases there's a reason behind the post-validation-failure behavior. At a certain point leaving it up to "whatever happens on .unwrap() failure" isn't good enough anymore.
This is assuming that the process could have done anything sensible while it had the malformed feature file. It might be in this case that this was one configuration file of several and maybe the program could have been built to run with some defaults when it finds this specific configuration invalid, but in the general case, if a program expects a configuration file and can't do anything without it, panicking is a normal thing to do. There's no graceful handling (beyond a nice error message) a program like Nginx could do on a syntax error in its config.
The real issue is further up the chain where the malformed feature file got created and deployed without better checks.
I do not think that if the bot detection model inside your big web proxy has a configuration error it should panic and kill the entire proxy and take 20% of the internet with it. This is a system that should fail gracefully and it didn't.
> The real issue
Are there single "real issues" with systems this large? There are issues being created constantly (say, unwraps where there shouldn't be, assumptions about the consumers of the database schema) that only become apparent when they line up.
I don't know too much about how the feature file distribution works but in the event of failure to read a new file, wouldn't logging the failure and sticking with the previous version of the file be preferable?
That's exactly the point (ie just prior to distribution) where a simple sanity check should have been run and the config replacement/update pipeline stopped on failure. When they introduced the 200 entry limit memory optimised feature loader it should have been a no-brainer to insert that sanity check in the config production pipeline.
Or even truncating the features to their limit and alerting through logs that there is likely performance degradation in their Bot Management.
I'm really confused how so many people are finding it acceptable to bring down your entire reverse-proxy because the length of feature sets for the ML model in one of your components was longer than expected.
One feature failing like this should probably log the error and fail closed. It shouldn't take down everything else in your big proxy that sits in front of your entire business.
Yea, Rust is safe but it’s not magic. However Nginx doesn’t panic on malformed config. It exits with hopefully a helpful error code and message. The question is then could the cloudflare code have exited cleanly in a way that made recovery easier instead of just straight panicking.
Would expect with a message meet that criteria of exiting with a more helpful error message? From the postmortem it seems to me like they just didn’t know it even was panicing
> However Nginx doesn’t panic on malformed config. It exits with hopefully a helpful error code and message.
The thing I dislike most about Nginx is that if you are using it as a reverse proxy for like 20 containers and one of them is up, the whole web server will refuse to start up:
nginx: [emerg] host not found in upstream "my-app"
Obviously making 19 sites also unavailable just because one of them is caught in a crash loop isn't ideal. There is a workaround involving specifying variables, like so (non-Kubernetes example, regular Nginx web server running in a container, talking to other containers over an internal network, like Docker Compose or Docker Swarm):
location / {
resolver 127.0.0.11 valid=30s; # Docker DNS
set $proxy_server my-app;
proxy_pass http://$proxy_server:8080/;
proxy_redirect default;
}
Sadly, if you try to use that approach, then you just get:
nginx: [emerg] "proxy_redirect default" cannot be used with "proxy_pass" directive with variables
Sadly, switching the redirect configuration away from the default makes some apps go into a redirect loop and fail to load: mostly legacy ones, where Firefox shows something along the lines of "The page isn't redirecting properly". It sucks especially badly if you can't change the software that you just need to run and suddenly your whole Nginx setup is brittle. Apache2 and Caddy don't have such an issue.
That's to say that all software out there has some really annoying failure modes, even is Nginx is pretty cool otherwise.
To be fair, this failed in the non-rust path too because the bot management returned that all traffic was a bot. But yes, FL2 needs to catch panics from individual components but I’m not sure if failing open is necessarily that much better (it was in this case but the next incident could easily be the result of failing open).
But more generally you could catch the panic at the FL2 layer to make that decision intentional - missing logic at that layer IMHO.
Catching panic probably isn’t a great idea if there’s any unsafe code in the system. (Do the unsafe blocks really maintain heap invariants if across panics?)
Unsafe blocks have nothing to do with it. Yes - they maintain all the same invariants as safe blocks or those unsafe blocks are unsound regardless of panics. But there’s millions of way to architect this (eg a supervisor process that notices which layer in FL2 is crashing and just completely disables that layer when it starts up the proxy again. There’s challenges here because then you have to figure out what constitutes a perma crashing (eg what if it’s just 20% of all sites? Do you disable?). And in the general case you have the fail open/fail close decision anyway which you should just annotate individual layers with.
But the bigger change is to make sure that config changes roll out gradually instead of all at once. That’s the source of 99% of all widespread outages
I think the parent is implying that the panic should be "caught" via a supervisor process, Erlang-style, rather than implying the literal use of `catch_unwind` to resume within the same process.
Supervisor is the brutalist way. But catch_unwind may be needed for perf and other reasons.
But ultimately it’s not the panic that’s the problem but a failure to specify how panics within FL2 layers should be handled; each layer is at least one team and FL2’s job is providing a safe playground for everyone to safely coexist regardless of the misbehavior of any single component
But as always such failures are emblematic of multiple things going wrong at once. You probably want to end up using both catch_unwind for the typical case and the supervisor for the case where there’s a segfault in some unsafe code you call or native library you invoke.
I also mention the fundamental tension of do you want to fail open or closed. Most layers should probably fail open. Some layers (eg auth) it’s safer to fail closed.
The unwrap should be replaced by code that creates enough alerting to make a P0 incidident from their canary deployment immediately.
OR even, the bot code crashing should itself be generating alerts.
Canary deployment would be automatically rolled back until P0 incident resolved.
All of this could probably have happened and contained at their scale in less than a minute as they would likely generate enough "omg the proxy cannot handle its config" alerts off of a deployment of 0.001% near immediately.
Agreed - a big question why the file wasn’t test driven in staging and progressively rolled out. And also what alerting was missing within FL2 that they couldn’t pinpoint the unwrap instantly.
I'm not a fan of rust, but I don't think that is the only takeaway. All systems have assumptions about their input and if the assumption is violated, it has to be caught somewhere. It seems like it was caught too deep in the system.
Maybe the validation code should've handled the larger size, but also the db query produced something invalid. That shouldn't have ever happened in the first place.
> It seems like it was caught too deep in the system.
Agreed, that's also my takeaway.
I don't see the problem being "lazy programmers shouldn't have called .unwrap()". That's reductive. This is a complex system and complex system failures aren't monocausal.
The function in question could have returned a smarter error rather than panicking, but what then? An invariant was violated, and maybe this system, at this layer, isn't equipped to take any reasonable action in response to that invariant violation and dying _is_ the correct thing to do.
But maybe it could take smarter action. Maybe it could be restarted into a known good state. Maybe this service could be supervised by another system that would have propagated its failure back to the source of the problem, alerting operators that a file was being generated in such a way that violated consumer invariants. Basically, I'm describing a more Erlang model of failure.
Regardless, a system like this should be able to tolerate (or at least correctly propagate) a panic in response to an invariant violation.
The takeaway here isn’t about Rust itself, but that the Rust marketing crew’s claims that we constantly read on HN and elsewhere about the Result type magically saving you from making mistakes is not a good message to send.
They would also tell you that .unwrap() has no place in production code, and should receive as much scrutiny as an `unsafe` block in code review :)
The point of option is the crash path is more verbose and explicit than the crash-free path. It takes more code to check for NULL in C or nil in Go; it takes more code in Rust to not check for Err.
1. They don’t. There is presumably some hypothetical world where they would tell you if you start asking questions, but nobody buying into the sales pitch ever asks questions.
2. You’re getting confused by technology again. This isn’t about technology.
> Today, many friends pinged me saying Cloudflare was down. As a core developer of the first generation of Cloudflare FL, I'd like to share some thoughts.
> This wasn't an attack, but a classic chain reaction triggered by “hidden assumptions + configuration chains” — permission changes exposed underlying tables, doubling the number of lines in the generated feature file. This exceeded FL2's memory preset, ultimately pushing the core proxy into panic.
> Rust mitigates certain errors, but the complexity in boundary layers, data flows, and configuration pipelines remains beyond the language's scope. The real challenge lies in designing robust system contracts, isolation layers, and fail-safe mechanisms.
> Hats off to Cloudflare's engineers—those on the front lines putting out fires bear the brunt of such incidents.
> Technical details: Even handling the unwrap correctly, an OOM would still occur. The primary issue was the lack of contract validation in feature ingest. The configuration system requires “bad → reject, keep last-known-good” logic.
> Why did it persist so long? The global kill switch was inadequate, preventing rapid circuit-breaking. Early suspicion of an attack also caused delays.
> Why not roll back software versions or restart?
> Rollback isn't feasible because this isn't a code issue—it's a continuously propagating bad configuration. Without version control or a kill switch, restarting would only cause all nodes to load the bad config faster and accelerate crashes.
> Why not roll back the configuration?
> Configuration lacks versioning and functions more like a continuously updated feed. As long as the ClickHouse pipeline remains active, manually rolling back would result in new corrupted files being regenerated within minutes, overwriting any fixes.
This tweet thread invokes genuine despair in me. Do we really have to outsource even our tweets to LLMs? Really? I mean, I get spambots and the like tweeting mass-produced slop. But what compels a former engineer of the company in question to offer LLM-generated "insight" to the outage? Why? For what purpose?
* For clarity, I am aware that the original tweets are written in Chinese, and they still have the stench of LLM writing all over them; it's not just the translation provided in the above comment.
This particular excerpt is reeking of it with pretty much every line. I'll point out the patterns in the English translation, but all of these patterns apply cross-language.
"Classic/typical "x + y"", particularly when diagnosing an issue. This one is a really easy tell because humans, on aggregate, do not use quotation marks like this. There is absolutely no reason to quote these words here, and yet LLMs will do a combined quoted "x + y" where a human would simply write something natural like "hidden assumptions and configuration chains" without extraneous quotes.
> The configuration system requires “bad → reject, keep last-known-good” logic.
Another pattern with overeager usage of quotes is this ""x → y, z"" construct with very terse wording.
> This wasn't an attack, but a classic chain reaction
LLMs aggressively use "Not X, but Y". This is also a construct commonly used by humans, of course, but aside from often being paired with an em-dash, another tell is whether it actually contributes anything to the sentence. "Not X, but Y" is strongly contrasting and can add a dramatic flair to the thing being constrasted, but LLMs overuse it on things that really really don't need to be dramatised or contrasted.
> Rust mitigates certain errors, but the complexity in boundary layers, data flows, and configuration pipelines remains beyond the language's scope. The real challenge lies in designing robust system contracts, isolation layers, and fail-safe mechanisms.
Two lists of three concepts back-to-back. LLMs enjoy, love, and adore this construct.
> Hats off to Cloudflare's engineers—those on the front lines putting out fires bear the brunt of such incidents.
This kind of completely vapid, feel-good word soup utilising a heroic analogy for something relatively mundane is another tell.
And more broadly speaking, there's a sort of verbosity and emptiness of actual meaning that permeates through most LLM writing. This reads absolutely nothing like what an engineer breaking down an outage looks like. Like, the aforementioned line of... "Rust mitigates certain errors, but the complexity in boundary layers, data flows, and configuration pipelines remains beyond the language's scope. The real challenge lies in designing robust system contracts, isolation layers, and fail-safe mechanisms.". What is that actually communicating to you? It piles on technical lingo and high-level concepts in a way that is grammatically correct but contains no useful information for the reader.
Bad writing exists, of course. There's plenty of bad writing out there on the internet, and some of it will suffer from flaws like these even when written by a human, and some humans do like their em-dashes. But it's generally pretty obvious when the writing is taken on aggregate and you see recognisable pattern after pattern combined with em-dashes combined with shallowness of meaning combined with unnecessary overdramatisations.
Swift has implicit unwrap (!), and explicit unwrap (?).
I don't like to use implicit unwrap. Even things that are guaranteed to be there, I treat as explicit (For example, (self.view?.isEnabled ?? false), in a view controller, instead of self.view.isEnabled).
So what happens if it ends up being nil? How does your app react?
In this particular case, I would rather crash. It’s easier to spot in a crash report and you get a nice stack trace.
Silent failure is ultimately terrible for users.
Note: for the things I control I try to very explicitly model state in such a way as I never need to force unwrap at all. But for things beyond my control like this situation, I would rather end the program than continue with a state of the world I don’t understand.
Yeah @IBOutlets are generally the one thing that are allowed to be implicitly-unwrapped optionals. They go along with using storyboards & xibs files with Interface Builder. I agree that you really should just crash if you are attempting to access one and it is nil. Either you have done something completely incorrect with regards to initializing and accessing parts of your UI and want to catch that in development, or something has gone horribly, horribly, horribly with UIKit/AppKit and storyboard/xib files are not being loaded properly by the system.
A good tool for catching stuff during development, is the humble assert()[0]. We can use precondition()[1], to do the same thing, in ship code.
The main thing is, is to remain in control, as much as possible. Rather than let the PC leave the stack frame, throw the error immediately when it happens.
> Silent failure is ultimately terrible for users.
Agreed.
Unfortunately, crashes in iOS are “silent failures,” and are a loss of control.
What this practice does, is give me the option to handle the failure “noisily,” and in a controlled manner; even if just emitting a log entry, before calling a system failure. That can be quite helpful, in threading. Also, it gives me the option to have a valid value applied, if there’s a structural failure.
But the main reason that I do that with @IBOutlets, is that it forces me to acknowledge, throughout the rest of the code, that it’s an optional. I could always treat implicit optionals as if they were explicit, anyway. This just forces me to.
I have a bunch of practices that folks can laugh at, but my stuff works pretty effectively, and I sleep well.
Crashes are silent failures but as I mentioned: you can get a lot of your crashes reported via the App Store. This is why I prefer crashes in this situation: it gives me something actionable over silent failures on the client.
And you will find the problem very early if you crash. You are much less likely to find the problem if you don’t.
What have you found useless about the crash reports from the App Store? It would be really nice for it to have something like a breadcrumb capability, but typically the stack trace of the crash is sufficient to see what went wrong.
This is terrible. The whole reason they introduced this is because IBOutlets would get silently disconnected and then in the field a user would complain that a feature stopped working.
Crash early, crash often. Find the bugs and bad assumptions.
> without taking the time to understand why they do things, the way they do.
Oh I am aware. They do it because
A) they don’t have a mental model of correct execution. Events just happen to them with a feeling of powerlessness. So rather than trying to form one they just litter the code with cases things that might happen
> As I've gotten older, the sharp edges have been sanded off.
B) they have grown in bad organizations with bad incentives that penalize the appearance of making mistakes. So they learn to hide them.
For example there might be an initiative that rewards removing crashes in favor of silent error.
> This is textbook "parse, don't validate" anti-pattern.
How so? “Parse, don’t validate” implies converting input into typed values that prevent representation of invalid state. But the parsing still needs to be done correctly. An unchecked unwrap really has nothing to do with this.
In addition, it looks like this system wasn't on any kind of 1%/10%/50%/100% rollout gating. Such a rollout would trivially have shown the poison input killing tasks.
To me it reads like there was a gradual rollout of the faulty software responsible for generating the config files, but those files are generated on approximately one machine, then propogated across the whole network every 5 minutes.
> Bad data was only generated if the query ran on a part of the cluster which had been updated. As a result, every five minutes there was a chance of either a good or a bad set of configuration files being generated and rapidly propagated across the network.
It looks like changing the permissions triggered creation of a new feature file, and it was ingestion of that file leading to blowing a size limit that crashed the systems.
The file should be versioned and rollout of new versions should be staged.
(There is definitely a trade-off; often times in the security critical path, you want to go as fast as possible because changes may be blocking a malicious actor. But if you move too fast, you break things. Here, they had a potential poison input in the pathway for synchronizing this state and Murphy's Law suggests it was going to break eventually, so the question becomes "How much damage can we tolerate when it does?")
> It looks like changing the permissions triggered creation of a new feature file, and it was ingestion of that file leading to blowing a size limit that crashed the systems.
That feature file is generated every 5 minutes at all times; the change to permissions was rolled out gradually over the clickhouse cluster, and whether a bad version of that file was generated depended on whether the part of the cluster that had the bad permissions generated the file.
While this is true, I wish that Rust had more of a first-class support for `no_panic`. Every solution we do have is hacky. I wish that I could guarantee that there were no panic calls anywhere in a code path.
But I could screw it up in Go, if I made the same assumptions
fvs, err := features.AppendWithNames(..)
if err != nil {
// this will NEVER break
panic(err)
}
Ultimately I don't think language design can be the sole line of defence against system failures; it can only guide developers to think about error cases
Right, but the point isn't to make errors impossible; the point is to have them be 1) less likely to write, and 2) easier to spot on review.
People's biggest complaints about golang's errors:
1. You have to _TYPE_OUT_ what to do on EVERY.SINGLE.ERROR. SOO BOORING!
2. They clutter up the code and make it look ugly.
Rust is so much cleaner and more convenient (they say)! Just add ?, or .unwrap()!
Well, with ".unwrap()", you can type it fast enough that you're on to the next problem before it occurs to your brain to think about what to do if there is an error. Whereas, in golang, by the time you type in, "if err != nil {", you've broken the flow enough that now you're much more likely to be thinking, "Hmm, could this ever fail? What should we do if it does?" That break in flow is annoying, but necessary.
And ".unwrap()" looks so unassuming, it's easy to overlook on review; that "panic()" looks a lot more dangerous, and again, would be more likely to trigger a reviewer into thinking, "Wait, is it OK if this thing panics? Is this really so unlikely to happen?"
Renaming it `.unwrap_or_panic()` would probably help with both.
Unfortunately none of the meanings Wikipedia knows [1] seems to fit this usage. Did you perhaps mean "taboo"?
I disagree that "unwrap()" seems as scary as "panic()", but I will certainly agree to sibling commenters have a point when they say that "bar, _ := foo()" is a lot less scary than "unwrap()".
That may be me, but `.unwrap()` is much more obvious than `_`:
- it's literally written out that you're assuming it to be Ok
- there are no indications that the `_` is an error: it could very well be some other return value from the function. in your example, it could be the number of appended features, etc
That's why Go's error handling is indeed noisy: it's noise and you reduce noise by not handling errors. Rust's is terse yet verbose: if you add stuff it's because you're doing something wrong. You explicitly spelled out the error is being ignored.
> And it would be far more obvious that an error message is being ignored.
Haven't used Go so maybe I'm missing some consideration, but I don't see how ", _" is more obvious than ".unwrap()". If anything it seems less clear, since you need to check/know the function's signature to see that it's an error being ignored (wouldn't be the case for a function like https://pkg.go.dev/math#Modf).
I haven't been writing Rust for that long (about 2 years) but every time I see .unwrap() I read it as 'panic in production'. Clippy needs to have harder checks on unwrap.
If the error had been an exception instead of a result, could have bubbled up
I have been saying for years that Rust botched error handling in unfixable ways. I will go to the grave believing Rust fumbled.
The design of the Rust language encourages people to use unwrap() to turn foreseeable runtime problems into fatal errors. It's the path of least resistance, so people will take it.
Rust encourages developers to consider only the happy path. No wonder it's popular among people who've never had to deal with failure.
All of the concomitant complexity--- Result, ?, the test thing, anyhow, the inability for stdlib to report allocation failure --- is downstream of a fashion statement against exceptions Rust cargo-culted from Go.
The funniest part is that Rust does have exceptions. It just calls them panics. So Rust code has to deal with the ergonomic footgun of Result but pays anyway for the possibility of exceptions. (Sure, you can compile with panic=abort. You can't count on it.)
I could not be more certain that Rust should have been a language with exceptions, not Result, and that error objects are a gross antipattern we'll regret for decades.
Errors work just like exceptions especially if you use the ? operator and let the error bubble up the chain. This is the Rust equivalent of an unhandled exception and the ripcord being pulled.
In C++, functions are error-colored by default. You write "noexcept" if you want your function to be infallible-colored instead.
(You usually want to make a function infallible if you're using your noexcept function as part of a cleanup path or part of a container interface that allows for more optimizations of it knows certain container operations are infallible.)
Rust makes infallibility the syntactic default and makes you write Result to indicate fallibility. People often don't want to color their functions this way. Guess what happens when a programmer is six levels deep in infallible-colored function calls and does something that can fail.
.unwrap()
Guess what, in Rust, is fallible?
Mutex acquire.
Guess what you need to do often on infallible cleanup paths?
By the way - does this discussion matter and were they wrong to use unwrap()?
The way they wrote the code means that having more than 200 features is a hard non-transient error - even if they recovered from it, it meant they'd have had the same error when the code got to the same place.
I'm sure when the process crashed, k8s restarted the pod or something - then it reran the same piece of code and crashed in the same place.
While I don't necessarily agree with crashing as business strategy, I don't think that doing anything other than either dropping the extra rules or allocating more memory - neither of which the original code was built to do (probably by design).
The code made the local hard assumption that there won't ever be more than 200 rules and its okay to crash if that count is exceeded.
If you design your code around an invariant never being violated (which is fine), you have to make it clear on a higher level that they did.
This isn't a Rust problem (though Rust does make it easy to do the wrong thing here imo)
Instead of crashing when applying the new config, it's more common to simply ignore the new config if it cannot be applied. You keep running in the last known good state. Operators then get alerts about the failures and can diagnose and resolve the underlying issue.
That's not always foolproof, e.g. a freshly (re)started process doesn't have any prior state it can fall back to, so it just hard crashes. But restarts are going to be rate limited anyways, so even then there is time to mitigate the issue before it becomes a large scale outage
Not panicking code is tedious to write. It is not realistic to expect everything to be non panic. There is a reason that panicking exists in the first place.
Them calling unwrap on a limit check is the real issue imo. Everything that takes in external input should assume it is bad input and should be fuzz tested imo.
In the end, what is the point of having a limit check if you are just unwrapping on it
Using the question mark operator [1] and even adding in some anyhow::context goes a long way to being able to fail fast and return an Err rather then panicking.
Sure you need to handle Results all the way up the stack but it forces you to think about how those nested parts of your app will fail as you travel back up the stack.
Interesting to see Rust error handling flunk out in practice.
It may be that forcing handling at every call tends to makes code verbose, and devs insensitized to bad practice. And the diagnostic Rust provided seems pretty garbage.
There is bad practice here too -- config failure manifesting as request failure, lack of failing to safe, unsafe rollout, lack of observability.
Back to language design & error handling. My informed view is that robustness is best when only major reliability boundaries need to be coded.
This the "throw, don't catch" principle with the addition of catches on key reliability boundaries -- typically high-level interactions where you can meaningfully answer a failure.
For example, this system could have a total of three catch clauses "Error Loading Config" which fails to safe, "Error Handling Request" which answers 5xx, and "Socket Error" which closes the HTTP connection.
> It may be that forcing handling at every call tends to makes code verbose
Rust has a lot of helpers to make it less verbose, even that error they demonstrate could've been written in some form `...code()?` with `?` helper that would have propagated the error forwards.
However I do acknowledge that writing Error types is boring sometimes so people don't bother to change their error types and just unwrap. But even my dinghy little apps for my personal use I do simple serach `unwrap` and make sure I have as few as possible.
I don't understand how your takeaway is that this is a language flaw other than to assume that you have some underlying disdain for Rust. That's fine, but state it clearly please.
The end result would've been the exact same if they "handled" the error: a bunch of 500s. The language being used doesn't matter if an invariant in your system is broken.
Some languages and style guides simply forbid throwing exceptions without catching / proper recovery. Google C++ bans exceptions and the main mechanism for propogating errors is `absl::Status` which the caller has to check. Not familiar with Rust but it seems unwrap is such a thing that would be banned.
> Not familiar with Rust but it seems unwrap is such a thing that would be banned.
Panics aren't exceptions, any "panic" in Rust can be thought of as an abort of the process (Rust binaries have the explicit option to implement panics as aborts). Companies like Dropbox do exactly this in their similar Rust-based systems, so it wouldn't surprise me if Cloudflare does the same.
"Banning exceptions" wouldn't have done anything here, what you're looking for is "banning partial functions" (in the Haskell sense).
Yeah I know but isn't unwrap basically a trivial way to: (1) give up catching the exception/error (the E part in `Result<T, E>`) that the callee throws; and also (2) escalate it to the point that nothing can catch it. It has such a benign name.
Unwrap is used in places where in C++ you would just have undefined behavior. It wouldn't make any more sense to blanket ban it than it would ban ever dereferencing a pointer just in case its null - even if you just checked that it wasn't null.
Rust's foo: Option<&T> is rust's rough equivalent to C++'s const T* foo. The C++ *foo is equivalent to the rust unsafe{ *foo.unwrap_unchecked() }, or in safe code *foo.unwrap() (which changes the undefined behavior to a panic).
Rust's unwrap isn't the same as std::expected::value. The former panics - i.e. either aborts the program or unwinds depending on context and is generally not meant to be handled. The latter just throws an exception that is generally expected to be handled. Panics and exceptions use similar machinery (at least they can depending on compiler options) but they are not equivalent - for example nested panics in destructors always abort the program.
In code that isn't meant to crash `unwind` should be treated as a sign saying that "I'm promising that this will never happen", but just like in C++ where you promise that pointers you deference are valid and signed integers you add don't overflow making promises like that is a necessary part of productive programming.
Linting is not good enough. The compiler should refuse to compile the code without it marked with an explicit annotation. Too much Rust code is panic happy since using casual use of `unwrap` is perma-etched into everyone's minds by the amount of demo code out there using unwrap.
> same like chess, engine is better than human grandmaster because its solvable math field
Might be worth noting that your description of chess is slightly incorrect. Chess technically isn't solved in the sense that the optimal move is known for any arbitrary position is known; it's just that chess engines are using what amounts to a fancy brute force for most of the game and the combination of hardware and search algorithm produces a better result than the human brain does. As such, chess engines are still capable of making mistakes, even if actually exploiting them is a challenge.
> because these thing called BEST MOVE and BAD MOVE there in chess
The thing is that there is no known general objective criteria for "best" and "bad" moves. The best we have so far is based on engine evaluations, but as I said before that is because chess engines are better at searching the board's state space than humans, not because chess engines have solved chess in the mathematical sense. Engines are quite capable of misevaluating positions, as demonstrated quite well by the Top Chess Engine Championship [0] where one engine thinks it made a good move while the other thinks that move is bad, and this is especially the case when resources are limited.
The closest we are to solving chess are via tablebases, which are far from covering the entire state space and are basically as much of an exemplar of pure brute force as you can get.
> "chess engines are still capable of making mistakes", I'm sorry no
If you think chess engines are infalliable, then why does the Top Chess Engine Championship exist? Surely if chess engines could not make mistakes they would always agree on a position's evaluation and what move should be made, and therefore such an exercise would be pointless?
> inaccurate yes but not mistake
From the perspective to attaining perfect play an inaccuracy is a mistake.
"The thing is that there is no known general objective criteria for "best" and "bad" moves."
are you playing chess or not?????? if you playing chess then its oblivious how to differentiate bad move and best move
Yes it is objective, these thing called best move not without reason
"If you think chess engines are infalliable, then why does the Top Chess Engine Championship exist?"
to create better chess engine like what do even talking about here????, are you saying just because there are older bad engine that mean this thing is pointless ????
if you playing chess up to a decent level 1700+ (like me), you know that these argument its wrong and I assure you to learn chess to a decent level
up until that point that you know high level chess is brute force games and therefore solvable math
> if you playing chess then its oblivious how to differentiate bad move and best move
The key words in what I said are "general" and "objective". Yes, it's possible to determine "good" or "bad" moves in specific positions. There's no known method to determine "good" or "bad" moves in arbitrary positions, as would be required for chess to be considered strongly solved.
Furthermore, if it's "obvious" how to differentiate good and bad moves then we should never see engines blundering, right?
So (for example) how do you explain this game between Stockfish and Leela where Stockfish blunders a seemingly winning position [0]? After 37... Rdd8 both Stockfish and Leela think white is clearly winning (Stockfish's evaluation is +4.00, while Leela's evaluation is +3.81), but after 38. Nxb5 Leela's evaluation plummets to +0.34 while Stockfish's evaluation remains at +4.00. In the end, it turns out Leela was correct after 40... Rxc6 Stockfish's evaluation also drops from +4.28 to 0.00 as it realizes that Leela has a forced stalemate.
Or this game also between Stockfish and Leela where Leela blunders into a forced mating sequence and doesn't even realize it for a few moves [1]?
Engines will presumably always play what they think is the "best" move, but clearly sometimes this "best" move is wrong. Evidently, this means differentiating "good" and "bad" moves is not always obvious.
> Yes it is objective, these thing called best move not without reason
If it's objective, then why is it possible for engines to disagree on whether a move is good or bad, as they do in the above example and others?
> to create better chess engine like what do even talking about here????
The ability to create better chess engines necessarily implies that chess engines can and do make mistakes, contrary to what you asserted.
> are you saying just because there are older bad engine that mean this thing is pointless ????
No. What I'm saying is that your explanation for why chess engines are better than humans is wrong. Chess engines are not better than humans because they have solved chess in the mathematical sense; chess engines are better than humans because they search the state space faster and more efficiently than humans (at least until you reach 7 pieces on the board).
> up until that point that you know high level chess is brute force games and therefore solvable math
"Solvable" and "solved" are two very different things. Chess is solvable, in theory. Chess is very far from being solved.
This is why the Erlang/Elixir methodology of having supervision and letting things crash gracefully is so useful. You can either handle every single error gracefully or handle crashing gracefully - it's much easier and more realistic in large codebases to do the later.
This would not have helped: the code would crash before doing anything useful at all.
If anything, the "crash early" mentality may even be nefarious: instead of handling the error and keeping the old config, you would spin on trying to load a broken config on startup.
Continuing only makes sense for cases you know you can handle.
_In theory_ they could have used the old config, but maybe there are reasons that’s not possible in Cloudflare’s setup. Whether or not that’s an invariant violation or just an error that can be handled and recovered from is a matter of opinion in system design.
And crashing on an invariant violation is exactly the right thing to do rather than proceed in an undefined state.
Given the context and what the configuration file contains, I'd argue it's mission-critical for the software to keep running with the previous configuration. Otherwise you might shutdown the internet. Honestly, I'm pretty sure their pre-rewrite version had such logic, and it was forgotten or still on the TODO pile for the rewrite version.
At a previous job (cloud provider), we've had exactly this kind of issue, with exactly the same root cause. The entrypoint for the whole network had a set of rules (think a NAT gateway) that were reloaded periodically from the database. Someone rewrote that bit of plumbing from Python to Go. Someone else performed a database migration. Suddenly, the plumbing could not find the data, and pushed an empty file to prod. The rewrite lacked "if empty, do nothing and raise an alert", that the previous one had. I'll let you imagine what happened next :)
Say what you want exception haters, but at least in exceptions-as-default languages, the decision of a particular issues is fatal to the whole program can be decided centrally at a high level, and not every choice is forced to be up to individual discretion.
Safe things should be easy, dangerous things should be hard.
This .unwrap() sounds too easy for what it does, certainly much easier than having an entire try..catch block with an explicit panic. Full disclosure: I don't actually know Rust.
Any project has to reason about what sort of errors can be tolerated gracefully and which cannot. Unwrap is reasonable in scenarios you expect to never be reached, because otherwise your code will be full of all sorts of possible permutations and paths that are harder to reason about and may cascade into extremely nuanced or subtle errors.
Rust also has a version of unwrap called "expect" where you provide a string that logs why the unwrap occurred. It's similar, but for pieces of code that are crucial it could be a good idea to require all 'unwraps' to instead be 'expects' so that people at least are forced to write down a reason why they believe the unwrap can never be reached.
I wonder if similar to infrastructure resilience, code resilience is also required for critical services that can never go down? Instead of relying on a single implementation for a critical service, have multiple independent implementations in different languages.
Back when I was running my own DNS servers, I did always ensure that primary and secondary were running on different platforms and different software.
That's such a bad take after reading the article. If you're going to write a system that preallocates and is based on hard assumptions about max size - the panic/unwrap approach is reasonable.
The config bug reaching prod without this being caught and pinpointed immediately is the strange part.
It's reasonable when testing protocols exercise the panic scenario. This is the problem with punting on error recovery. Nobody checks faults that propagate across domains of responsibility.
This is a bummer. The unwrap()'ing function already returned a result and should have just propagated the error. Presumably the caller could have handled more sensibly than just panic'ing.
tokio default behavior within a task is to ignore panics, such as an Err/None unwrap, and only crash that task, so it's impact limited so that's nice, maybe that's where the snowblindness came from.
it'd be kinda hard to amend the clippy lints to ignore coroutine unwraps but still pipe up on system ones. i guess.
edit: i think they'd have to be "solely-task-color-flavored" so definitely probably not trivial to infer
At Facebook they name certain "escape hatch" functions in a way that inescapably make them look like a GIANT EYESORE. Stuff like DANGEROUSLY_CAST_THIS_TO_THAT, or INVOKE_SUPER_EXPENSIVE_ACTION_SEE_YOU_ON_CODE_REVIEW. This really drives home the point that such things must not be used except in rare extraordinary cases.
If unwrap() were named UNWRAP_OR_PANIC(), it would be used much less glibly. Even more, I wish there existed a super strict mode when all places that can panic are treated as compile-time errors, except those specifically wrapped in some may_panic_intentionally!() or similar.
React.__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED comes to mind. I did have to reach to this before, but it certainly works for keeping this out of example code and other things like reading other implementations without the danger being very apparent.
At some point it was renamed to __CLIENT_INTERNALS_DO_NOT_USE_OR_WARN_USERS_THEY_CANNOT_UPGRADE which is much less fun.
right and if the language designers named it UNWRAP_OR_PANIC() then people would rightfully be asking why on earth we can't just use a try-catch around code and have an easier life
But a panic can be caught and handled safely (e.g. via std:: panic tools). I'd say that this is the correct use case for exceptions (ask Martin Fowler, of all people).
There is already a try/catch around that code, which produces the Result type, which you can presumptuously .unwrap() without checking if it contains an error.
Instead, one should use the question mark operator, that immediately returns the error from the current function if a Result is an error. This is exactly similar to rethrowing an exception, but only requires typing one character, the "?".
How so? An exception is a value that's given the closest, conceptually appropriate, point that was decided to handle the value, allowing you to keep your "happy path" as clean code, and your "exceptional circumstances" path at the level of abstraction that makes sense.
It's way less book-keeping with exceptions, since you, intentionally, don't have to write code for that exceptional behavior, except where it makes sense to. The return by value method, necessarily, implements the same behavior, where handling is bubbled up to the conceptually appropriate place, through returns, but with much more typing involved. Care is required for either, since not properly bubbling up an exception can happen in either case (no re-raise for exceptions, no return after handling for return).
There are many many pages of text discussing this topic, but having programmed in both styles, exceptions make it too easy for programmer to simply ignore them. Errors as values force you to explicitly handle it there, or toss it up the stack. Maybe some other languages have better exception handling but in Python it’s god awful. In big projects you can basically never know when or how something can fail.
I would claim the opposite. If you don't catch an exception, you'll get a halt.
With return values, you can trivially ignore an exception.
let _ = fs::remove_file("file_doesn't_exist");
or
value, error = some_function()
// carry on without doing anything with error
In the wild, I've seen far more ignoring return errors, because of the mechanical burden of having type handling at every function call.
This is backed by decades of writing libraries. I've tried to implement libraries without exceptions, and was my admittedly cargo-cult preference long ago, but ignoring errors was so prevalent among the users of all the libraries that I now always include a "raise" type boolean that defaults to True for any exception that returns an error value, to force exceptions, and their handling, as default behavior.
> In big projects you can basically never know when or how something can fail.
How is this fundamentally different than return value? Looking at a high level function, you can't know how it will fail, you just know it did fail, from the error being bubbled up through the returns. The only difference is the mechanism for bubbling up the error.
Maybe some water is required for this flame war. ;)
Correction: unchecked exceptions are hidden control flow. Checked exceptions are quite visible, and I think that more languages should use them as a result.
I'd categorize them more as "event handlers" than "hidden". You can't know where the execution will go at a lower level, but that's the entire point: you don't care. You put the handlers at the points where you care.
> don't have to write code for that exceptional behavior, except where it makes sense to.
The great Raymond Chen wrote an excellent blog post on how this isn't really true, and how exceptions can lure programmers into mistakenly thinking they can just forget about failure cases.
I mean his post seems obviously wrong or ill chosen to support his point. Surely you can see that an inner implementation of the icon class requiring a special hidden order on which properties to set first can happen in any language and also really isn't related at all to whether you use try-catch handling or error values as return codes.
What he seems to be saying is that "obviously in C I would be checking the icon handle for being non-null so clearly error value handling is superior" but this is only obvious to someone knowing the API and checking values for validity has to be done in exception based code too. It's just that exception based code doesn't pretend that it cannot panic somewhere where you don't know. The default, better assumption for programming is that you don't know what this code is doing but it should just work. Unchecked exception handling is the best way to fit that paradigm, you should not have to care about every single line and what it does and constantly sort of almost obsessively check error values of all the APIs you ever use to have this false hope that it cannot panic because you did your duty. No, it can still panic and all this error checking is not helping you program better or more clearly or faster. It swamps the code with so many extra lines that it's practically double the size. All this makes it less clear and that is also what his post shows.
> Surely you can see that an inner implementation of the icon class requiring a special hidden order
In practice, programmers don't find it easy to keep in mind that certain functions might throw. This is a real problem with unchecked exceptions and with C-style error codes that sloppy programmers might ignore entirely.
> [...] on which properties to set first can happen in any language
A carefully designed library using a statically typed functional language, especially a pure functional language, might sometimes be able to eliminate such hidden ordering bugs.
Rust used to have a feature to help the compiler detect invalid ordering of imperative operations, called typestates. This feature has since been mostly removed, though, as it saw little use. [0]
> isn't related at all to whether you use try-catch handling or error values as return codes
I guess Chen is assuming a reasonably diligent programmer who makes a habit of never discarding status/error values returned by functions. C++'s [[nodiscard]] can help ensure this.
(Of course, outside of C++, those aren't the only options. Idiomatic Haskell and Zig code forces the programmer to explicitly handle the possibility of an error. Same goes for Java's checked exceptions.)
> What he seems to be saying is that "obviously in C I would be checking the icon handle for being non-null so clearly error value handling is superior"
I don't think he's exactly arguing for the C-style approach, he's more just criticizing exceptions, especially unchecked exceptions. I agree the C-style approach has considerable problems.
> It's just that exception based code doesn't pretend that it cannot panic somewhere where you don't know.
With checked exceptions, you know precisely which operations can throw.
> Unchecked exception handling is the best way to fit that paradigm, you should not have to care about every single line and what it does and constantly sort of almost obsessively check error values of all the APIs you ever use to have this false hope that it cannot panic because you did your duty
You do need to care about every line, or your plausible-looking code is likely to misbehave when an exception occurs, as Chen's post demonstrates. Unchecked exceptions deprive the compiler of the ability to ensure good exception-handling coverage. There is no error-handling model that allows to programmer to write good code by pretending errors won't arise.
(I presume that by panic you mean throw an unchecked exception.)
...and you can? try-catch is usually less ergonomic than the various ways you can inspect a Result.
try {
data = some_sketchy_function();
} catch (e) {
handle the error;
}
vs
result = some_sketchy_function();
if let Err(e) = result {
handle the error;
}
Or better yet, compare the problematic cases where the error isn't handled:
data = some_sketchy_function();
vs
data = some_sketchy_function().UNWRAP_OR_PANIC();
In the former (the try-catch version that doesn't try or catch), the lack of handling is silent. It might be fine! You might just depend on your caller using `try`. In the latter, the compiler forces you to use UNWRAP_OR_PANIC (or, in reality, just unwrap) or `data` won't be the expected type and you will quickly get a compile failure.
What I suspect you mean, because it's a better argument, is:
which is fair, although how often is it really the right thing to let all the errors from 4 independent sources flow together and then get picked apart after the fact by inspecting `e`? It's an easier life, but it's also one where subtle problems constantly creep in without the compiler having any visibility into them at all.
it's practically always the case that you use a try-catch for more than just one source / line of code. I mean except for database/network calls I don't think I even remember a single case where I ever used a try-catch just for a single line of code. The subtle problems come from errors handling via values. You check but do you check perfectly? What happens when APIs change and the underlying functions add more error cases, then you constantly have more work to do. Nonstop constant error checking that you don't care about. This is exactly where humans are terrible: Really important work that is drudgery and where if you ever mess up once, you fail in very painful ways. Exception handling solves all of this, it fits how humans should be working and it fits the underlying hardware reality as well: We are big picture, we should not be designing languages for describing logic that force us to do drudgery work constantly and care about implementation details of every single thing we call.
Unwrap isn't a synonym for laziness, it's just like an assertion, when you do unwrap() you're saying the Result should NEVER fail, and if does, it should abort the whole process. What was wrong was the developer assumption, not the use of unwrap.
It also makes it very obvious in the code, something very dangerous is happening here. As a code reviewer you should see an unwrap() and have alarm bells going off. While in other languages, critical errors are a lot more hidden.
> What was wrong was the developer assumption, not the use of unwrap.
How many times can you truly prove that an `unwrap()` is correct and that you also need that performance edge?
Ignoring the performance aspect that often comes from a hat-trick, to prove such a thing you need to be wary of the inner workings of a call giving you a `Return`. That knowledge is only valid at the time of writing your `unwrap()`, but won't necessarily hold later.
Also, aren't you implicitly forcing whoever changes the function to check for every smartass dev that decided to `unwrap` at their callsite? That's bonkers.
I doubt that this unwrap was added for performance reasons; I suspect it was rather added because the developer temporarily didn't want to deal with what they thought was an unlikely error case while they were working on something else; and no other system recognized that the unwrap was left in and flagged it before it was deployed on production servers.
If I were Cloudflare I would immediately audit the codebase for all uses of unwrap (or similar rust panic idioms like expect), ensure that they are either removed or clearly documented as to why it's worth crashing the program there, and then add a linter to their CI system that will fire if anyone tries to check in a new commit with unwrap in it.
Panics are for unexpected error conditions, like your caller passed you garbage. Results are for expected errors, like your caller passed you something but it's your job to tell if it's garbage.
So the point of unwrap() is not to prove anything. Like an assertion it indicates a precondition of the function that the implementer cannot uphold. That's not to say unwrap() can't be used incorrectly. Just that it's a valid thing to do in your code.
> No more than returning an int by definition means the method can return -2.
What? Returning an int does in fact mean that the method can return -2. I have no idea what your argument is with this, because you seem to be disagreeing with the person while actually agreeing with them.
The difference is functions which return Result have explicitly chosen to return a Result because they can fail. Sure, it might not fail in the current implementation and/or configuration, but that could change later and you might not know until it causes problems. The type system is there to help you - why ignore it?
Because it would be a huge hassle to go into that library and write an alternate version that doesn't return a Result. So you're stuck with the type system being wrong in some way. You can add error-handling code upfront but it will be dead code at that point in time, which is also not good.
As a hypothetical example, when making a regex, I call `Regex::new(r"/d+")` which returns a result because my regex could be malformed and it could miscompile. It is entirely reasonable to unwrap this, though, as I will find out pretty quickly that it works or fails once I test the program.
Yeah, I think I expressed wrongly here. A more correct version would be: "when you do unwrap() you're saying that an error on this particular path shouldn't be recoverable and we should fail-safe."
It's a little subtler than this. You want it to be easy to not handle an error while developing, so you can focus on getting the core logic correct before error-handling; but you want it to be hard to deploy or release the software without fully handling these checks. Some kind of debug vs release mode with different lints seems like a reasonable approach.
What's the point of this sarcastic comment? Do you think that some people claim that Rust's memory safety guarantees mean that a Rust program is incapable of crashing or having a bug? This is a dumb thing to claim certainly, but I'm not aware of anyone actually making this claim.
I'm also not sure what you're getting at with the comment about exception handling being lame. I think the ML/Haskell inspired model that Rust uses of having a parameterized Result type for fallible operations is generally better than exceptions for a variety of reasons (although maybe better Exception semantics could help with some of this), but what does this have to do with match blocks?
I know, this is "Monday morning quarterbacking", but that's what you get for an outage this big that had me tied up for half a day.