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

Allow comparison for error responses against range definitions (5XX, 4XX) #60

Open
milda-a opened this issue Oct 2, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@milda-a
Copy link

milda-a commented Oct 2, 2024

We have consumer contracts for specific errors, say 403 and 500 response from a provider.
The provider has range error definitions in the open api spec, such as 4XX and 5XX.
Currently, if those two are present, Pactflow fails the contract by stating that there are no definitions for 403 and 500, when the provider has the range definitions for 4XX and 5XX (Pactflow expects 403 and 500 to be specifically defined).

It would be great if we could compare consumer specific error responses against provider range error responses.

@mefellows mefellows added the enhancement New feature or request label Oct 21, 2024
@milda-a
Copy link
Author

milda-a commented Oct 21, 2024

Screenshot 2024-10-21 at 11 01 03 Screenshot 2024-10-21 at 11 01 20

Two screenshots added, one showing the 4XX handling on provider side, other showing 400 error not handled on consumer contract.

@mefellows
Copy link
Contributor

In case it's helpful, this PR introduced support for "negative" scenario testing: #30

I think it explicitly calls out the response codes it supports, it might be straightforward enough to extend the logic to ranges.

@milda-a
Copy link
Author

milda-a commented Oct 31, 2024

hey @mefellows, I had a go over the codebase and that PR, I don't believe it's what we need to change specifically, as that particular part of the code (in the PR) doesn't get reached when we get this error. The problem is before that, because it's not finding an exact match of the error code between the mock and the spec.
The issue is that the code is defined as a number by default, so I guess it would need to be redefined as a string (to maybe match a pattern or something?) specifically to catch these 4XX and 5XX codes (perhaps 2XX for valid responses, depending on how the spec is written).

I'm a bit reluctant to make too extensive a change, given the above example was not quite what we're looking for, but could try to put some time into it if the above paragraph makes sense from your guys' POV.

Thanks

@mefellows
Copy link
Contributor

The issue is that the code is defined as a number by default, so I guess it would need to be redefined as a string (to maybe match a pattern or something?) specifically to catch these 4XX and 5XX codes (perhaps 2XX for valid responses, depending on how the spec is written).

Yes, that's a point. V4 spec supports Matchers on status codes, but that would entail a much bigger change as currently all matching rules and generators are ignored in this comparison.

I don't know how the lookup in the OAS happens in this code base, but I imagine we would need to build an index of the supported ranges and check the status code in the pact file against the range in the OAS prior to or as a replacement to exact matching. This would need to be enabled behind a new flag (e.g. --compare-status-code-range or something).

I think the rule might be as follows:

  1. For the given interaction, find the relevant resource (existing logic)
  2. For the matching resource find all status codes
  3. If the endpoint specifies one or more status codes that don't contain a range (i.e. have exact values), this matching response schema takes precedence for comparison
  4. Otherwise, if no match takes place and if the endpoint specifies one or more ranges, then check the status code in the consumer contract matches the given range (e.g. 2XX, 3XX etc.).
  5. If no matches found, then error (existing behaviour)

Relevent spec item: https://spec.openapis.org/oas/v3.1.1.html#patterned-fields-0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants