fix to register messages when message parsing (#530)

ktr0731 · Apr 24, 2022 · 155e704 · 155e704
1 parent 72b34a2
commit 155e704
Show file tree

Hide file tree

Showing 12 changed files with 96 additions and 9 deletions.
diff --git 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
@@ -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/...testdata/fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_and_json_format.golden b/...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
@@ -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/...s/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_against_to_grpc-web_server.golden b/...s/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/.../fixtures/teste2e_cli-call_failure_unary_rpc_with_--enrich_flag_without_reflection.golden b/.../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/...failure_unary_rpc_with_--enrich_flag_without_reflection_against_to_grpc-web_server.golden b/...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/...ll_unaryheadertrailerfailure_by_selecting_package_and_service_with_enriched_output.golden b/...ll_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
@@ -99,3 +99,7 @@ message UnaryHeaderRequest {}
 message MapResponse {
   map<string, Name> names = 1;
 }
+
+message FailureDetail {
+  string code = 1;
+}
diff --git 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
@@ -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
@@ -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.