Repository: go-kratos/kratos
Pull request #1451 (2021-09-08 12:33:08)

fix: enhance errors.FromError
add converting support for grpc status whose details not contain any  `errdetails.ErrorInfo`

<!--
🎉 Thanks for sending a pull request to Kratos! Here are some tips for you:

1. If this is your first time contributing to Kratos, please read our contribution guide: https://go-kratos.dev/en/docs/community/contribution/
2. Ensure you have added or ran the appropriate tests and lint for your PR, please use `make lint` and `make test` before filing your PR, use `make clean` to tidy your go mod.
3. If the PR is unfinished, you may need mark it as a WIP(Work In Progress) PR or draft PR
4. Please use a conventional commits format title: `<type>[optional scope]: <description>`
    
    Some suggestion on <type>:

    fix: A bug fix
    feat: A new feature
    test: Adding missing tests or correcting existing tests
    refactor: A code change that neither fixes a bug nor adds a feature
    break: Changes has break change

    docs: Documentation only changes
    deps: Changes external dependencies
    style: Changes that do not affect the meaning of the code (white-space, formatting, etc)
    chore Daily work, examples, etc.
    ci: Changes to our CI configuration files and scripts
-->

#### Description (what this PR does / why we need it):
<!--
* The description should include the motivation for this PR or contrast this with previous behavior
-->
enhance errors.FromError
add converting support for grpc status whose details not contain any  `errdetails.ErrorInfo`

ie. a grpc status with code `codes.InvalidArgument` should be converted to `http.StatusBadRequest` instead of `UnknownCode`.

codecov-commenter: # [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1451?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) Report
> Merging [#1451](https://codecov.io/gh/go-kratos/kratos/pull/1451?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (f0d7e3d) into [main](https://codecov.io/gh/go-kratos/kratos/commit/2024fa7cdbaca110f10e41a06e5d5ee1124b4c06?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) (2024fa7) will **increase** coverage by `0.03%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/go-kratos/kratos/pull/1451/graphs/tree.svg?width=650&height=150&src=pr&token=XwnmBTz78x&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)](https://codecov.io/gh/go-kratos/kratos/pull/1451?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)

```diff
@@            Coverage Diff             @@
##             main    #1451      +/-   ##
==========================================
+ Coverage   64.82%   64.85%   +0.03%     
==========================================
  Files          64       64              
  Lines        2925     2928       +3     
==========================================
+ Hits         1896     1899       +3     
  Misses        870      870              
  Partials      159      159              
```


| [Impacted Files](https://codecov.io/gh/go-kratos/kratos/pull/1451?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos) | Coverage Δ | |
|---|---|---|
| [errors/errors.go](https://codecov.io/gh/go-kratos/kratos/pull/1451/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos#diff-ZXJyb3JzL2Vycm9ycy5nbw==) | `81.81% <100.00%> (+1.04%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1451?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/go-kratos/kratos/pull/1451?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Last update [2024fa7...f0d7e3d](https://codecov.io/gh/go-kratos/kratos/pull/1451?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=go-kratos).


Patch:
diff --git a/errors/errors.go b/errors/errors.go
index 8b2e7f55..64466fcb 100644
--- a/errors/errors.go
+++ b/errors/errors.go
@@ -5,7 +5,6 @@ import (
 	"fmt"
 
 	httpstatus "github.com/go-kratos/kratos/v2/transport/http/status"
-
 	"google.golang.org/genproto/googleapis/rpc/errdetails"
 	"google.golang.org/grpc/status"
 	"google.golang.org/protobuf/proto"
@@ -102,16 +101,19 @@ func FromError(err error) *Error {
 	}
 	gs, ok := status.FromError(err)
 	if ok {
+		ret := New(
+			httpstatus.FromGRPCCode(gs.Code()),
+			UnknownReason,
+			gs.Message(),
+		)
 		for _, detail := range gs.Details() {
 			switch d := detail.(type) {
 			case *errdetails.ErrorInfo:
-				return New(
-					httpstatus.FromGRPCCode(gs.Code()),
-					d.Reason,
-					gs.Message(),
-				).WithMetadata(d.Metadata)
+				ret.Reason = d.Reason
+				return ret.WithMetadata(d.Metadata)
 			}
 		}
+		return ret
 	}
 	return New(UnknownCode, UnknownReason, err.Error())
 }


Test patch:
diff --git a/errors/errors_test.go b/errors/errors_test.go
index b9287950..1408ec67 100644
--- a/errors/errors_test.go
+++ b/errors/errors_test.go
@@ -5,6 +5,10 @@ import (
 	"fmt"
 	"net/http"
 	"testing"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+	"google.golang.org/grpc/test/grpc_testing"
 )
 
 func TestError(t *testing.T) {
@@ -46,4 +50,11 @@ func TestError(t *testing.T) {
 	if se.Reason != "reason" {
 		t.Errorf("got %+v want %+v", se, err)
 	}
+
+	gs2, _ := status.New(codes.InvalidArgument, "bad request").WithDetails(&grpc_testing.Empty{})
+	se2 := FromError(gs2.Err())
+	// codes.InvalidArgument should convert to http.StatusBadRequest
+	if se2.Code != http.StatusBadRequest {
+		t.Errorf("convert code err, got %d want %d", UnknownCode, http.StatusBadRequest)
+	}
 }