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

Match should skip unexported fields as they won't be serialized #201

Merged
merged 2 commits into from
Feb 9, 2022

Conversation

codyaray
Copy link
Contributor

@codyaray codyaray commented Feb 8, 2022

Some of our DTO objects include unexported fields.

This small patch just ignores those fields, as they aren't serialized by the JSON encoder. This means they shouldn't form part of the API contract. And they can even cause panics if they include types that can't be handled.

Although I ran into this issue while experimenting with GRPC + Pact, I think this fix makes sense outside of this context as well. Any manually-written object might also have non-exported fields that shouldn't be part of the API Contract, while still ideally being supported by dsl.Match.

GRPC Details

Note: these details probably aren't needed. This gets into the complications of my experiments with GRPC + Pact. But I wanted to include it for completeness.

I received this panic when creating the consumer pact:

2022-02-08 09:07:15.6648  ┃ panic: match: unhandled type: func() [recovered]
2022-02-08 09:07:15.6649  ┃     panic: match: unhandled type: func()
2022-02-08 09:07:15.6649  ┃ 
2022-02-08 09:07:15.6649  ┃ goroutine 67 [running]:
2022-02-08 09:07:15.6649  ┃ testing.tRunner.func1.2({0x3256480, 0xc00059c120})
2022-02-08 09:07:15.6649  ┃     /usr/local/opt/go/libexec/src/testing/testing.go:1209 +0x36c
2022-02-08 09:07:15.6649  ┃ testing.tRunner.func1()
2022-02-08 09:07:15.6649  ┃     /usr/local/opt/go/libexec/src/testing/testing.go:1212 +0x3b6
2022-02-08 09:07:15.6649  ┃ panic({0x3256480, 0xc00059c120})
2022-02-08 09:07:15.6649  ┃     /usr/local/opt/go/libexec/src/runtime/panic.go:1047 +0x266
2022-02-08 09:07:15.6649  ┃ github.com/pact-foundation/pact-go/dsl.match({0x393c530, 0x322b520}, {{0x1}, {{0x0, 0x0}, {0x0, 0x0}}, {0x0, 0x0}, {0x0, ...}})
2022-02-08 09:07:15.6649  ┃     /Users/cody/git/go/pkg/mod/github.com/pact-foundation/[email protected]/dsl/matcher.go:338 +0x7e5

This was caused by an interaction like

		pact.
			AddInteraction().
			UponReceiving("A hello world request").
			WithRequest(dsl.Request{
				Method: "GET",
				Path:   dsl.Term("/v1/example/echo", "/v1/example/echo"),
			}).
			WillRespondWith(dsl.Response{
				Status:  200,
				Body:    pactpb.Match(&apiv1.SayHelloResponse{}),
				Headers: commonHeaders,
			})

That pactpb helper delegates to dsl.Match(src) but adds support for GRPC well-known types, like google.protobuf.Timestamp.

The apiv1.SayHelloResponse is from a protobuf like

// SayHelloRequest defines the response parameters for a StayHello request
message SayHelloResponse {
    // Message to say to the person
    string message = 1;
    // Time at which this message was created
    google.protobuf.Timestamp date_time = 2;
}

Compiling with the go generator results in

// SayHelloRequest defines the response parameters for a StayHello request
type SayHelloResponse struct {
	state         protoimpl.MessageState
	sizeCache     protoimpl.SizeCache
	unknownFields protoimpl.UnknownFields

	// Message to say to the person
	Message string `protobuf:"bytes,1,opt,name=message,proto3" json:"message,omitempty"`
	// Time at which this message was created
	DateTime *timestamppb.Timestamp `protobuf:"bytes,2,opt,name=date_time,json=dateTime,proto3" json:"date_time,omitempty"`
}

The panic above is due to the unexported state field:

type MessageState struct {
	pragma.NoUnkeyedLiterals
	pragma.DoNotCompare
	pragma.DoNotCopy

	atomicMessageInfo *MessageInfo
}

whose DoNotCompare is

type DoNotCompare [0]func()

Hence the panic on unhandled type: func()

So by just ignoring unexported fields (as I think we should be doing anyway), we can avoid this whole mess. :)

@mefellows
Copy link
Member

Thanks for this! I'll merge it in now and port it to the 2.x.x branch also.

I hope to support the plugin interface in Golang soon, which means you'll be able to try using the protobufs plugin (and soon after, the gRPC one).

@mefellows mefellows merged commit 25a5ad5 into pact-foundation:master Feb 9, 2022
@mefellows
Copy link
Member

v1.6.9 released with this in it!

@codyaray
Copy link
Contributor Author

Awesome, thanks! Looking forward to the gRPC bits too :)

@codyaray codyaray deleted the unexported branch February 10, 2022 19:35
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.

2 participants