status: handle wrapped errors

[psyhatter]

RELEASE NOTES:

• status: status.Code and status.FromError handle wrapped errors

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: psyhatter / name: Rusakov Andrei (6cc3ab3)

[psyhatter]

Hello, @easwars. Please take a look at PR. It seems like a small one =)

[easwars]

@psyhatter : Just wondering the use case behind the change? I agree that it would be nice for the status package to handle wrapped errors.

@dfawley : As I understand, this would be a behavior change. I vaguely remember issues/PRs/questions about switching the status package to support wrapped errors in the past, but I don't quite remember the outcome of those discussions. I feel that this would a good change. If we go with this, we would need to update the docstrings for these functions and also make a bigger noise in the release notes. But I also wanted to check with your thoughts on this before proceeding much further with the review.

[dfawley]

I'm okay with this particular aspect of error wrapping. IIRC we didn't do this earlier because errors.As was too new for us to use given our Go verison support policy.

But we should take a look over the old related issues/PRs and make sure we aren't forgetting anything that we decided previously.

[psyhatter]

@psyhatter : Just wondering the use case behind the change?

Suppose we have a sentinel error:

package model

import (
	"google.golang.org/grpc/codes"
	"google.golang.org/grpc/status"
)

var ErrUserNotFound = status.Errorf(codes.NotFound, "user not found")

At the repository level, we return this sentinel error so that it can be handled at the service level with errors.Is(err, model.ErrUserNotFound). But sometimes handling is not required, but it is required to wrap the received error in order to add context to it:

sender, err := s.repo.GetUser(ctx, senderID)
if err != nil {
	return fmt.Errorf("getting sender: %w", err)
}

receiver, err := s.repo.GetUser(ctx, receiverID)
if err != nil {
	return fmt.Errorf("getting receiver: %w", err)
}

But in this case, the grpc server will return an incorrect error code, because it uses this function, which does not know how to parse wrapped errors

Probably, sentinel errors used at the service level should not implement transporter level errors, and there should be some kind of transformation. But this is a fairly convenient way when there is only one transport layer

[biryukovmaxim]

@easwars what this pr waiting for?

[easwars]

@easwars what this pr waiting for?

Nothing. Why do you ask?
We are having a discussion.

[easwars]

Moving to @dfawley for second set of eyes.

[dfawley]

LGTM, except for 2 small nits.

[dfawley]

Nit: this is not a complete sentence, and doesn't explain clearly (to me, anyway) what's happening. Maybe:

//   - If err was produced by this package or implements the method `GRPCStatus()
//     *Status`, or if err wraps a type satisfying this, the appropriate Status is returned.

[dfawley]

Nit:

const code, message = codes.Internal, "test description"

(to avoid these being mutable variables when they should be invariants.)

And the other test cases, below.