[WIP] [Core] Propose minimal error handling methodology

[bryanchriswhite]

Objective

Consistent and ubiquitous error handling methodology. Concerns:

• Testing
  • Asserting specific error cases using high-level APIs (i.e. not using regular expressions to match error messages, etc.)
• Logging
  • Debugging
    • Convenient access to stack traces (e.g. print stack trace of errors on DEBUG level)
• Errors sent over the network
  • Can't leak metadata (e.g. peer's shouldn't see a detailed DB error from one another)

Origin Document

Goals

• Improve accuracy and maintainability of error case test code.
• Improve developer experience while debugging.
• Improve readability and consistency in error handling logic across the codebase.
• Ensure consistency in error presentation.
• Ensure errors sent over the network can't leak metadata.

Deliverable

• Proposal for a minimal, coherent error framework which addresses the outlined concerns
  • Defines "proper presentation" for:
    • logging
    • end-user (i.e. normal operation, not debugging)
    • debugging
• Documentation of a minimal, example application of the methodology which covers each concern (see "Testing Methodology", below).

Non-goals / Non-deliverables

• Changing existing error handling code.

Testing Methodology

1. Write an simple, example test which asserts for a specific error.
2. Ensure proper presentation when running tests.
3. Write a simple, example script which generates and logs the error but then exits normally.
4. Ensure proper presentation when logging.
5. Update the script to make the error fatal.
6. Ensure proper presentation while debugging.

---

Creator: @bryanchriswhite

[Olshansk]

Adding this discussion for reference:

[Olshansk]

From @h5law

Not sure if this is something ppl are doing or a convention other places but can we decide when not using an error in shared/core/types/errors.go but with errors.New or fmt.Errorf that we prefix the error with a word like unable, failed, or error such as:

• logger.Error().Err(err).Msg("failed to get ibc module") instead of logger.Error().Err(err).Msg("retrieving ibc module")
• logger.Error().Err().Msg("error creating new client") instead of logger.Error().Err(err).Msg("creating new client")

This just makes the logs easier to scan. I recognise these log lines will have an error field but when reading with my eyes I think its easier if the error is extra clear to see when something actually went wrong, as the logs are not the most human readable.

Source:

For anyone reading this, I personally also like using iTerm's triggers to highlight specific words using colors.