pact-foundation / pact-reference

Reference implementations for the pact specifications
https://pact.io
MIT License
93 stars 46 forks source link

ffi v0.4.5 ERROR verifying plugin interactions #278

Closed mefellows closed 1 year ago

mefellows commented 1 year ago

When verifying this gRPC consumer test, the verification process fails with the following error:

2023-05-29T05:31:09.4366676Z Verifying a pact between grpcconsumer and grpcprovider
2023-05-29T05:31:09.4366884Z 
2023-05-29T05:31:09.4367158Z   Route guide - GetFeature (0s loading, 544ms verification)
2023-05-29T05:31:09.4367552Z      Given feature 'Big Tree' exists
2023-05-29T05:31:09.4367712Z 
2023-05-29T05:31:09.4367719Z 
2023-05-29T05:31:09.4367791Z Failures:
2023-05-29T05:31:09.4367911Z 
2023-05-29T05:31:09.4368619Z 1) Verifying a pact between grpcconsumer and grpcprovider Given feature 'Big Tree' exists - Route guide - GetFeature - Failed to prepare interaction for verification - Failed to prepate the request: Did not find interaction with key '5827c9fe506611a0' in the Pact

The contract file does not contain any keys, which were removed by the changes in #264, so this error is extra suspicious to me. Note the rest of the non-plugin based example suite works OK, so this seems to be specific to plugins.

Test run (with DEBUG logs enabled): https://github.com/pact-foundation/pact-go/actions/runs/5108825373/jobs/9182994524 (I had to view the raw logs to be able to find the above error message due to the noise and slowness of the search capability in GitHub).

github-actions[bot] commented 1 year ago

πŸ‘‹ Hi! The 'smartbear-supported' label has just been added to this issue, which will create an internal tracking ticket in PactFlow's Jira (PACT-1046). We will use this to prioritise and assign a team member to this task. All activity will be public on this ticket. For now, sit tight and we'll update this ticket once we have more information on the next steps.

See our documentation for more information.

rholshausen commented 1 year ago

Fixed in gRPC plugin: https://github.com/pactflow/pact-protobuf-plugin/commit/4b2c8397394173663a1d68e92dee99b4215df9ae

mefellows commented 1 year ago

Thanks Ron. I just took a look at the change. Do I understand it correctly that an interaction must have a key on it now for the gRPC plugin to work? It appears that without a key, it won't be found in the pact file and thus an error would be raised? Given the recent changes to the ffi (and possibly to the non-FFI pathways) to not automatically assign a key, does this mean client libraries now must do so?

rholshausen commented 1 year ago

No, this is internal to the gRPC plugin (and possibly other plugins as well). The plugins get the key for the interaction to verify via the plugin interface, so we can't just drop it without changing that interface.

mefellows commented 1 year ago

cool, got it. Thanks, just wanted to make sure we didn't need to update all of the clients for this to work. I'll test it once the release is out.

mefellows commented 1 year ago

Gah! https://github.com/pactflow/pact-protobuf-plugin/actions/runs/5174044018/jobs/9319923289

rholshausen commented 1 year ago

It's ok, I ran the release locally and uploaded the artifacts.

mefellows commented 1 year ago

Ah, thanks - seems to be working again now!

So I'm now trying to re-understand how a Plugin can look up the interaction in the pact file during verification.

In PrepareInteractionForVerification and VerifyInteraction the plugin will receive an InteractionKey that has a unique reference for the plugin to (presumably) label and find an interaction.

In the case of PrepareInteractionForVerification if no key is set on the interaction in the consumer DSL, the Pact struct if parsed, will not have any interactions with a key on them, and thus the interaction can't be found this way. There is no other identifying information at this step that can be used to look up the correct interaction from the pact.

This is why had the previous query about the keys above. I'm confused as to how the gRPC plugin is able to lookup by the interaction's key, despite not setting keys explicitly from the client. The Pact struct serialised over the protobuf definition does not have any keys in it:

For example, here is a Golang log message for the incoming message for VerifyInteraction:

2023/06/05 22:03:42 [INFO] received VerifyInteraction request: interactionData:{body:{contentType:"application/matt"  content:{value:"MATTMATT"}}}  config:{fields:{key:"host"  value:{string_value:"localhost"}}  fields:{key:"port"  value:{number_value:50160}}}  pact:"{\"consumer\":{\"name\":\"matttcpconsumer\"},\"interactions\":[{\"description\":\"Matt message\",\"pending\":false,\"providerStates\":[{\"name\":\"the world exists\"}],\"request\":{\"contents\":{\"content\":\"MATThellotcpMATT\",\"contentType\":\"application/matt\",\"contentTypeHint\":\"DEFAULT\",\"encoded\":false}},\"response\":[{\"contents\":{\"content\":\"MATTtcpworldMATT\",\"contentType\":\"application/matt\",\"contentTypeHint\":\"DEFAULT\",\"encoded\":false}}],\"transport\":\"matt\",\"type\":\"Synchronous/Messages\"}],\"metadata\":{\"pactRust\":{\"ffi\":\"0.4.5\",\"mockserver\":\"1.1.1\",\"models\":\"1.1.2\"},\"pactSpecification\":{\"version\":\"4.0\"},\"plugins\":[{\"configuration\":{},\"name\":\"matt\",\"version\":\"0.0.7\"}]},\"provider\":{\"name\":\"matttcpprovider\"}}"  interactionKey:"928c6a4275c7cafd"

Note how the interactionKey is present on the struct, but there is no matching key in the Pact struct.

This leads me to think that unique_key is an internal method only available to the Rust library, and not something that can be relied upon over the wire, and is why I thought removing the key generation could be a breaking change as far as behaviour is concerned.

(UPDATE: I thought perhaps I could work around it by setting a key on the interaction but I don't think there is an FFI for it)

rholshausen commented 1 year ago

Unfortunately, we can't remove InteractionKey from PrepareInteractionForVerification, because that will change the interface to existing plugins. We will have to create a V2 of the plugin interface.

At the moment it works by magic.

mefellows commented 1 year ago

Unfortunately, we can't remove InteractionKey from PrepareInteractionForVerification, because that will change the interface to existing plugins. We will have to create a V2 of the plugin interface.

Yeah, that's fine we can't do that. I can see the value in having it if the interactions were keyed. It vastly simplifies the lookup for plugin authors.

At the moment it works by magic.

πŸ˜†

mefellows commented 1 year ago

I'm going to close this issue, and re-open a new one with the clarification on this last bit.