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

RFC: Distinguish technical failures when verifying pacts #505

Closed
flakey-bit opened this issue Jun 6, 2024 · 6 comments
Closed

RFC: Distinguish technical failures when verifying pacts #505

flakey-bit opened this issue Jun 6, 2024 · 6 comments
Labels
feature request Indicates new feature requests upstream Indicates that an issue relates to an upstream problem (such as in pact-reference)

Comments

@flakey-bit
Copy link

As provider, when we verify ourselves against published consumer Pacts (IPactVerifierSource::Verify), a PactFailureException can be thrown.

This exception doesn't distinguish between

  1. The provider simply not honouring one or more contracts &
  2. Technical failures (i.e. the server returning 500 responses) while evaluating the contracts

This is an important distinction - we'd like to fail our build if there are technical failures while self-evaluating against contracts, however if we simply don't honour a given contract then that's not reason to fail the build (the contract compatibilty will be published to the broker and we'll rely on can-i-deploy at deployment time).

Probably the simplest thing would be to introduce fields to PactFailureException providing more information on what happened (which contracts failed, what the status codes were etc). That shouldn't be a breaking change 🤞

No obvious downsides, other than the plumbing needed to pass the data in. As mentioned on the Slack, we might want something in core so that other implementations (not just dotnet) can benefit.

Describe alternatives you've considered
As a workaround, we've added a custom logging sink to our server when it evaluates the contracts. If any errors are logged then we fail the unit test (we're following the recommended approach whereby contract compatibility is evaluated in a unit test during the build).

Additional context
This came about because our provider codebase mocks/stubs various services when evaluating itself against Pact contracts. We were missing some stub setup calls, resulting in errors being logged / 500 responses.

@flakey-bit flakey-bit added the triage This issue is yet to be triaged by a maintainer label Jun 6, 2024
@adamrodger
Copy link
Contributor

Yeah I think having some extra info would be useful as to why verification failed. Obviously it would still throw an exception but you could choose to catch and ignore certain exceptions in this specific use case.

As you say, this would need changes to the core library instead of PactNet specifically at first, so it's probably best raising an issue there and linking here so it can be implemented once it's supported in FFI.

@adamrodger adamrodger added feature request Indicates new feature requests upstream Indicates that an issue relates to an upstream problem (such as in pact-reference) and removed triage This issue is yet to be triaged by a maintainer labels Jun 6, 2024
@flakey-bit
Copy link
Author

@adamrodger is pact-foundation/pact-reference#132 relevant?

@adamrodger
Copy link
Contributor

No, that was an issue whereby FFI always logged directly to stdout so you couldn't report the failure logs in test output, and that was sorted quite a while ago.

@flakey-bit
Copy link
Author

flakey-bit commented Jun 7, 2024

Having a look at the FFI code a bit (apologies, not a rust developer)

My rust-foo isn't good enough to work out what MismatchResult contains, however it might be everything we need is already available in the FFI?

@adamrodger

@flakey-bit
Copy link
Author

@adamrodger could you review whether the "upstream" label is still relevant based on my previous comment?

@flakey-bit
Copy link
Author

I've opened a PR: #508

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Indicates new feature requests upstream Indicates that an issue relates to an upstream problem (such as in pact-reference)
Projects
None yet
Development

No branches or pull requests

2 participants