Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: replace xerrors with standard Go error handling #108

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

basgys
Copy link

@basgys basgys commented Nov 7, 2024

What changed? Why?

Removed the dependency on golang.org/x/xerrors throughout the codebase and refactored error handling to use the standard fmt.Errorf function with the %w verb for error wrapping, in combination with Go's built-in errors package for unwrapping and type assertion.

There were 3 errors incorrectly formatted. Now that standard packages are used, go vet can detect them easily.

Why xerrors is no longer needed

Since Go 1.13, the language has supported native error wrapping with fmt.Errorf("...: %w", err), and since Go 1.20, the built-in error handling tools are mature enough to fully replace xerrors. The key improvements include:

  • Native error wrapping: fmt.Errorf with %w wraps errors effectively, allowing use of errors.Is and errors.As to inspect wrapped errors, matching the functionality xerrors previously offered.
  • Improved error formatting: Go 1.20 enhances error formatting, providing multi-line stack traces with %+v when needed, eliminating the need for xerrors' custom formatting.
  • Simplified dependencies: Removing xerrors reduces external dependencies, improving maintainability and compatibility with future Go updates.
  • Tooling: Benefit from all the standard tooling (e.g. lint)

How did you test the change?

  • unit test
  • integration test
  • functional test
  • adhoc test (described below)

Removed the dependency on `golang.org/x/xerrors` throughout the codebase and refactored error handling to use the standard `fmt.Errorf` function  with the `%w` verb for error wrapping, in combination with Go's built-in  `errors` package for unwrapping and type assertion.

### Why xerrors is no longer needed

Since Go 1.13, the language has supported native error wrapping with
`fmt.Errorf("...: %w", err)`, and since Go 1.20, the built-in error handling tools are mature enough to fully replace `xerrors`. The key improvements include:

- **Native error wrapping**: `fmt.Errorf` with `%w` wraps errors effectively, allowing use of `errors.Is` and `errors.As` to inspect wrapped errors, matching the functionality `xerrors` previously offered.
- **Improved error formatting**: Go 1.20 enhances error formatting, providing multi-line stack traces with `%+v` when needed, which eliminates the need  for `xerrors`' custom formatting.
- **Simplified dependencies**: Removing `xerrors` reduces external dependencies, improving maintainability and compatibility with future Go updates.
Now that the standard packages are used to wrap errors, linters can detect formatting errors. This commit fixes all errors detected by `go vet`.
@basgys basgys requested a review from a team as a code owner November 7, 2024 10:59
@cb-heimdall
Copy link
Collaborator

cb-heimdall commented Nov 7, 2024

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@basgys
Copy link
Author

basgys commented Nov 7, 2024

Errors detected before this patch:

go vet -printfuncs=wrapf,statusf,warnf,infof,debugf,failf,equalf,containsf,fprintf,sprintf ./...
# github.com/coinbase/chainstorage/internal/blockchain/client/rosetta
# [github.com/coinbase/chainstorage/internal/blockchain/client/rosetta]
internal/blockchain/client/rosetta/rosetta.go:508:15: fmt.Errorf format %w has arg request of wrong type github.com/coinbase/rosetta-sdk-go/types.BlockRequest
# github.com/coinbase/chainstorage/internal/blockchain/parser/aptos
# [github.com/coinbase/chainstorage/internal/blockchain/parser/aptos]
internal/blockchain/parser/aptos/aptos_native.go:655:16: fmt.Errorf format %w has arg transactionInfo.Type of wrong type string
internal/blockchain/parser/aptos/aptos_native.go:900:16: fmt.Errorf format %w has arg wcType.Type of wrong type string

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants