Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

>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.



> 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.


Not if everyone decides to 's/.unwrap()/.expect("Shouldn't happen")/g' in the code base.

Or the good old:

    let x = match res { 
       Ok(x) => x,
       Err(_) => unreachable!(),
    }


If the `.unwrap()` was replaced with

1:

  ?
2:

  map_err, or, or_else, etc.
3:

  match ... { 
    Ok(..) => {}, 
    Err(..) => {},
  }
4:

  if let ... {
  } 
Then it would have been idiomatic Rust code and wouldn't have failed at all.

The function signature returned a `Result<(), (ErrorFlags, i32)>`

Seems like it should have returned an Err((ErrorFlags, i32)) here. Case 2 or 3 above would have done nicely.

Removing unwrap() from Rust would have forced the proper handling of the function call and would have prevented this.

Unwrap() is Rust's original sin.


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.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: