Ensure that all panics are handled

[jaekwon]

Right before launch software finalization

[ValarDragon]

I thought we wanted to keep panics, so that things fail fast, and errors don't propagate. (As we'd prefer there not to be an unknown invariant broken somewhere)

[ValarDragon]

Or should we have invariant failing panics create a circuit breaker message?

[ebuchman]

We should have a discussion about this and document the actual strategy. It seems sometimes we're insistent about panics as part of an API (eg. nil keys in the KVStore) while other time's its not clear.

But there's also panics in many other parts of the code that might not be as well justified. Also we still panic in lots of places in the CLI when we find eg. empty accounts.

It would be very cool if we could catch panics and trigger the governance circuit breaker ...

[alexanderbez]

I'd definitely would like to be part of that convo/design discussion. Panics on low-level ops are certainly warranted, but operations closer to user-space (e.g. txs that lead to an account not being found) and those that have the potential to be handled gracefully should probably have their respective errors bubbled upstream.

[cwgoes]

It would be very cool if we could catch panics and trigger the governance circuit breaker ...

For panics in the state machine, I think this is a very interesting idea, particularly in combination with something like #1381.

[mossid]

Maybe we can define type PanicHandler which can be used for handling panic messages. When any module makes panic, it goes to recover() in runTx, and passed to registered PanicHandler(in this case gov.NewPanicHandler()) if it is not core panic type(#1382).

[ValarDragon]

Since we've punted the circuit breaker to postlaunch (AFAIK), should we keep all the panics prelaunch, so that the chain halts quickly?

[cwgoes]

Since we've punted the circuit breaker to postlaunch (AFAIK), should we keep all the panics prelaunch, so that the chain halts quickly?

I think so. It will be far, far easier to debug a problem if we halt quickly and figure out the cause rather than continue running the state machine and operate over a potentially more and more invalid state.

[jackzampolin]

Sounds like based on the discussion above we are going to punt this #postlaunch. Adjusting labels accordingly.

[jackzampolin]

Going to close this issue as we are planning to keep panics in the state machine.