penumbra-zone / penumbra

Penumbra is a fully private proof-of-stake network and decentralized exchange for the Cosmos ecosystem.
https://penumbra.zone
Apache License 2.0
363 stars 288 forks source link

Use error codes in error messages that will reach the frontend #3961

Open jessepinho opened 4 months ago

jessepinho commented 4 months ago

Is your feature request related to a problem? Please describe. There are a number of errors that can occur in core that get surfaced to the minifront during failed RPC calls or when broadcasting a transaction.

For example:

By the time these errors reach the frontend code (via Tendermint logs), it has text that looks like this: ConnectError: [unknown] Tendermint: failed to deliver transaction: check_stateful failed: undelegation was prepared for next epoch 16 but the next epoch is 23 We obviously don't want to show this text to the user, but we also don't want to parse the text into something human-readable.

At the moment, the way we deal with this error on the frontend is to maintain an array of known errors, which is brittle and will break if anyone on the core team changes the text of an error. And then we show the full text of the error to the user.

Describe the solution you'd like I'm proposing that we create a standardized set of error codes for all the different error cases. They could be numerical (penumbra(1234)) or strings (UNDELEGATION_PREPARED_FOR_WRONG_EPOCH) or whatever other style people prefer, as long as they're consistent. They could be included in the existing error messages in a standardized way — e.g., [UNDELEGATION_PREPARED_FOR_WRONG_EPOCH] undelegation was prepared for next epoch {} but the next epoch is {}. This way, on the frontend, we could just look for the machine-readable error code, and then decide how to display it to the user.

hdevalence commented 4 months ago

The CheckTx method actually provides a numeric error code.

However, we don't use it, because we don't explicitly enumerate our error types. This would be good to do, but unfortunately it will have to be post-mainnet, because it will require significant internal refactoring to explicitly enumerate all of the possible transaction errors.

In the meantime, frontend code should just show the raw error message; this is suboptimal from a UX perspective but it's less important than getting functionality fleshed out.

jessepinho commented 4 months ago

OK, got it, thanks. I'll leave this issue here to return to after mainnet.