35
Cloudflare outage on November 18, 2025 (blog.cloudflare.com)
submitted 11 months ago by to c/programming@programming.dev
7 comments fedilink hide all child comments
you are viewing a single comment's thread
view the rest of the comments
[–] 8 points 11 months ago (5 children)

They used .unwrap(...) in production, which can escape notice until there's an error, then it panics. It's better to always handle the potential error, or at least use ? to pass the error back to the caller.

  • permalink
  • fedilink
  • source
  • hideshow 5 child comments
    •
  • [–] 5 points 11 months ago* (last edited 11 months ago) (2 children)

    I've read a lot of people saying "oh what a horror, they used unwrap in production, they should NEVER do that". But I don't think that's true at all.

    1. The rust type system is not 100% flawless. For example you have a mathematically proven never-errors algorithm, but inside some steps have provisional Nones. At the end you just unwrap, since there's no point in changing the return type to an option to handle a non-existent error. You can't represent "this is None but I promise that it won't by the end" in rust's type system.
    2. Sometimes it's not worth handling the error. For example a simple CLI tool, just throw expects everywhere that shows why it failed, since most CLI tools just close anyway on failure, might as well panic.

    That being said, this was clearly a case where ? should have been used, since the function has a clear error case. And it also is a critical application that can't afford panicking as an error handling method.

    EDIT: Just to clarify, the way you would do this is by having a type Features that can error on creation/parsing which has a limit of 200 entries. And afterwards you can just use that type knowing that it will always have <= 200 entries. And you gracefully handle that one error case on creation/parsing.

    EDIT2: Another point for why I disagree with "production should never have unwrap":

    1. Unwrap is basically the same as assert(my_variable.is_some). I don't know a single person that would say "you should never have asserts in production code", the only reason I disable some assertions for release builds is for performance. Asserts are statements that should never do anything, by that logic they pose no harm by being there, so no need to remove them, their only effect should be wasting a few CPU cycles. Since asserts are only put when the programmer assures that a certain precondition will always be met. And if it is not, it's because the code changes and automatic tests should catch that.
  • permalink
  • fedilink
  • source
  • parent
  • hideshow 2 child comments
    •
  • [–] 1 point 11 months ago

    Honestly it annoys me how much the well has been poisoned with rust that we're even talking about the language here. There is so much focus on rust that we're not even talking about how they literally couldn't tell the difference between their software crashing in production and a ddos attack.

    They had no visibility into their runtime environment, and from my understanding of the Blogpost, didn't even look into the possibility until the entire cluster went down from this bad config.

    Like, even assuming they did input validation, what should the clickhouse services do when they're fed an invalid config? I'd argue the only sensible thing would be to refuse to start. But it seems like crashing wasn't being detected at all.

  • permalink
  • fedilink
  • source
  • parent
  • [–] 1 point 11 months ago

    Thank for elaborating my comment, but I never said never, only that it's usually better to avoid it.

    And yven if you think it's provably impossible to get an Error back now, someone or something may change an underlying function behaviour on you in the future and invalidate your proof. There are ways to limit that with version control and pinning and so on, but it's easy for an assumption to be overlooked when merging in new versions of things.

    So yes, I agree, better to use ? at least here, but like all guidelines, there may be times where you break it, accepting the risks.

  • permalink
  • fedilink
  • source
  • parent
  • [–] 5 points 11 months ago

    Yep. This was the difference between a silent, recoverable error and a loud failure.

    It seems like they're planning to remove all potential panics based on the end of their article. This would be a good idea considering the scale of the service's usage.

    (Also, for anyone who's not reading the article, the unwrap caused the service to crash, but wasn't the source of the issues to begin with. It was just what toppled over first.)

  • permalink
  • fedilink
  • source
  • parent