jawher/mow.cli

Add global panic interceptor

Opened this issue · 3 comments

oakad commented

In some apps, panics carry meaningful information and should be nicely reported on exit. User, of course, always has the option to defer a recovery block in the Action call back. However, in applications with many sub-commands and, correspondingly, Action call backs this results in excessive proliferation of the boilerplate code.

Considering that mov.cli already has some panic handling mechanics, may it be possible to add a global call back to an App object, which will receive, as its argument, a panic value propagating from whatever invoked subcommand and will have an option to either handle the panic or let it propagate further (by returning a bool value, perhaps)?

Hi @oakad.

Wouldn't the usual panic recovery technic be enough ?

defer func() {
	if r := recover(); r != nil {
		fmt.Printf("recovered: %s\n", r)
	}
}()
app.Run(os.Args)
oakad commented

The problem is with reporting.

Presently, mow.cli adds several stack frames on top of the propagating panic stack trace. This pollutes the logs and adds extra effort to troubleshooting (instead of problematic stack frame being somewhere near the top of the dump it is covered by meaningless mow.cli frames).

Consider the below example - the actual error I would like to see on top (exception.OnError) is burried below 5 frames of mow.cli stuff. If it was possible to somehow extract the object propagated by panic and its stack trace before mow.cli adds those frames the ergonomics of application troubleshooting will be considerably improved.

It may not sound like much, but those extra frames really make all the errors look the same and are very annoying.

xx/vendor/github.com/jawher/mow%2ecli.(*step).run(0xc0422024e0, 0x9dc440, 0xc042206270)
/yy/src/xx/vendor/github.com/jawher/mow.cli/flow.go:28 +0xdd
xx/vendor/github.com/jawher/mow%2ecli.(*step).run(0xc042202540, 0x9dc440, 0xc042206270)
/yy/src/xx/vendor/github.com/jawher/mow.cli/flow.go:20 +0xc5
xx/vendor/github.com/jawher/mow%2ecli.(*step).run(0xc042202630, 0x9dc440, 0xc042206270)
/yy/src/xx/vendor/github.com/jawher/mow.cli/flow.go:20 +0xc5
xx/vendor/github.com/jawher/mow%2ecli.(*step).run(0xc042202c90, 0x9dc440, 0xc042206270)
/yy/src/xx/vendor/github.com/jawher/mow.cli/flow.go:20 +0xc5
xx/vendor/github.com/jawher/mow%2ecli.(*step).callDo.func1(0xc042202cc0, 0x0, 0x0)
/yy/src/xx/vendor/github.com/jawher/mow.cli/flow.go:41 +0x53
panic(0x9dc440, 0xc042206270)
D:/Go/src/runtime/panic.go:505 +0x237
xx/loadtest/lib/exception.OnError(0xb5c360, 0xc04207a510)

I understand.

I'll need to think a bit on how to go about implementing this: If I want want to keep the original panic stack, I'll have to find another way to implement the before/after execution flow without using recover().