github.com/pkg/errors is archived

[pacoxu]

• Replace github.com/pkg/errors with stdlib errors go-winio#227 removed it in go-winio
• in this projects, we are using github.com/pkg/errors in many places

follow up of kubernetes/kubernetes#114912 and see kubernetes/kubernetes#113627 for some details.

[kolyshkin]

There are 419 cases of using errors.Wrapf which should be converted to fmt.Errorf in the following manner:

-               return errors.Wrapf(errdefs.ErrNotFound, "exec: '%s' in task: '%s' not found", wpse.tid, wpse.tid)
+               return fmt.Errorf("exec: %q in task: %q: %w", wpse.tid, wpse.tid, errdefs.ErrNotFound)

        if sid != req.ID {
-               return nil, errors.Wrapf(
-                       errdefs.ErrFailedPrecondition,
-                       "expected annotation '%s': '%s' got '%s'",
+               return nil, fmt.Errorf(
+                       "expected annotation %q: %q, got %q: %w",
                        annotations.KubernetesSandboxID,
                        req.ID,
-                       sid)
+                       sid,
+                       errdefs.ErrFailedPrecondition)
        }

(I also took a liberty to add a missing comma, and change %s to %q which will add double quotes around the string and also escape non-printable characters which usually helps in debugging.

[kolyshkin]

@katiewasnothere @kevpar @helsaawy PTAL 🙏🏻

[kolyshkin]

Also, in some cases when an error being wrapped already contains the text we want, it's better to take that into account to avoid stuttering, e.g.:

-               return nil, errors.Wrapf(errdefs.ErrAlreadyExists, "task with id: '%s' already exists", req.ID)
+               return nil, fmt.Errorf("task with id: %q: %w", req.ID, errdefs.ErrAlreadyExists)

[helsaawy]

unfortunately a simple replace won't work, since we rely on the stack traces that "github.com/pkg/errors" adds to the errors in certain cases, so we would need to reimplement the functionality:

hcsshim/internal/guest/gcserr/errors.go

Lines 69 to 90 in 1d69a9c

	func BaseStackTrace(e error) errors.StackTrace {
	type causer interface {
	Cause() error
	}
	cause := e
	var tracer StackTracer
	for cause != nil {
	serr, ok := cause.(StackTracer) //nolint:errorlint
	if ok {
	tracer = serr
	}
	cerr, ok := cause.(causer) //nolint:errorlint
	if !ok {
	break
	}
	cause = cerr.Cause()
	}
	if tracer == nil {
	return nil
	}
	return tracer.StackTrace()
	}

hcsshim/internal/guest/bridge/bridge.go

Lines 458 to 472 in 1d69a9c

	if stack := gcserr.BaseStackTrace(errForResponse); stack != nil {
	bottomFrame := stack[0]
	stackString = fmt.Sprintf("%+v", stack)
	fileName = fmt.Sprintf("%s", bottomFrame)
	lineNumberStr := fmt.Sprintf("%d", bottomFrame)
	if n, err := strconv.ParseUint(lineNumberStr, 10, 32); err == nil {
	lineNumber = uint32(n)
	} else {
	logrus.WithFields(logrus.Fields{
	"line-number": lineNumberStr,
	logrus.ErrorKey: err,
	}).Error("opengcs::bridge::setErrorForResponseBase - failed to parse line number, using -1 instead")
	}
	functionName = fmt.Sprintf("%n", bottomFrame)
	}

[kolyshkin]

unfortunately a simple replace won't work, since we rely on the stack traces that "github.com/pkg/errors" adds to the errors in certain cases, so we would need to reimplement the functionality:

I don't think this is a "must have" functionality here. It is only used from a single place (setErrorForResponseBase in internal/guest/bridge/bridge.go), and in there it seems to be optional. In any case, if an error stack trace is needed, it is to be reimplemented in some other way (say, whoever returns an error adds a stack trace explicitly).

I am (trying to) remove this as part of #2290.