Error vs. Panic: Should functions like strings.Repeat return an error value?

[Michael Whatcott]

The blog post on error handling in go establishes the following guideline:

> In Go, error handling is important. The language's design and conventions encourage you to explicitly check for errors where they occur (as distinct from the convention in other languages of throwing exceptions and sometimes catching them).

Here's the implementation of strings.Repeat:

    // Repeat returns a new string consisting of count copies of the string s.

    func Repeat(s string, count int) string {

    b := make([]byte, len(s)*count)

    bp := copy(b, s)

    for bp < len(b) {

    copy(b[bp:], b[:bp])

    bp *= 2

    }

    return string(b)

    }

Notice that when a negative value is supplied for the count argument, panic ensues because it attempts to make a byte slice of negative length:

    package main

    

    import "strings"

    

    func main() {

    strings.Repeat("panic!", -1)

    }

Output:

    panic: runtime error: makeslice: len out of range

Of course, returning two values from strings.Repeat would make it less convenient to use, but it does reflect what can happen if the value calculated for the count argument is negative for whatever reason.

Another option would have been to use a uint for the count argument but that would introduce a different kind of inconvenience for both the caller and the implementation.

I am fully aware of the go compatibility promise and I'm not proposing a change to strings.Repeat. I can code a custom Repeat function for my own purposes. The purpose of the question is to better understand error handling in go. 

So, considering the recommendation from the go blog to return errors rather than panic, should strings.Repeat (and other such functions) always be implemented with more rigorous error handling? Or, should it always the caller's job to validate inputs that might cause a panic if otherwise unchecked?

[Ian Lance Taylor]

There are many cases in the standard library where a function will
panic on invalid input. People can disagree about what is best, but
personally I think this is OK. It's essential that functions behave
predictably on invalid input, but if the input is completely invalid a
panic is OK. It points the programmer directly at the problem. It's
not fundamentally different from the fact that a[-1] will panic.
Returning an error instead of a panic pushes error handling onto all
programs for a case that should happen in no programs.

Of course it's essential to only panic on cases that can never happen
in a valid program. I/O operations, for just one example, can fail in
many ways, and it would never be appropriate to panic on an I/O error.

Ian

[Andy Balholm]

When a function is used incorrectly, a panic is appropriate. If the count comes from an untrusted source, the caller should check that it is non-negative (and not too large) before calling Repeat, rather than checking an error afterward.

Using a uint would actually be worse, since uint(int(-1)) is an extremely large number.

Andy

[Michael Whatcott]

Ah yes, operations that can and will fail are different than those that fail because of a logic error. Thanks!

[Michael Whatcott]

Good point integer overflow. Thanks!

On Thursday, June 30, 2016 at 10:30:46 AM UTC-6, Andy Balholm wrote:

When a function is used incorrectly, a panic is appropriate. If the count comes from an untrusted source, the caller should check that it is non-negati!e (and not too large) before calling Repeat, rather than checking an error afterward.

[Anmol Sethi]

What about gzip.NewWriterLevel(w, level)? It returns an error if level is invalid.
Shouldn’t it panic instead if the level is invalid?

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Andy Balholm]

Yes, panic would probably have been a better choice there. In the API cleanups leading up to Go 1, the team removed some of the error returns from the NewWriter functions in the compress/* packages (https://codereview.appspot.com/5639057), but maybe it would have been good to go farther.

Andy

[Dave Cheney]

Why panic, the method returns an error value that can be used to tell the caller they made a mistake.

[Andy Balholm]

In this particular case, the only possible error is that the compression level is invalid. The documentation says that the error will be nil if the level is valid.

[Dave Cheney]

If this function panic'd then people who raise issues to make it not panic, or they would work around it with recover(), both of which would be in less tested code paths.

If this function accepted any integer for level and silently clamped it to a reasonable minimum or maximum value, then people would raise an issue to expose the value that was chosen.

Returning an error is the least worst choice.

[Martin Geisler]

Hi Dave

On Thu, Jul 7, 2016 at 2:08 AM, Dave Cheney <da...@cheney.net> wrote:
> If this function panic'd then people who raise issues to make it not panic,
> or they would work around it with recover(), both of which would be in less
> tested code paths.

As a newcomer to Go, it's fun to me that you call using recover a
"work around". Throwing and catching an exception is the bread and
butter of handling errors in all other languages I know :-)

Having to test for a variable being non-nil and manually returning it
up the call chain is very very clunky and yet it's seen as the
ideomatic and good way in Go. That's very strange to me, and from
reading about Go around the net, strange to a lot of people.


--
Martin Geisler

[Dave Cheney]

Hi Martin,

Go doesn't have exceptions, it really doesn't. Some people like to pretend that panic/recover are exceptions, but really they are not [1].

Thanks

Dave

1. https://golang.org/doc/faq#exceptions

[Martin Geisler]

On Thu, Jul 7, 2016 at 9:44 AM, Dave Cheney <da...@cheney.net> wrote:
> Hi Martin,
>
> Go doesn't have exceptions, it really doesn't. Some people like to pretend
> that panic/recover are exceptions, but really they are not [1].

Yes, sure, they're called something else :-) But they behavior matches
what you get with exceptions in other langauges: stack-unwinding until
you find a handler that deals with the error. True, in Go you need to
manually check (with a type switch, maybe) that you can handle the
error after you call recover() -- that's normally part of the syntax
in other languages.

> 1. https://golang.org/doc/faq#exceptions

--
Martin Geisler

[Axel Wagner]

go's panic correspond, if anything, to unchecked exceptions in Java and they should be treated as such, i.e. they should never be caught and only be thrown on a clear programmer error.

And once you restrict yourself to checked exceptions, there really isn't an advantage of a try-catch over an if err != nil.

But, in any case, this discussion has been rehashed ad infinitum.

--
Martin Geisler

[Lucio]

On Thursday, 7 July 2016 01:45:17 UTC+2, Dave Cheney wrote:

Why panic, the method returns an error value that can be used to tell the caller they made a mistake.

There's a subtle line there and Go's philosophy is not firmly on one side of it. I think the key phrase would have to be: "this error should not occur". When it does, you panic, because you have no idea what to do, no idea how to get out of there and, quite likely, no idea how the hell you got there in the first place.

Any other context, return an error code, let the invoker figure out whether SHE wants to panic.

Oh, the other condition that may justify a panic would be if corruption is detected AND there is no possible recovery to a known healthy state. All of these sound to me a lot like much closer to the hardware then Go should be allowed to go.

Lucio.