Should we swallow panics?

[NiloCK]

From discussion on #302

Until recently we have avoided panics in go-nitro, preferring instead to return and handle errors in all cases.

This is tedious, and the tedium both slows development and potentially adds friction to certain function calls (ie, x := makeSomething() is a lot more compact than x, err := makeSomething() + errorHandling). Go language design people are pretty explicit about this being a feature rather than a bug: error conditions should be explicitly handled rather than glossed over.

But... in some cases like:

• functions that only exist in test code
• failures which ought to be impossible

the overhead of properly bubbling and handling these errors is probably not worth the effort. The alternative to a error is panic, which itself bubbles up the call stack and halts the program if unhandled.

As things stand, there is no panic handling in go-nitro. It is considered impolite for panics to escape a package / module.

Instead an error condition in go-nitro should (probably) cause a shut-down of the go-nitro client / engine, alert the consuming application, and log this event.

Something approximating:

// swallowPanic prevents go-nitro panics from escaping to the consuming application
func (e *Engine) swallowPanic() {
	if r := recover(); r != nil {
		e.logger.Printf("Fatal error in go-nitro client: %v", r)
	}
}

// Run kicks of an infinite loop that waits for communications on the supplied channels, and handles them accordingly
func (e *Engine) Run() {
	defer e.swallowPanic()
// ...

could be a suitable panic-boundary.

[geoknee]

I think you make a pretty convincing argument.

It is considered impolite for panics to escape a package / module.

Do you have a source for this?

[NiloCK]

I knew when writing this that I was begging for a citation needed here.

From some HN thread (about an article titled Don't Panic):

[panics vs errors] was a point of huge debate early on in the development of Noms. Some team members just could not accept the constant litter of if err != nil no matter how many times I repeated that Rob Pike says it's OK. ... It’s probably why some parts of Go internally have also used panics for flow control (the JSON decoder, famously), and just convert them to errors at the boundaries. (bolding mine)

From Effective Go:

real library functions should avoid panic

I've come across similar sentiment in a few locations which I forget (SO threads, blog posts, etc). I don't recall anyone ever arguing for allowing panics to escape from library code.