Panicking in public API ?

[Tay]

Hi,

Just a quick question. I know it's well accepted that panics leaking to the public API of a library is generally a no-go.

Yet, are there any exception to the rule?

For instance, I have a library that instantiates some database prepared statements (so, the majority of the elements are instantiated and used in the main function). I would like to panic instead of returning an error because, if db.Prepare(q) returns an error, there is no point in continuing, the error is barely recoverable. Besides, it will allow for a better looking API so to speak.

Any comments?

[Tamás Gulácsi]

Panic is for unrecoverable errors. If that query is provided by the user, then return an error.
If it is part of your library, and cannot be wrong, then panic.

[Dan Kortschak]

Speaking as someone who is probably to blame for a significant
proliferation of public facing panics, that example is probably not a
good place for it.

There are uses in the standard library of public facing panics, but
they generally fall into the categories of simulating the type system
(in reflect), or compiler-time known things that should never fail (in
text/template and html/template where the commonly used functions
return an error and a conversion "must" function convert a non-nil
error to a panic.

[Axel Wagner]

On Tue, Jan 7, 2020 at 8:10 AM Tay <welcometot...@gmail.com> wrote:

Hi,

Just a quick question. I know it's well accepted that panics leaking to the public API of a library is generally a no-go.

Not *that* well accepted :) I tend to disagree. But maybe I'm simply in a vanishingly small minority.

Yet, are there any exception to the rule?

The standard library offers a lot of exceptions. The reflect and the math/big packages, for example, use panics explicitly to alert to bugs. Going further, the vast majority of methods which uses pointer-receivers "leaks panics" if called on a nil-pointer, or any function which calls methods on an interface, if that's nil - which means almost all packages panic in at least *some* circumstances.

Personally, I consider panics "run-time type-errors". That is, they indicate a bug that couldn't be caught statically by the type-system - so the program shouldn't have compiled in the first place and crashing it is the right choice. Dereferencing a nil-pointer or indexing out-of-bounds fall into that category. So does using reflect to do otherwise invalid operations (which is why reflect panics). IMO, a straight-out rejection of panics doesn't make sense, unless you assume your type-system is perfect and so there are no bug-free programs.

Another way to look at it, is that a panic in general dumps a stack-trace, while an error is being presented to the human using the resulting software. And that stack-traces in general are not actionable to the user of a software, but only its developer - while error message don't contain the necessary details to debug a program, but can (should!) provide actionable advise to its user. Thus, panics are the right tool to use when reporting an issue that requires programmer attention and errors are the right tool when reporting an issue that requires user-attention (or, of course, can be handled programmatically).¹

For instance, I have a library that instantiates some database prepared statements (so, the majority of the elements are instantiated and used in the main function). I would like to panic instead of returning an error because, if db.Prepare(q) returns an error, there is no point in continuing, the error is barely recoverable. Besides, it will allow for a better looking API so to speak.

A prepared statement could fail to compile for reasons that are not bugs, as AFAIK they are compiled in the database-server. So if that's unavailable, or there is a version-incompatibility, or some resource is exhausted… statement-preparation could fail and just indicate a normal error, just like os.Open or the like. In other words, an error in preparing a statement could originate outside the program itself.

So, based on my own perspective above, I wouldn't panic in this case. Note that whether or not that error is recoverable (in your opinion) is immaterial for that. Even if the program immediately stops - if it does so with a panic and a stack-trace, a user will just throw up their hands in frustration, but if it provides an actionable error-message, they can fix that issue and retry.

Of course, the fun part about compiling a statement is that it can *also* fail due to bugs, by the way. So, the advanced version of these rules would attempt to distinguish between the two and panic e.g. on a syntax-error in a string-literal (to make it easier to debug and harder to accidentally put into production) and return an error in external error conditions. I have some code that does such checks.

Axel

[1] Corollary, of course, is "don't put stack-traces into your errors" :)

Any comments?

--
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.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/25204eae-8550-4a78-94a3-6a63e9906f20%40googlegroups.com.

[Christian Mauduit]

I'd suggest you expose both kind of APIs as in:

* https://golang.org/pkg/regexp/#Compile
* https://golang.org/pkg/regexp/#MustCompile

Implementing the `Must` flavor is trivial, just call the standard func
and panic() if you get an error. As a side effect you'll get a unique
point where to panic(), and callers will recognize it's likely yo
panic() because the function has "Must" in its name.

If in doubt, implement only the func that returns the error, the other
is just syntaxic sugar IMHO. Example:

https://golang.org/src/regexp/regexp.go?s=10768:10804#L298

panics() have been, in my Golang experience, a pain to deal with. Most
of the time they are just below a "TODO refactor that sh*t" comment, or
they would deserve that comment anyway. Proper error handling takes
time, but it's an investment that pays of.

My 2c.

Christian.

--
Christian Mauduit __/\__ ___
uf...@ufoot.org \~ ~ / (`_ \ ___
https://ufoot.org /_o _\ \ \_/ _ \_
int q = (2 * b) || !(2 * b); \/ \___/ \__)

[Brian Candler]

On Tuesday, 7 January 2020 07:47:28 UTC, Axel Wagner wrote:

Personally, I consider panics "run-time type-errors". That is, they indicate a bug that couldn't be caught statically by the type-system - so the program shouldn't have compiled in the first place and crashing it is the right choice. Dereferencing a nil-pointer or indexing out-of-bounds fall into that category. So does using reflect to do otherwise invalid operations (which is why reflect panics). IMO, a straight-out rejection of panics doesn't make sense, unless you assume your type-system is perfect and so there are no bug-free programs.

Another way to look at it, is that a panic in general dumps a stack-trace, while an error is being presented to the human using the resulting software. And that stack-traces in general are not actionable to the user of a software, but only its developer - while error message don't contain the necessary details to debug a program, but can (should!) provide actionable advise to its user. Thus, panics are the right tool to use when reporting an issue that requires programmer attention and errors are the right tool when reporting an issue that requires user-attention (or, of course, can be handled programmatically).¹

Or to put it another way: if the caller of your function violates your *API contract* then panic.  The bug is with the caller for not invoking your function properly, and therefore the calling code needs to be fixed.

By that definition: if the API contract says "you must pass in a valid SQL statement", and you can say with 100% certainty that the SQL they passed in is invalid, then by all means panic.  But if you can't  be sure of that - e.g. the problem could be a failure to connect to the database - then return an error.

[Tay]

Many thanks everyone for the insight.

Instead of accepting a raw query string and try to compile it, I will defer that part to the library user. So, I will simply accept a *sql.Stmt.

That way, I won't have to handle the various failure modes within the library and leave the issue to the end user.

Thanks again,

[Scott Pakin]

On Tuesday, January 7, 2020 at 12:47:28 AM UTC-7, Axel Wagner wrote:

Thus, panics are the right tool to use when reporting an issue that requires programmer attention and errors are the right tool when reporting an issue that requires user-attention (or, of course, can be handled programmatically).

Nicely presented guideline!

— Scott

[K.S. Bhaskar]

Those are good guidelines. I'd like to add a couple of nuances.

For “system” or “operations” errors (we have a database engine that executes in the address space of processes, so there can be errors such as an IO error or an inability to expand because of insufficient space in the file systems), neither the user nor the programmer can do much. In such cases, log the error to the syslog and return an error that the application can catch and do something with, like terminate gracefully. In such cases a stack trace or dump only uses up more storage space.

When writing out stack traces or core dumps (we do the latter under certain circumstances), make the content and location operationally configurable. Sometimes processes can contain sensitive (confidential) data. In development and test environments, you usually do want all or most of the data. When an application is promoted to production, you may – or may not – want that core dump to contain data from the process heap (for example). As you should not change the application when you promote to production, this type of configuration should be done externally, e.g, with environment variables, configuration files, etc.

Regards

– Bhaskar