sourcenetwork / defradb

DefraDB is a Peer-to-Peer Edge Database. It's the core data storage system for the Source Network Ecosystem, built with IPLD, LibP2P, CRDTs, and Semantic open web properties.
445 stars 43 forks source link

[EPIC] Handling errors #488

Closed fredcarle closed 4 months ago

fredcarle commented 2 years ago

The current approach to handling errors in Defra is lacking in consistency and as such creates a situation where logging and API responses cannot expect a standardized error format.

In addition, it's currently not possible by simply using the stdlib errors package to generate a stack trace of where the error was generated. This led to github.com/pkg/errors being introduced in some parts of the code base (api/http). pkg/errors is currently in maintenance mode until the release of Go 2.0 so @sourcenetwork/database-team prefers to avoid using it.

Go 1.13 changed the way the stdlib works with errors but it's not yet at the level it should be: https://blog.dharnitski.com/2019/09/09/go-errors-are-not-pkg-errors/

It does however expose interfaces that we can use to implement a Defra specific error package. https://go.dev/blog/go1.13-errors

The solution, then, is to create a defradb/errors package that will allow us to create consistent error handling patterns across the code base. The new defradb/errors package should define and implement relevant interfaces and each Defra package could define error types specific to its context (if needed).

We can refer to https://github.com/upspin/upspin for ideas.

Other resources:

Child issues:

### Tasks
 - [x] https://github.com/sourcenetwork/defradb/issues/545
 - [x] https://github.com/sourcenetwork/defradb/issues/750
 - [x] https://github.com/sourcenetwork/defradb/issues/751
 - [ ] #257
orpheuslummis commented 2 years ago

As Shahzad mentionned, might want to activate wrapcheck linter as part of this

orpheuslummis commented 2 years ago

A popular Go linter, staticcheck, recommends a particular way of formatting errors:

(related issue #161)

The reasoning being that the reporting of wrapped errors chain them together and it's more readable when they don't begin with uppercase or ends with punctuation.

orpheuslummis commented 2 years ago

Some errors should be made available to the defra-consumer. As an example: client.DB.AddSchema returns errors.New("Collection already exists") via db.db.CreateCollection and it should be available to use in a errors.Is() call.

AndrewSisley commented 4 months ago

Closing, child items have been completed