Repository: ktr0731/evans (go)

Issue:
Custom message is not shown on status details
## Describe the bug
When a gRPC server returns error with own custom message in status details, evans doesn't display the message even with --enrich option.
If the message type is ptype (e.g google.protobuf.Timestamp), is it shown. Message types built in the evans binary could be handled.

It happens with both giving proto files or using reflection.
I've confirmed the custom message is provided as details using another tool https://github.com/fullstorydev/grpcurl.

## To reproduce
Here is a Dockerfile to reproduce it: https://github.com/terashi58/grpc-details-test

```
$ git clone https://github.com/terashi58/grpc-details-test.git
$ cd grpc-details-test
$ Docker build . -t grpc-details-test
$ docker run --rm -it grpc-details-test
docker:# tmux
docker:tmux0:# ./bin/server
2022/04/08 16:51:53 server listening at [::]:50051

[Ctrl-b c]
docker:tmux1:# echo '{"name":"gopher","error_message":"fail"}' | evans --proto ./src/proto/hello.proto cli call --enrich Say
content-type: application/grpc

code: Internal
number: 13
message: "error with details"
details:
  {"@type": "type.googleapis.com/google.protobuf.Timestamp", "value": "2022-04-08T16:52:41.277665562Z"}

evans: code = Internal, number = 13, message = "error with details"
```

Summary of the sample codes:

Here is the proto definition:
```hello.proto
package sample;

service Hello {
  rpc Say(SayRequest) returns (SayResponse) {}
}

message Extension {
  string message = 1;
}

message SayRequest {
  string name = 1;
  string error_message = 2;
}

message SayResponse {
  string greet = 1;
}
```

And the RPC implementation:

```.go
func (s *server) Say(ctx context.Context, in *pb.SayRequest) (*pb.SayResponse, error) {
	log.Printf("Received: %v", in.GetName())
	if msg := in.GetErrorMessage(); msg != "" {
		st := status.New(codes.Internal, "error with details")
		st2, err := st.WithDetails(
			&pb.Extension{Message: msg},
			ptypes.TimestampNow(),
		)
		if err != nil {
			return nil, err
		}
		return nil, st2.Err()
	}
	return &pb.SayResponse{Greet: "Hello " + in.GetName()}, nil
}
```

The rpc returns error with the `custom sample.Extension` and `google.protobuf.Timestamp` as status details.
evans shows the Timestamp only in the details section.

## Expected behavior
The custom message is also shown in the details section:

```
docker:tmux1:# echo '{"name":"gopher","error_message":"fail"}' | evans --proto ./src/proto/hello.proto cli call --enrich Say
content-type: application/grpc

code: Internal
number: 13
message: "error with details"
details:
  {"@type": "type.googleapis.com/sample.Extension", "message": "fail"}
  {"@type": "type.googleapis.com/google.protobuf.Timestamp", "value": "2022-04-08T16:52:41.277665562Z"}

evans: code = Internal, number = 13, message = "error with details"
```

## Environment
- OS: Ubuntu 20.04 + debian:bullseye docker image
- Terminal: iTerm2 + tmux (in docker)
- Evans version: 0.10.4
- `protoc` version: 3.20.0
- `protoc` plugin version (if you are using):
  - protoc-gen-go: 1.28.0
  - protoc-gen-go-grpc: 1.2.0

## Additional context
<!-- Add any other context about the problem here. -->
You can run the sample server in the docker image by `docker run -p 50051:50051 --rm grpc-details-test ./bin/server` and test from a local evans.


Pull request description:
fix to register messages when message parsing
This fixes #529.
In Protocol Buffers v2, we must register messages explicitly by using protoregistry.

Patch:
diff --git a/go.mod b/go.mod
index 9644f2e..912a4bf 100644
--- a/go.mod
+++ b/go.mod
@@ -19,7 +19,7 @@ require (
 	github.com/ktr0731/go-prompt v0.2.4
 	github.com/ktr0731/go-shellstring v0.1.3
 	github.com/ktr0731/go-updater v0.1.5
-	github.com/ktr0731/grpc-test v0.1.10
+	github.com/ktr0731/grpc-test v0.1.11
 	github.com/ktr0731/grpc-web-go-client v0.2.8
 	github.com/manifoldco/promptui v0.9.0
 	github.com/matryer/moq v0.2.7
diff --git a/go.sum b/go.sum
index e812c95..66f7d57 100644
--- a/go.sum
+++ b/go.sum
@@ -728,8 +728,8 @@ github.com/ktr0731/go-shellstring v0.1.3/go.mod h1:f0/XX0hsm6A02Es/UDQ9MGd3VqpCH
 github.com/ktr0731/go-updater v0.1.5 h1:AiaQxJoI0OTGqvsJYdIITCOaEzQQOqU2MHKUwttVShY=
 github.com/ktr0731/go-updater v0.1.5/go.mod h1:dsdOg7a9sj6ttcOU5ZxPCtKdm9WeB1hwcX/7oKgz9/0=
 github.com/ktr0731/grpc-test v0.1.4/go.mod h1:v47616grayBYXQveGWxO3OwjLB3nEEnHsZuMTc73FM0=
-github.com/ktr0731/grpc-test v0.1.10 h1:W+fdfNrcSY0+9aiIIx+CWpFcNjdcqysPQchDcuMk4Pk=
-github.com/ktr0731/grpc-test v0.1.10/go.mod h1:AP4+ZrqSzdDaUNhAsp2fye06MXO2fdYY6YQJifb588M=
+github.com/ktr0731/grpc-test v0.1.11 h1:sXacyTdat8F1SWURSqECG1Cu546A5sYgI88XVexJ66Q=
+github.com/ktr0731/grpc-test v0.1.11/go.mod h1:AP4+ZrqSzdDaUNhAsp2fye06MXO2fdYY6YQJifb588M=
 github.com/ktr0731/grpc-web-go-client v0.2.8 h1:nUf9p+YWirmFwmH0mwtAWhuXvzovc+/3C/eAY2Fshnk=
 github.com/ktr0731/grpc-web-go-client v0.2.8/go.mod h1:1Iac8gFJvC/DRfZoGnFZsfEbEq/wQFK+2Ve1o3pHkCQ=
 github.com/ktr0731/modfile v1.11.2/go.mod h1:LzNwnHJWHbuDh3BO17lIqzqDldXqGu1HCydWH3SinE0=
diff --git a/idl/proto/proto.go b/idl/proto/proto.go
index 4e5e98f..d0c20fb 100644
--- a/idl/proto/proto.go
+++ b/idl/proto/proto.go
@@ -14,6 +14,9 @@ import (
 	"github.com/ktr0731/evans/grpc/grpcreflection"
 	"github.com/ktr0731/evans/idl"
 	"github.com/pkg/errors"
+	"google.golang.org/protobuf/reflect/protodesc"
+	"google.golang.org/protobuf/reflect/protoregistry"
+	"google.golang.org/protobuf/types/dynamicpb"
 )
 
 type spec struct {
@@ -140,7 +143,7 @@ func LoadFiles(importPaths []string, fnames []string) (idl.Spec, error) {
 		fileDescs = append(fileDescs, d.GetDependencies()...)
 	}
 
-	return newSpec(fileDescs), nil
+	return newSpec(fileDescs)
 }
 
 // LoadByReflection receives a gRPC reflection client, then tries to instantiate a new idl.Spec by using gRPC reflection.
@@ -149,10 +152,10 @@ func LoadByReflection(client grpcreflection.Client) (idl.Spec, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to list packages by gRPC reflection")
 	}
-	return newSpec(fileDescs), nil
+	return newSpec(fileDescs)
 }
 
-func newSpec(fds []*desc.FileDescriptor) idl.Spec {
+func newSpec(fds []*desc.FileDescriptor) (idl.Spec, error) {
 	var (
 		encounteredPkgs = make(map[string]interface{})
 		encounteredSvcs = make(map[string]interface{})
@@ -161,6 +164,7 @@ func newSpec(fds []*desc.FileDescriptor) idl.Spec {
 		rpcDescs        = make(map[string][]*desc.MethodDescriptor)
 		msgDescs        = make(map[string]*desc.MessageDescriptor)
 	)
+
 	for _, f := range fds {
 		if _, encountered := encounteredPkgs[f.GetPackage()]; !encountered {
 			pkgNames = append(pkgNames, f.GetPackage())
@@ -175,8 +179,27 @@ func newSpec(fds []*desc.FileDescriptor) idl.Spec {
 			rpcDescs[fqsn] = append(rpcDescs[fqsn], svc.GetMethods()...)
 		}
 
-		for _, m := range f.GetMessageTypes() {
-			msgDescs[m.GetFullyQualifiedName()] = m
+		prfd, err := protodesc.NewFile(f.AsFileDescriptorProto(), protoregistry.GlobalFiles)
+		if err != nil {
+			return nil, errors.Wrapf(err, "failed to new protodesc")
+		}
+
+		if _, err := protoregistry.GlobalFiles.FindFileByPath(prfd.Path()); errors.Is(err, protoregistry.NotFound) {
+			if err := protoregistry.GlobalFiles.RegisterFile(prfd); err != nil {
+				return nil, err
+			}
+		}
+
+		for i := 0; i < prfd.Messages().Len(); i++ {
+			md := prfd.Messages().Get(i)
+			if _, err := protoregistry.GlobalTypes.FindMessageByName(md.FullName()); !errors.Is(err, protoregistry.NotFound) {
+				continue
+			}
+
+			mt := dynamicpb.NewMessageType(md)
+			if err := protoregistry.GlobalTypes.RegisterMessage(mt); err != nil {
+				return nil, errors.Wrapf(err, "failed to register message %s", mt.Descriptor().FullName())
+			}
 		}
 	}
 
@@ -190,7 +213,7 @@ func newSpec(fds []*desc.FileDescriptor) idl.Spec {
 		svcDescs:  svcDescs,
 		rpcDescs:  rpcDescs,
 		msgDescs:  msgDescs,
-	}
+	}, nil
 }
 
 // FullyQualifiedServiceName returns the fully-qualified service name.


Test patch:
diff --git a/e2e/cli_test.go b/e2e/cli_test.go
index 6206149..1584b3f 100644
--- a/e2e/cli_test.go
+++ b/e2e/cli_test.go
@@ -515,6 +515,14 @@ func TestE2E_CLI(t *testing.T) {
 			unflatten:        true,
 			assertWithGolden: true,
 		},
+		"call failure unary RPC with --enrich flag without reflection": {
+			commonFlags:      "--proto testdata/test.proto",
+			cmd:              "call",
+			args:             "--file testdata/unary_call.in --enrich api.Example.UnaryHeaderTrailerFailure",
+			unflatten:        true,
+			assertWithGolden: true,
+			expectedCode:     1,
+		},
 		"call failure unary RPC with --enrich flag": {
 			commonFlags:      "-r",
 			cmd:              "call",
@@ -542,6 +550,15 @@ func TestE2E_CLI(t *testing.T) {
 			unflatten:        true,
 			assertWithGolden: true,
 		},
+		"call failure unary RPC with --enrich flag without reflection against to gRPC-Web server": {
+			commonFlags:      "--web --proto testdata/test.proto",
+			cmd:              "call",
+			args:             "--file testdata/unary_call.in --enrich api.Example.UnaryHeaderTrailerFailure",
+			web:              true,
+			unflatten:        true,
+			assertWithGolden: true,
+			expectedCode:     1,
+		},
 		"call failure unary RPC with --enrich flag against to gRPC-Web server": {
 			commonFlags:      "--web -r",
 			cmd:              "call",
diff --git a/e2e/e2e_test.go b/e2e/e2e_test.go
index 3998a1c..da6b3e4 100644
--- a/e2e/e2e_test.go
+++ b/e2e/e2e_test.go
@@ -53,6 +53,11 @@ func TestMain(m *testing.M) {
 	cleanup2 := setEnv("XDG_CACHE_HOME", cacheDir)
 	defer cleanup2()
 
+	// TODO: Make registered types to be independent instead of ignoring conflicts.
+	// Need to make grpc-status-details-bin decoding to be used a passed registry.
+	cleanup3 := setEnv("GOLANG_PROTOBUF_REGISTRATION_CONFLICT", "ignore")
+	defer cleanup3()
+
 	goleak.VerifyTestMain(m, goleak.IgnoreTopFunction("github.com/desertbit/timer.timerRoutine"))
 }
 
diff --git a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_and_json_format.golden b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_and_json_format.golden
index b0e923c..cd37512 100644
--- a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_and_json_format.golden
+++ b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_and_json_format.golden
@@ -22,6 +22,10 @@
             "type": "type"
           }
         ]
+      },
+      {
+        "@type": "type.googleapis.com/api.FailureDetail",
+        "code": "001"
       }
     ]
   },
diff --git a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag.golden b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag.golden
index 231d64f..7038293 100644
--- a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag.golden
+++ b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag.golden
@@ -11,4 +11,5 @@ message: "internal error"
 details: 
   {"@type": "type.googleapis.com/google.rpc.BadRequest", "fieldViolations": [{"description": "description", "field": "field"}]}
   {"@type": "type.googleapis.com/google.rpc.PreconditionFailure", "violations": [{"description": "description", "subject": "subject", "type": "type"}]}
+  {"@type": "type.googleapis.com/api.FailureDetail", "code": "001"}
 
diff --git a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_against_to_grpc-web_server.golden b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_against_to_grpc-web_server.golden
index 55385eb..1d915ab 100644
--- a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_against_to_grpc-web_server.golden
+++ b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_against_to_grpc-web_server.golden
@@ -12,4 +12,5 @@ message: "internal error"
 details: 
   {"@type": "type.googleapis.com/google.rpc.BadRequest", "fieldViolations": [{"description": "description", "field": "field"}]}
   {"@type": "type.googleapis.com/google.rpc.PreconditionFailure", "violations": [{"description": "description", "subject": "subject", "type": "type"}]}
+  {"@type": "type.googleapis.com/api.FailureDetail", "code": "001"}
 
diff --git a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_without_reflection.golden b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_without_reflection.golden
new file mode 100644
index 0000000..7038293
--- /dev/null
+++ b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_without_reflection.golden
@@ -0,0 +1,15 @@
+content-type: application/grpc
+header_key1: header_val1
+header_key2: header_val2
+
+trailer_key1: trailer_val1
+trailer_key2: trailer_val2
+
+code: Internal
+number: 13
+message: "internal error"
+details: 
+  {"@type": "type.googleapis.com/google.rpc.BadRequest", "fieldViolations": [{"description": "description", "field": "field"}]}
+  {"@type": "type.googleapis.com/google.rpc.PreconditionFailure", "violations": [{"description": "description", "subject": "subject", "type": "type"}]}
+  {"@type": "type.googleapis.com/api.FailureDetail", "code": "001"}
+
diff --git a/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_without_reflection_against_to_grpc-web_server.golden b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_without_reflection_against_to_grpc-web_server.golden
new file mode 100644
index 0000000..1d915ab
--- /dev/null
+++ b/e2e/testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_without_reflection_against_to_grpc-web_server.golden
@@ -0,0 +1,16 @@
+content-type: application/grpc-web+proto
+header_key1: header_val1
+header_key2: header_val2
+vary: Origin
+
+trailer_key1: trailer_val1
+trailer_key2: trailer_val2
+
+code: Internal
+number: 13
+message: "internal error"
+details: 
+  {"@type": "type.googleapis.com/google.rpc.BadRequest", "fieldViolations": [{"description": "description", "field": "field"}]}
+  {"@type": "type.googleapis.com/google.rpc.PreconditionFailure", "violations": [{"description": "description", "subject": "subject", "type": "type"}]}
+  {"@type": "type.googleapis.com/api.FailureDetail", "code": "001"}
+
diff --git a/e2e/testdata/fixtures/teste2e_repl-call_unaryheadertrailerfailure_by_selecting_package_and_service_with_enriched_output.golden b/e2e/testdata/fixtures/teste2e_repl-call_unaryheadertrailerfailure_by_selecting_package_and_service_with_enriched_output.golden
index 06a8e38..7dd6208 100644
--- a/e2e/testdata/fixtures/teste2e_repl-call_unaryheadertrailerfailure_by_selecting_package_and_service_with_enriched_output.golden
+++ b/e2e/testdata/fixtures/teste2e_repl-call_unaryheadertrailerfailure_by_selecting_package_and_service_with_enriched_output.golden
@@ -13,5 +13,6 @@ message: "internal error"
 details: 
   {"@type": "type.googleapis.com/google.rpc.BadRequest", "fieldViolations": [{"description": "description", "field": "field"}]}
   {"@type": "type.googleapis.com/google.rpc.PreconditionFailure", "violations": [{"description": "description", "subject": "subject", "type": "type"}]}
+  {"@type": "type.googleapis.com/api.FailureDetail", "code": "001"}
 
 
diff --git a/e2e/testdata/test.proto b/e2e/testdata/test.proto
index 38e1f59..c2f15fd 100644
--- a/e2e/testdata/test.proto
+++ b/e2e/testdata/test.proto
@@ -99,3 +99,7 @@ message UnaryHeaderRequest {}
 message MapResponse {
   map<string, Name> names = 1;
 }
+
+message FailureDetail {
+  string code = 1;
+}