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

feat: Pass request to pact mock server if no interaction found #44

Merged
merged 3 commits into from
Oct 6, 2023

Conversation

nicholas-lebrun-form3
Copy link
Contributor

@nicholas-lebrun-form3 nicholas-lebrun-form3 commented Oct 5, 2023

pact-proxy returns an error to the caller when an endpoint is invoked that does not match any interaction URLs. This prevents the pact mock server from correctly receiving unexpected requests.

A new env variable is added to allow passing unrecognised requests to pact mock server. This in turns allows us to retreive unexpected requests when issuing a verification GET request.

This change is opt-in via the env var to preserve the default behaviour of pact-proxy in existing repositories.

@nicholas-lebrun-form3 nicholas-lebrun-form3 force-pushed the nl-pass-requests-if-unrecognized branch 3 times, most recently from 879dd95 to 792bed8 Compare October 5, 2023 16:06
The pact-proxy would previously return an error to the caller when an endpoint was invoked
that did not match any interaction URLs. This prevents the pact mock server from correctly receiving
unexpected requests.

To allow pact mock server to receive unexpected requests, so it can report them in the verification endpoint,
a new configuration was added.

Setting the env var REJECT_UNRECOGNIZED_INTERACTIONS to false will result in the requests being passed to
the pact mock service instead of rejecting them at the pact-proxy level.

The default value of this is set to true so that the default behaviour of pact-proxy remains unchanged.
Clients must opt-into this behaviour when deploying pact-proxy.
@nicholas-lebrun-form3 nicholas-lebrun-form3 force-pushed the nl-pass-requests-if-unrecognized branch from 792bed8 to ea7f451 Compare October 5, 2023 16:11
@nicholas-lebrun-form3 nicholas-lebrun-form3 marked this pull request as ready for review October 5, 2023 16:12
@nicholas-lebrun-form3 nicholas-lebrun-form3 requested a review from a team as a code owner October 5, 2023 16:12
@@ -13,6 +13,9 @@ func NewFromEnv() (pactproxy.Config, error) {
ctx := context.Background()

var config pactproxy.Config

config.RejectUnrecognizedInteractions = true
Copy link

@adam-steinbok-form3 adam-steinbok-form3 Oct 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It feels like it might be more safe to flip the logic here. i.e. have the flag be AllowUnrecognizedInteractions.
That way the effect of not setting the field at all is the same as the existing functionality, and this won't be a breaking change if (for whatever reason) a caller is manually constructing a pactproxy.Config instead of using NewFromEnv.

internal/..., ignore me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do agree it's a bit confusing that the logic is flipped. I chose to do this because I was not satisfied with names I came up for the variable. Happy to change it if we come up with a good name.

What do you think of ForwardUnrecognisedRequests? I thought of it at first but thought it was unclear where they would be forwarded to.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's better, I went ahead and made the change.

@nicholas-lebrun-form3 nicholas-lebrun-form3 changed the title fix: Pass request to pact mock server if no interaction found feat: Pass request to pact mock server if no interaction found Oct 5, 2023
@nicholas-lebrun-form3 nicholas-lebrun-form3 merged commit 4b791c4 into master Oct 6, 2023
2 checks passed
@nicholas-lebrun-form3 nicholas-lebrun-form3 deleted the nl-pass-requests-if-unrecognized branch October 6, 2023 16:16
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.

3 participants