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

Handle nil requests in GRPC client method builder #2186

Merged
merged 1 commit into from
Jul 8, 2019
Merged

Handle nil requests in GRPC client method builder #2186

merged 1 commit into from
Jul 8, 2019

Conversation

delkopiso
Copy link
Contributor

@delkopiso delkopiso commented Jul 8, 2019

What's up

The GRPC client remote method builder function casts the request interface into a pointer to the proto request struct. However, if the endpoint's input is an empty message then the client will be passing in a nil request interface. This causes a panic at the site of the interface cast.

What this does

Checks if the proto request obj is nil before casting. If it's nil, then we return a pointer to the empty proto request struct instead of casting the request interface to it.

Before

// BuildDoStuffFunc builds the remote method to invoke for "dummy" service
// "doStuff" endpoint.
func BuildDoStuffFunc(grpccli dummypb.DummyClient, cliopts ...grpc.CallOption) goagrpc.RemoteFunc {
	return func(ctx context.Context, reqpb interface{}, opts ...grpc.CallOption) (interface{}, error) {
		for _, opt := range cliopts {
			opts = append(opts, opt)
		}
		return grpccli.DoStuff(ctx, reqpb.(*dummypb.DoStuffRequest), opts...)
	}
}

After

// BuildDoStuffFunc builds the remote method to invoke for "dummy" service
// "doStuff" endpoint.
func BuildDoStuffFunc(grpccli dummypb.DummyClient, cliopts ...grpc.CallOption) goagrpc.RemoteFunc {
	return func(ctx context.Context, reqpb interface{}, opts ...grpc.CallOption) (interface{}, error) {
		for _, opt := range cliopts {
			opts = append(opts, opt)
		}
		if reqpb != nil {
			return grpccli.DoStuff(ctx, reqpb.(*dummypb.DoStuffRequest), opts...)
		}
		return grpccli.DoStuff(ctx, &dummypb.DoStuffRequest{}, opts...)
	}
}

Fixes #2185

What's up

The GRPC client remote method builder function casts the request interface into a pointer to the proto request struct. However, if the endpoint's input is an empty message then the client will be passing in a nil request interface. This causes a panic at the site of the interface cast.

What this does

Checks if the proto request obj is nil before casting. If it's nil, then we return a pointer to the empty proto request struct instead of casting the request interface to it.

Fixes #2185
@raphael
Copy link
Member

raphael commented Jul 8, 2019

This is great, thank you! Do you think you could back-port this fix to v2 as well? (simply create a branch off of v2 and cherry-pick this merge commit).

@raphael raphael merged commit 72fab95 into goadesign:v3 Jul 8, 2019
@delkopiso delkopiso deleted the nil-request-client-method-builder branch July 9, 2019 00:40
@delkopiso
Copy link
Contributor Author

@raphael yeah, just created #2187

raphael pushed a commit that referenced this pull request Jul 9, 2019
What's up

The GRPC client remote method builder function casts the request interface into a pointer to the proto request struct. However, if the endpoint's input is an empty message then the client will be passing in a nil request interface. This causes a panic at the site of the interface cast.

What this does

Checks if the proto request obj is nil before casting. If it's nil, then we return a pointer to the empty proto request struct instead of casting the request interface to it.

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

Successfully merging this pull request may close these issues.

v3: GRPC client invoke method builder panics for empty messages
2 participants