When should I use Result vs panic! for invalid server requests?

help
1

I am creating a gRPC-web service using Tokio & Tonic. This is not a developer-facing API, there will only ever be one client for the service which I fully control, and that client is responsible for making valid requests to the server. There are various ways in which a request might be 'invalid', for example it could be missing a required field or violate some business logic precondition.

I'm differentiating here between an invalid request, one which by definition represents a client programming error on my part, and a normal error, such as the database being unavailable or attempting to look up a URL which doesn't exist.

In the specific case of invalid requests, is it better to rely on Result or panic!? Are there particular advantages to using Result and particular problems with using panic!?

2

Personally I would prefer to see all error situations, invalid requests etc, detected in the application and handled with Result. In the case of 'invalid' request that might mean finding an error in a Result and responding to the client with whatever error message. You don't want to panic, you want to continue gracefully. After all, invalid request is not an exceptional event, it should be regarded as a perfectly normal possibility.

Then I would reserve panics for situations you cannot get out of. Better to kill the program and start afresh than try to limp along. Panics are for those error situations you have forgotten to handle in your program or cannot handle in your program.

5 Likes
3

When should I use Result vs panic! for invalid server requests?

It's simple: you should always use Result and never panic! for errors that must be anticipated. Invalid requests can come from incorrect (or worse yet, adversarial) use of your server's HTTP endpoints. That shouldn't crash the server, since I/O and parsing are the prototypical examples of operations which are always expected to fail, and which must be handled gracefully.

10 Likes
4

You can use either. If you want to recover from such errors, if you use panic, then you cannot use the abort on panic option. I generally prefer to use panic! (and the various assert macros) for programming errors.

5

To me, an HTTP server would use catch_unwind on requests, and produce HTTP 500 on panics.

Anything else you want to return (400s, etc) should be using Results or similar, not panics.

4 Likes
6

This is a strong opinion, loosely held:

panic! means that something the program author(s) believed impossible has happened. This makes abort-on-panic reasonable - something believed to be impossible actually happened, so all assumptions are suspect - and implies that the only way to prevent the panic happening again is to revisit the program and fix the bug. In this view, catch_unwind's use case is to avoid aborting the whole program when a bad assumption is found.

Result means that an error condition can happen, and that you want something elsewhere to handle that error. It might turn into a panic! (via unwrap or expect), it might result in a HTTP 500 or HTTP 400 error to the client, depending on the error, it might trigger use of an alternative code path. At the point of returning a Result::Err, you don't care how it's handled, you just want someone else to make the decision about what's broken.

In general, reach for Result if the error is caused by input (e.g. the client sending an invalid request) or if the error is one that the caller might recover from in some way. Use panic! only if the route to panicking code should never be possible in your opinion.

So, for example:

/// Threshold below which we use the default - *must* be greater than 0
const THRESHOLD: i64 = 128;
fn foo(in: i64, default: u64) -> u64 {
    if in < THRESHOLD {
        return default;
    }
    in.try_into().unwrap()
}

is fine to panic in unwrap(), because as a programmer, I believe that it is impossible to find a value of in that is greater than or equal to 128, but does not fit in u64. If I'm wrong, or if someone later changes THRESHOLD to be negative, that's a bug that needs a recompile to fix the program.

On the other hand:

fn bar(in: i64, threshold: i64, default: u64) -> Result<u64,  TryFromIntError> {
    if in < threshold {
        return default;
    }
    in.try_into()
}

cannot use unwrap sensibly; it is possible for the supplied parameter threshold to be negative, and the next layer out should make the decision about what to do here.

4 Likes
7

This can be very emotional discussion, so let me just cite the docs:

panic! should be used when a program reaches an unrecoverable state.

5 Likes
8

I agree with this one.

One way I'll often phrase it, which I think is essentially the same idea, is as follows:

If it happens, I want you to file a bug on me.

That applies to Rust panics, to HTTP 500s, etc. It doesn't necessarily mean I'll make it start to work -- I might just change it to an HTTP 503, for example -- but I'll change it to stop going to the "this shouldn't happen" handler.

(This does assume that you have a reasonable turnaround on fixes, but you should have that anyway for "normal" software. Of course if you're doing something like fly-by-wire systems you need a completely different approach to handling things.)

5 Likes
9

Your phrasing is the same base idea, yes :smiley:

And even in fly-by-wire systems, you want a panic equivalent (I've worked on systems under similar constraints, just money at risk and not life). The necessity for panic comes from needing some way to handle the case where the world is obviously wrong (implying that the program is broken), and where the best way to recover is to throw away everything the program currently believes about the world and start again.

In the high reliability sphere, this was the sort of thing that happened because ECC failed to catch a multi-bit memory error (rare), or because a rat ate part-way through a critical cable before getting a shock and leaving it broken. Now the program's assumptions about the world are invalid (temporarily in the ECC case, more permanently in the part-eaten wire case), and the best thing we can do operationally is to reboot fast and rebuild state.

2 Likes
10

I would also add that panic!() can be useful when you are prototyping or want to hack something together that you know you'll throw away in a day or two anyway. Typically it's because you don't want to handle certain edge cases just yet and want to stub it out with unwrap() or todo!().

I'd argue this still comes under the definitions from @scottmcm and @farnz's. I "know" my sloppy code won't ever hit any errors because it's a prototype and I'll be manually feeding it good inputs anyway.

2 Likes
11

Panic is a stupid thing and it should not be even in the language unless all you do is write small terminal tools... most people who write services must make sure they never ever end up panicing... so personally i treat panic as a deprecated feature and never ever let any code that could legit panic and take my service down

12

Ah yes, who wouldn't want to write explicit error handling for expressions like x += 1, a/b, &slice[index], for i in 0..upper { ... }, and Box::new()?

(causes for panic are: integer overflow is an error, divide by zero, index out of bounds, uses += internally, allocations may fail)

1 Like
13

blame the language... i'm just arguing that any decent software bigger than simple terminal tools must never ever panic... Anyone writing a library should never panic (because they don't know where and how its used). Having it in a programming language is a mistake imo...

I did something like panic for my java services also thinking its a good idea, a special WTF (What a Terrible Failure) exception that when thrown will terminate the app... it wasn't long before i realized that its an idiotic idea

14

The question is - what should be done instead? In the five cases provided above, at least. If you don't know this, then why are you so sure that there is any better solution at all?

2 Likes
15

Its simple since rust does not support throwing exceptions like other languages you pay for it by bloating the code with tuples as return types from each function.

I try to always write pure deterministic functions where i can return some actual type guaranteed, but its unavoidable that most of your functions are going to return Results, and you will have to deal with all the dance that comes with that...

So to sum this up : always use Result and never ever use panic.

16

Note that Rust panic!s are implemented using the exact same mechanisms as C++ throws.

The societal expectations of them are different, but mechanically they're the same thing.

2 Likes
17

Not exactly, because panic!s can be changed to aborts, for example, while exceptions are meant for catching them. This is related to the societall expectations: because panics are unrecoverable exceptions (this is not only social convention; it is also documented), they can be made unrecoverable. But in fact, the fact that panics and exceptions are implemented, by default, using unwinding, is kind of implementation detail, at least for libraries.

18

Given -fno-cxx-exceptions and such, I don't know that the situation is that different.

2 Likes
19

...except that it is called no exceptions, while in Rust it's just changing panic! behavior. Many people don't want to use exceptions, so the compilers provide flags to disable them. But that's disabling a language feature; it's non standard-compliant. In Rust, however, panics can unwind, or abort with a backtrace, or abort without a backtrace and even without a message at all. The only requirement is that you call panic! on unrecoverable error. What the program does - that's not specified, although can be customized.

20

But rust doesn't have a standard, so it's a hard comparison to make. You could equally imagine a world where panics were specified to always unwind -- and given catch_unwind that's not unreasonable -- but where panic=unwind is a compiler extension.

1 Like