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.

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

Custom message is not shown on status details #529

Open
terashi58 opened this issue Apr 8, 2022 · 8 comments · Fixed by #530
Open

Custom message is not shown on status details #529

terashi58 opened this issue Apr 8, 2022 · 8 comments · Fixed by #530
Labels

Comments

@terashi58
Copy link

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:

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:

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

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.

@terashi58 terashi58 added the bug label Apr 8, 2022
@ktr0731
Copy link
Owner

ktr0731 commented Apr 15, 2022

I'm sorry for the late reply.
This is probably due to the lack of the user-defined type registered in the message registry which is held in Evans. However, I don't know why it was caused, so we still need to dig down more.
I'll try to find the root cause when I have time.

@ktr0731
Copy link
Owner

ktr0731 commented Apr 24, 2022

@terashi58
This issue is fixed in v0.10.5, so try it out!

@terashi58
Copy link
Author

@ktr0731
Thanks for your fix!

I confirmed the custom message in status details can be displayed now.
But when the proto file contains import and it is executed with --reflection option, the evans command gets an error.

$ echo '{"name":"gopher","error_message":"fail"}' | evans -r cli call --enrich Say
evans: failed to run CLI mode: 1 error occurred:

* failed to instantiate the spec: failed to new protodesc: proto: could not resolve import "proto/sub.proto": not found

I updated the minimum example (https://github.com/terashi58/grpc-details-test). You can reproduce with the same way above but running make evans-reflect at the end.

I guess this is a side-effect of the #530. If it is better to manage in separate issues, I'll file another one.

@ktr0731
Copy link
Owner

ktr0731 commented Apr 26, 2022

@terashi58
I'm sorry for the inconvenience. We reverted changes in v0.10.5 and bump v0.10.6 🙇
Also, we continue to work on this issue to resolve the regression.

@ktr0731 ktr0731 reopened this May 7, 2022
@ktr0731
Copy link
Owner

ktr0731 commented May 7, 2022

Hi, @terashi58.
We just merged a PR to resolve this issue. Could you try it again before publishing a release?
The changes were merged into the master branch. If you don't have a Go environment, please tell me your environment. I'll build an executable.

@terashi58
Copy link
Author

Thank you @ktr0731!
I confirmed the latest evans doesn't crash and can display the details on my example.
I also tested on my real environment, then it gets not to crash but still cannot display some details. Maybe it misses some proto definitions in certain conditions (might be importing from another package?).

I believe the latest build is improvement without regression from the last release.
Let me investigate the missing pattern deeper. I'll file another ticket once I figured out the detailed condition.

@ktr0731
Copy link
Owner

ktr0731 commented May 8, 2022

Thank you for the feedback! We'll fix the root issue as soon as possible if you figured out the details 🙇

@terashi58
Copy link
Author

I found that a message in the details is not shown if the message is not defined in a file which contains rpc definitions nor which is imported from files containing rpc definitions.
I'll describe how to reproduce it in another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants