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

Replace github.com/pkg/errors package #1838

Open
shahpratikr opened this issue Jan 4, 2023 · 7 comments
Open

Replace github.com/pkg/errors package #1838

shahpratikr opened this issue Jan 4, 2023 · 7 comments
Assignees
Labels
dependencies Pull requests that update a dependency file enhancement frozen good first issue

Comments

@shahpratikr
Copy link
Contributor

shahpratikr commented Jan 4, 2023

Currently, we are using the github.com/pkg/errors package to report error messages. This package is no longer maintained and the repository has been archived.
We also use the fmt.Errorf function in some places.
Use fmt.Errorf everywhere or find a suitable alternative and replace all error handling.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 4, 2023

Thanks for opening this issue 👍. The team will review it shortly.

If this is a bug report, make sure to include clear instructions how on to reproduce the problem with minimal reproducible examples, where possible. If this is a security report, please review our security policy as outlined in SECURITY.md.

If you haven't already, please take a moment to review our project's Code of Conduct document.

@denisvmedia
Copy link
Contributor

Note, I didn't study this issue deeply, but there is a list of what I think should be considered, before applying the proposed changes.
Changing to fmt.Errorf - although also supports wrapping - may have an undesired effect of changing the error message.
Also, fmt.Errorf does not include error stack.
Additionally, there's at least one usage of errors.WithStack(err).

@denisvmedia
Copy link
Contributor

Possible other options I see here:

  • find a supported fork of pkg/errors
  • create own fork of pkg/errors
  • build own reusable replacement standalone package

The most robust (but at the same time the most effort consuming) solution will be the third option.

@pavannd1 pavannd1 changed the title Use fmt.Errorf function to report all the errors Replace github.com/pkg/errors package Apr 13, 2023
@github-actions github-actions bot added the stale label Jul 13, 2023
@julio-lopez julio-lopez added dependencies Pull requests that update a dependency file frozen and removed stale labels Jul 14, 2023
@kanisterio kanisterio deleted a comment from github-actions bot Jul 14, 2023
@pavannd1 pavannd1 removed the triage label Sep 1, 2023
@e-sumin
Copy link
Contributor

e-sumin commented Sep 27, 2023

I collected some statistics about what we are using and what could be useful:

+--------------------+-----+
|        FN          |     |
+--------------------+-----+
| errors.New         | 221 |
| errors.Cause       |   7 |
| errors.Is          |   1 |
| errors.As          |   1 |
| errors.Unwrap      |   2 |
| errors.WithStack   |  21 |
| errors.WithMessage |   7 |
| errors.Wrap        | 470 |
| errors.Wrapf       | 976 |
| errors.Errorf      | 145 |
| fmt.Errorf         |  45 |
+--------------------+-----+

I've checked which error libraries are available and found two that offer rich functionality.
Here is a comparison of the features we are currently using and those we could potentially use:

+---------------------------------+------------------+----------------------+----------------------------+---------------------+
|            FEATURES             |    std errors    | github.com/pkg/errors| github.com/rotisserie/eris | emperror.dev/errors |
+---------------------------------+------------------+----------------------+----------------------------+---------------------+
| errors.New                      | Supported        | Supported            | Supported                  | Supported           |
| errors.Cause                    | Not Supported    | Supported            | Not Supported              | Supported           |
| errors.Is                       | Supported        | Supported            | Supported                  | Supported           |
| errors.As                       | Supported        | Supported            | Supported                  | Supported           |
| errors.Unwrap                   | Supported        | Supported            | Supported                  | Supported           |
| errors.WithStack                | Not Supported    | Supported            | Not Supported              | Supported           |
| errors.WithMessage              | Not Supported    | Supported            | Not Supported              | Supported           |
| errors.Wrap                     | Not Supported    | Supported            | Supported                  | Supported           |
| errors.Wrapf                    | Not Supported    | Supported            | Supported                  | Supported           |
| errors.Errorf                   | Supported        | Supported            | Supported                  | Supported           |
| fmt.Errorf                      | Supported        | Supported            | Supported                  | Supported           |
| Attach values from context      | Not Supported    | Not Supported        | Supported                  | Supported           |
| Attach map[string]interface{}   | Not Supported    | Not Supported        | Supported                  | Supported           |
| Dump values as string with Error| Not Supported    | Not Supported        | Supported                  | Supported           |
| Convert error to JSON           | Not Supported    | Not Supported        | Supported                  | Supported           |
+---------------------------------+------------------+----------------------+----------------------------+---------------------+

@e-sumin
Copy link
Contributor

e-sumin commented Sep 27, 2023

After discussing with @viveksinghggits and @pavannd1, we decided to implement our own solution based on the standard errors package.

@mitar
Copy link

mitar commented Oct 8, 2023

Shameless plug: you can also switch it to drop-in replacement gitlab.com/tozd/go/errors. It fixes many issues, is maintained, and supports modern Go's error patterns (sentinel errors, %w formatting, joined errors, etc.). It also provides some nice utility functions and structured details so that it is easy to extract dynamic data out of errors (instead of trying to get them out of formatted strings). Has improved error formatting and JSON marshaling of errors. It is interoperable with other errors packages and does not require a dependency on it to extract data (e.g., stack trace) from errors.

@krishnaduttp
Copy link

I collected some statistics about what we are using and what could be useful:

+--------------------+-----+
|        FN          |     |
+--------------------+-----+
| errors.New         | 221 |
| errors.Cause       |   7 |
| errors.Is          |   1 |
| errors.As          |   1 |
| errors.Unwrap      |   2 |
| errors.WithStack   |  21 |
| errors.WithMessage |   7 |
| errors.Wrap        | 470 |
| errors.Wrapf       | 976 |
| errors.Errorf      | 145 |
| fmt.Errorf         |  45 |
+--------------------+-----+

I've checked which error libraries are available and found two that offer rich functionality. Here is a comparison of the features we are currently using and those we could potentially use:

+---------------------------------+------------------+----------------------+----------------------------+---------------------+
|            FEATURES             |    std errors    | github.com/pkg/errors| github.com/rotisserie/eris | emperror.dev/errors |
+---------------------------------+------------------+----------------------+----------------------------+---------------------+
| errors.New                      | Supported        | Supported            | Supported                  | Supported           |
| errors.Cause                    | Not Supported    | Supported            | Not Supported              | Supported           |
| errors.Is                       | Supported        | Supported            | Supported                  | Supported           |
| errors.As                       | Supported        | Supported            | Supported                  | Supported           |
| errors.Unwrap                   | Supported        | Supported            | Supported                  | Supported           |
| errors.WithStack                | Not Supported    | Supported            | Not Supported              | Supported           |
| errors.WithMessage              | Not Supported    | Supported            | Not Supported              | Supported           |
| errors.Wrap                     | Not Supported    | Supported            | Supported                  | Supported           |
| errors.Wrapf                    | Not Supported    | Supported            | Supported                  | Supported           |
| errors.Errorf                   | Supported        | Supported            | Supported                  | Supported           |
| fmt.Errorf                      | Supported        | Supported            | Supported                  | Supported           |
| Attach values from context      | Not Supported    | Not Supported        | Supported                  | Supported           |
| Attach map[string]interface{}   | Not Supported    | Not Supported        | Supported                  | Supported           |
| Dump values as string with Error| Not Supported    | Not Supported        | Supported                  | Supported           |
| Convert error to JSON           | Not Supported    | Not Supported        | Supported                  | Supported           |
+---------------------------------+------------------+----------------------+----------------------------+---------------------+

@e-sumin which tool have you used to get this clean evaluation ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement frozen good first issue
Projects
None yet
Development

No branches or pull requests

7 participants