Panic on OpenBSD

[mpldr]

Hi, I'm one of the developers of aerc and we received a report from a user with a crash caused by pty.

The report is here and if questions come up, please feel free to ask in this thread. Maybe the Stack Trace will be enough to find and fix the issue though.

aerc-crash-20220405-134420.log

[kr]

Hi, thanks for filing this issue.

Based on this stack trace, it looks like you're calling pty.StartWithAttrs with a nil pointer as the first argument.

Here's the relevant line from the stacktrace:

github.com/creack/pty.StartWithAttrs(0x0, 0xc000265818, 0xc00026e380)

The 0x0 is a nil *exec.Cmd value. This function call happens on this line:

	/home/ariel/go/aerc/widgets/terminal.go:248 +0x139

I hope this helps!

[mpldr]

Yes, I am still looking into how this could happen, however, I feel like causing a crash should not be the go-to method of error handling. Instead a check whether it's nil and a corresponding error would be a better solution.

[kr]

It is appropriate to panic when client code supplies a nil pointer as an argument that's not meant to take a nil pointer. That's the usual (and, in my opinion, best) way to do it in Go.

[mpldr]

I think the "Don't Panic" Proverb sums this up pretty well. This is an error, not an issue that may lead to inconsistent state. If nil is provided that is an obvious error and should be handled gracefully instead of by blowing up.

[mpldr]

And as far as it being the usual way, I have a hard time finding such behaviour in either stdlib or 3rd party libraries.

[kr]

"Don't panic" is for runtime errors such as bad user input, out of disk, etc, not for bugs.

It's not clear if this issue leads to inconsistent state, but it does indicate inconsistent state already exists. When we reach this point in the code, we know something has happened that the program author didn't intend. By definition, the application doesn't know what its state ought to be. In other words, a bug.

Here are some examples of panic-on-bug (specifically, panic-on-nil) from the standard library:

https://go.dev/play/p/GU9WI0TvdKI
https://go.dev/play/p/amKFsFgqLzo
https://go.dev/play/p/8IRkJNxCGzo
https://go.dev/play/p/7K13by3_R9C

I picked a few functions at random, these are just the first four I tried.

[mpldr]

Interesting, thanks. Never noticed that… Thanks for showing me.

[kr]

It seems like there's nothing for this package to do here, so I'm gonna close this issue. But feel free to comment here or open a new issue if I've missed anything. Thanks again!