5 ms·

I wish more people (and crate authors) would treat panic!() as it really should be treated: only for absolutely unrecoverable errors that indicate that some sor

by kelnos 2y ago

I wish more people (and crate authors) would treat panic!() as it really should be treated: only for absolutely unrecoverable errors that indicate that some sort of state is corrupted and that continuing wouldn't be safe from a data- or program-integrity perspective. Even then, though, I do see a need to catch panics in some situations: if I'm writing some sort of API or web service, and there's some inconsistency in a particular request (even if it's because of a bug I've written), I probably really would prefer only that request to abort, not for the entire process to be torn down, terminating any other in-flight requests that might be just fine. But otherwise, you really should just not be catching panics at all.

tcfhgj 2y ago
would you consider panics acceptable when you think it cannot panic in practice? e.g. unwraping/expecting a value for a key in a map when you inserted that value before and know it hasn't been removed? you could have a panic though, if you wrongly make assumptions
- pdimitar 2y ago
  I don't speak for anyone else but I'm not using `unwrap` and `expect`. I understand the scenario you outlined but I've accepted it as a compromise and will `match` on a map's fetching function and will have an `Err` branch. I will fight against program aborts as hard as I possibly can. I don't mind boilerplate to be the price paid and will provide detailed error messages even in such obscure error branches. Again, speaking only for myself. My philosophy is: the program is no good for me dead.
  - von_lohengramm 2y ago
    > the program is no good for me dead That may be true, but the program may actually be bad for you if it does something unexpected due to an unforeseen state.
    - pdimitar 2y ago
      Agreed, that's why I don't catch panics either -- if we get to that point I'm viewing the program as corrupted. I'm simply saying that I do my utmost to never use potentially panicking Rust API and prefer to add boilerplate for `Err` branching.
  - int_19h 2y ago
    So what do you do in the error branch if something like out-of-bounds index happens? Wrap and propagate the error to the caller?
    - pdimitar 2y ago
      Usually yes. But I lean much more to writing library-like code, I admit. When I have to make a decision on an app-level, it becomes a different game though. I don't have a clear-and-cut answer for that.
      - int_19h 2y ago
        This implies that every function in your library that ever has to do anything that might error out - e.g. integer arithmetic or array indexing - has to be declared as returning the corresponding Result to propagate the error. Which means that you are now imposing this requirement (to check for internal logic bugs in library code) onto the user of your library.
        pdimitar 2y ago
        Well, I don't write as huge a code as this though, nor does it have as many layers. Usually I just use the `?` and `.map_err` (or `anyhow` / `thiserror`) to delegate and move on with life. I have a few places where I do pattern-matches to avoid exactly what you described: imposing the extra internal complexity to users. Which is indeed a bad thing and I am trying to fight it. Not always succeeding.
- conradludgate 2y ago
  Not the same person, but I first try and figure out an API that allows me to not panic in the first place. Panics are a runtime memory safe way to encode an invariant, but I will generally prefer a compile time invariant if possible and not too cumbersome. However, yes I will panic if I'm not already using unsafe and I can clearly prove the invariant I'm working with.
- nextaccountic 2y ago
  Obviously yes. For the same reason it's acceptable that myvec[i] panics (it will panic if i is out of bounds - but you already figured out that i is in bounds) and a / b panic for a and b integers (it will panic if b is zero, but if your code is not buggy you already tested if b is zero prior to dividing right?) Panic is absolutely fine for bugs, and it's indeed what should happen when code is buggy. That's because buggy code can make absolutely no guarantees on whether it is okay to continue (arbitrary data structures may be corrupted for instance) Indeed it's hard to "treat an error" when the error means code is buggy. Because you can rarely do anything meaningful about that. This is of course a problem for code that can't be interrupted.. which include the Linux kernel (they note the bug, but continue anyway) and embedded systems. Note that if panic=unwind you have the opportunity to catch the panic. This is usually done by systems that process multiple unrelated requests in the same program: in this case it's okay if only one such request will be aborted (in HTTP, it would return a 5xx error), provided you manually verify that no data structure shared by requests would possibly get corrupted. If you do one thread per request, Rust does this automatically; if you have a smaller threadpool with an async runtime, then the runtime need to catch panics for this to work.
  - monkeyelite 2y ago
    > Note that if panic=unwind you have the opportunity to catch the panic. And now your language has exceptions - which break control flow and make reasoning about a program very difficult - and hard to optimize for a compiler.
    - nextaccountic 2y ago
      Yeah, but this isn't the only bad thing about unwinding. Much worse than just catching panics is the fact that a panic in a thread takes down only that thread (except if it is in the main thread). If your program is multithreaded, panic=unwind makes it much harder to understand how it reacts to errors, unless you take measures to shut down the program if any thread panic (which again, requires catch_unwind if you have unwinding). Also: that's why locks in Rust have poisoning, they exist so that panics propagate between threads: if a thread panics while holding a lock, any other thread attempting to acquire this lock will panic too (which is better than a deadlock for sure) And that's why my programs get compiled with panic=abort, that makes panics just quit the program, with no ability to catch them, and no programs in zombie states where some threads panicked and others keep going on. But see, catch_panic is an escape hatch. It's not meant to be used as a general error handling mechanism and even when doing FFI, Rust code typically converts exceptions in other languages into Results (at a performance cost, but who cares). But Rust needs a escape right, it is a low level language. And there is at least one case where the catch_unwind is fully warranted: when you have an async web server with multiple concurrent requests and you need panics to take down only a single request, and not the whole server (that would be a DoS vector). If that weren't possible, then async Rust couldn't have feature parity with sync Rust (which uses a thread-per-request model, and where panics kill the thread corresponding to the request)
deleted 2y ago
[deleted]
wyager 2y ago
> only for absolutely unrecoverable errors Unfortunately even the Rust core language doesn't treat them this way. I think it's arguably the single biggest design mistake in the Rust language. It prevents a ton of useful stuff like temporarily moving out of mutable references. They've done a shockingly good job with the language overall, but this is definitely a wart.
willtemperley 2y ago
Using a Rust lib from Swift on macOS I definitely want to catch panics - to access security scoped resources in Rust I need the Rust code to execute in process (I believe) but I’d also like it not to crash the entire app.
monkeyelite 2y ago
> I probably really would prefer only that request to abort, not for the entire process to be torn down, This is a sign you are writing an operating system instead of using one. Your web server should be handling requests from a pool of processes - so that you get real memory isolation and can crash when there is a problem.
- tsimionescu 2y ago
  Even if you used a pool of processes, that's still not one process per request, and you still don't want one request crashing to tear down unrelated requests.
  - monkeyelite 2y ago
    I question both things. I would first of all handle each request in its own process. If there was a special case that would not work, then the design dictates that requests are not independent and there must be risk of interference (they are in the same process!) What I definitely do not want is a bug ridden “crashable async sub task” system built in my web program.
    - tsimionescu 2y ago
      This is simply a wrong idea about how to write web servers. You're giving up scalability massively, only to gain a minor amount of safety - one that is virtually irrelevant in a memory safe language, which you should anyway use. The overhead of process-per-request, or even thread-per-request, is absurd if you're already using a memory safe language.
      - monkeyelite 2y ago
        > You're giving up scalability massively you’re vastly over estimating the overhead of processes and number of simultaneous web connections. > only to gain a minor amount of safety What you’re telling me is performance (memory?) is such a high priority you’re willing to make correctness and security tradeoffs. And I’m saying thats ok, one of those is crashing might bring down more than one request. > one that is virtually irrelevant in a memory safe language Your memory safe language uses C libraries in its process. Memory safe languages have bugs all the time. The attack surface is every line of your program and runtime. Memory is only one kind of resource and privilege. Process isolation is key for managing resource access - for example file descriptors. Chrome is a case study if these principles. Everybody thought isolating JS and HTML pages should be easy - nobody could get it right and chrome instead wrapped each page in a process.
selfmodruntime 2y ago
Honestly, I don't think libraries should ever panic. Just return an UnspecifiedError with some sort of string. I work daily with rust, but I wish no_std and an arbitrary no_panic would have better support.
- burntsushi 2y ago
  Example docs for `foo() -> Result<(), UnspecifiedError>`: # Errors `foo` returns an error called `UnspecifiedError`, but this only happens when an anticipated bug in the implementation occurs. Since there are no known such bugs, this API never returns an error. If an error is ever returned, then that is proof that there is a bug in the implementation. This error should be rendered differently to end users to make it clear they've hit a bug and not just a normal error condition. Imagine if I designed `regex`'s API like this. What a shit show that would be. If you want a less flippant take down of this idea and a more complete description of my position, please see: https://burntsushi.net/unwrap/ https://burntsushi.net/unwrap/ > Honestly, I don't think libraries should ever panic. Just return an UnspecifiedError with some sort of string. The latter is not a solution to the former. The latter is a solution to libraries having panicking branches. But panics or other logically incorrect behavior can still occur as a result of bugs.
  - stouset 2y ago
    My main issue with panics is poor interop across FFI boundaries.
    - burntsushi 2y ago
      This is like saying, "my main issue with bugs is that they result in undesirable behavior." Panicking should always be treated as a bug. They are assertions.
  - 0x457 2y ago
    Funny that as a user of this library, I would just unwrap this, and it results in the same outcome as if library panicked.

by kelnos 2y ago

tcfhgj 2y ago

would you consider panics acceptable when you think it cannot panic in practice? e.g. unwraping/expecting a value for a key in a map when you inserted that value before and know it hasn't been removed? you could have a panic though, if you wrongly make assumptions

pdimitar 2y ago
I don't speak for anyone else but I'm not using `unwrap` and `expect`. I understand the scenario you outlined but I've accepted it as a compromise and will `match` on a map's fetching function and will have an `Err` branch. I will fight against program aborts as hard as I possibly can. I don't mind boilerplate to be the price paid and will provide detailed error messages even in such obscure error branches. Again, speaking only for myself. My philosophy is: the program is no good for me dead.
- von_lohengramm 2y ago
  > the program is no good for me dead That may be true, but the program may actually be bad for you if it does something unexpected due to an unforeseen state.
  - pdimitar 2y ago
    Agreed, that's why I don't catch panics either -- if we get to that point I'm viewing the program as corrupted. I'm simply saying that I do my utmost to never use potentially panicking Rust API and prefer to add boilerplate for `Err` branching.
- int_19h 2y ago
  So what do you do in the error branch if something like out-of-bounds index happens? Wrap and propagate the error to the caller?
  - pdimitar 2y ago
    Usually yes. But I lean much more to writing library-like code, I admit. When I have to make a decision on an app-level, it becomes a different game though. I don't have a clear-and-cut answer for that.
    - int_19h 2y ago
      This implies that every function in your library that ever has to do anything that might error out - e.g. integer arithmetic or array indexing - has to be declared as returning the corresponding Result to propagate the error. Which means that you are now imposing this requirement (to check for internal logic bugs in library code) onto the user of your library.
      - pdimitar 2y ago
        Well, I don't write as huge a code as this though, nor does it have as many layers. Usually I just use the `?` and `.map_err` (or `anyhow` / `thiserror`) to delegate and move on with life. I have a few places where I do pattern-matches to avoid exactly what you described: imposing the extra internal complexity to users. Which is indeed a bad thing and I am trying to fight it. Not always succeeding.
conradludgate 2y ago
Not the same person, but I first try and figure out an API that allows me to not panic in the first place. Panics are a runtime memory safe way to encode an invariant, but I will generally prefer a compile time invariant if possible and not too cumbersome. However, yes I will panic if I'm not already using unsafe and I can clearly prove the invariant I'm working with.
nextaccountic 2y ago
Obviously yes. For the same reason it's acceptable that myvec[i] panics (it will panic if i is out of bounds - but you already figured out that i is in bounds) and a / b panic for a and b integers (it will panic if b is zero, but if your code is not buggy you already tested if b is zero prior to dividing right?) Panic is absolutely fine for bugs, and it's indeed what should happen when code is buggy. That's because buggy code can make absolutely no guarantees on whether it is okay to continue (arbitrary data structures may be corrupted for instance) Indeed it's hard to "treat an error" when the error means code is buggy. Because you can rarely do anything meaningful about that. This is of course a problem for code that can't be interrupted.. which include the Linux kernel (they note the bug, but continue anyway) and embedded systems. Note that if panic=unwind you have the opportunity to catch the panic. This is usually done by systems that process multiple unrelated requests in the same program: in this case it's okay if only one such request will be aborted (in HTTP, it would return a 5xx error), provided you manually verify that no data structure shared by requests would possibly get corrupted. If you do one thread per request, Rust does this automatically; if you have a smaller threadpool with an async runtime, then the runtime need to catch panics for this to work.
- monkeyelite 2y ago
  > Note that if panic=unwind you have the opportunity to catch the panic. And now your language has exceptions - which break control flow and make reasoning about a program very difficult - and hard to optimize for a compiler.
  - nextaccountic 2y ago
    Yeah, but this isn't the only bad thing about unwinding. Much worse than just catching panics is the fact that a panic in a thread takes down only that thread (except if it is in the main thread). If your program is multithreaded, panic=unwind makes it much harder to understand how it reacts to errors, unless you take measures to shut down the program if any thread panic (which again, requires catch_unwind if you have unwinding). Also: that's why locks in Rust have poisoning, they exist so that panics propagate between threads: if a thread panics while holding a lock, any other thread attempting to acquire this lock will panic too (which is better than a deadlock for sure) And that's why my programs get compiled with panic=abort, that makes panics just quit the program, with no ability to catch them, and no programs in zombie states where some threads panicked and others keep going on. But see, catch_panic is an escape hatch. It's not meant to be used as a general error handling mechanism and even when doing FFI, Rust code typically converts exceptions in other languages into Results (at a performance cost, but who cares). But Rust needs a escape right, it is a low level language. And there is at least one case where the catch_unwind is fully warranted: when you have an async web server with multiple concurrent requests and you need panics to take down only a single request, and not the whole server (that would be a DoS vector). If that weren't possible, then async Rust couldn't have feature parity with sync Rust (which uses a thread-per-request model, and where panics kill the thread corresponding to the request)

deleted 2y ago

[deleted]

wyager 2y ago

> only for absolutely unrecoverable errors Unfortunately even the Rust core language doesn't treat them this way. I think it's arguably the single biggest design mistake in the Rust language. It prevents a ton of useful stuff like temporarily moving out of mutable references. They've done a shockingly good job with the language overall, but this is definitely a wart.

willtemperley 2y ago

Using a Rust lib from Swift on macOS I definitely want to catch panics - to access security scoped resources in Rust I need the Rust code to execute in process (I believe) but I’d also like it not to crash the entire app.

monkeyelite 2y ago

> I probably really would prefer only that request to abort, not for the entire process to be torn down, This is a sign you are writing an operating system instead of using one. Your web server should be handling requests from a pool of processes - so that you get real memory isolation and can crash when there is a problem.

tsimionescu 2y ago
Even if you used a pool of processes, that's still not one process per request, and you still don't want one request crashing to tear down unrelated requests.
- monkeyelite 2y ago
  I question both things. I would first of all handle each request in its own process. If there was a special case that would not work, then the design dictates that requests are not independent and there must be risk of interference (they are in the same process!) What I definitely do not want is a bug ridden “crashable async sub task” system built in my web program.
  - tsimionescu 2y ago
    This is simply a wrong idea about how to write web servers. You're giving up scalability massively, only to gain a minor amount of safety - one that is virtually irrelevant in a memory safe language, which you should anyway use. The overhead of process-per-request, or even thread-per-request, is absurd if you're already using a memory safe language.
    - monkeyelite 2y ago
      > You're giving up scalability massively you’re vastly over estimating the overhead of processes and number of simultaneous web connections. > only to gain a minor amount of safety What you’re telling me is performance (memory?) is such a high priority you’re willing to make correctness and security tradeoffs. And I’m saying thats ok, one of those is crashing might bring down more than one request. > one that is virtually irrelevant in a memory safe language Your memory safe language uses C libraries in its process. Memory safe languages have bugs all the time. The attack surface is every line of your program and runtime. Memory is only one kind of resource and privilege. Process isolation is key for managing resource access - for example file descriptors. Chrome is a case study if these principles. Everybody thought isolating JS and HTML pages should be easy - nobody could get it right and chrome instead wrapped each page in a process.

selfmodruntime 2y ago

Honestly, I don't think libraries should ever panic. Just return an UnspecifiedError with some sort of string. I work daily with rust, but I wish no_std and an arbitrary no_panic would have better support.

burntsushi 2y ago
Example docs for `foo() -> Result<(), UnspecifiedError>`: # Errors `foo` returns an error called `UnspecifiedError`, but this only happens when an anticipated bug in the implementation occurs. Since there are no known such bugs, this API never returns an error. If an error is ever returned, then that is proof that there is a bug in the implementation. This error should be rendered differently to end users to make it clear they've hit a bug and not just a normal error condition. Imagine if I designed `regex`'s API like this. What a shit show that would be. If you want a less flippant take down of this idea and a more complete description of my position, please see: https://burntsushi.net/unwrap/ https://burntsushi.net/unwrap/ > Honestly, I don't think libraries should ever panic. Just return an UnspecifiedError with some sort of string. The latter is not a solution to the former. The latter is a solution to libraries having panicking branches. But panics or other logically incorrect behavior can still occur as a result of bugs.
- stouset 2y ago
  My main issue with panics is poor interop across FFI boundaries.
  - burntsushi 2y ago
    This is like saying, "my main issue with bugs is that they result in undesirable behavior." Panicking should always be treated as a bug. They are assertions.
- 0x457 2y ago
  Funny that as a user of this library, I would just unwrap this, and it results in the same outcome as if library panicked.