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.

Sign up for GitHub

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(client): prioritize BizStatusError in DefaultClientErrorHandler #1253

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

feat(client): prioritize BizStatusError in DefaultClientErrorHandler #1253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion client/middlewares.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,8 +161,17 @@ func newIOErrorHandleMW(errHandle func(context.Context, error) error) endpoint.M

// DefaultClientErrorHandler is Default ErrorHandler for client
// when no ErrorHandler is specified with Option `client.WithErrorHandler`, this ErrorHandler will be injected.
// for thrift、KitexProtobuf, >= v0.4.0 wrap protocol error to TransError, which will be more friendly.
// For thrift、KitexProtobuf >= v0.4.0, wraps protocol error to TransError, which will be more friendly.
// For thrift、KitexProtobuf >= v0.8.1, returns BizStatusError directly if it is set.
func DefaultClientErrorHandler(ctx context.Context, err error) error {
rpcInfo := rpcinfo.GetRPCInfo(ctx)
// If BizStatusErr is not nil, it means that the business logic has been processed and the error has been set
// and transmitted to the client. In this case, just return the bizErr directly.
bizErr := rpcInfo.Invocation().BizStatusErr()
if bizErr != nil {
return bizErr
}

switch err.(type) {
// for thrift、KitexProtobuf, actually check *remote.TransError is enough
case *remote.TransError, thrift.TApplicationException, protobuf.PBError:
Expand Down
15 changes: 11 additions & 4 deletions client/middlewares_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func TestDefaultErrorHandler(t *testing.T) {
reqCtx := rpcinfo.NewCtxWithRPCInfo(context.Background(), ri)

// Test TApplicationException
err := DefaultClientErrorHandler(context.Background(), thrift.NewTApplicationException(100, "mock"))
err := DefaultClientErrorHandler(reqCtx, thrift.NewTApplicationException(100, "mock"))
test.Assert(t, err.Error() == "remote or network error[remote]: mock", err.Error())
var te thrift.TApplicationException
ok := errors.As(err, &te)
Expand All @@ -154,7 +154,7 @@ func TestDefaultErrorHandler(t *testing.T) {
test.Assert(t, te.TypeId() == 100)

// Test PbError
err = DefaultClientErrorHandler(context.Background(), protobuf.NewPbError(100, "mock"))
err = DefaultClientErrorHandler(reqCtx, protobuf.NewPbError(100, "mock"))
test.Assert(t, err.Error() == "remote or network error[remote]: mock")
var pe protobuf.PBError
ok = errors.As(err, &pe)
Expand All @@ -168,18 +168,25 @@ func TestDefaultErrorHandler(t *testing.T) {
test.Assert(t, te.TypeId() == 100)

// Test status.Error
err = DefaultClientErrorHandler(context.Background(), status.Err(100, "mock"))
err = DefaultClientErrorHandler(reqCtx, status.Err(100, "mock"))
test.Assert(t, err.Error() == "remote or network error: rpc error: code = 100 desc = mock", err.Error())
// Test status.Error with remote addr
err = ClientErrorHandlerWithAddr(reqCtx, status.Err(100, "mock"))
test.Assert(t, err.Error() == "remote or network error["+tcpAddrStr+"]: rpc error: code = 100 desc = mock", err.Error())

// Test other error
err = DefaultClientErrorHandler(context.Background(), errors.New("mock"))
err = DefaultClientErrorHandler(reqCtx, errors.New("mock"))
test.Assert(t, err.Error() == "remote or network error: mock")
// Test other error with remote addr
err = ClientErrorHandlerWithAddr(reqCtx, errors.New("mock"))
test.Assert(t, err.Error() == "remote or network error["+tcpAddrStr+"]: mock")

// Test BizStatusError set
ri.Invocation().(rpcinfo.InvocationSetter).SetBizStatusErr(kerrors.NewBizStatusError(1024, "biz"))
err = DefaultClientErrorHandler(reqCtx, errors.New("mock"))
err, ok = kerrors.FromBizStatusError(err)
test.Assert(t, ok, "should return BizStatusError here")
test.Assert(t, err.Error() == "biz error: code=1024, msg=biz", err.Error())
}

func TestNewProxyMW(t *testing.T) {
Expand Down
Loading