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

Invalid pact file created after migration to pact v10: pact:matcher:type in response body #950

Closed
5 tasks done
M-Landwehr opened this issue Sep 23, 2022 · 7 comments
Closed
5 tasks done
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@M-Landwehr
Copy link

Software versions

  • OS: Windows 10
  • Consumer Pact library: pact-foundation/pact: 10.1.4,
  • Node Version: 16.17.0

Issue Checklist

Please confirm the following:

  • I have upgraded to the latest
  • I have the read the FAQs in the Readme
  • I have triple checked, that there are no unhandled promises in my code and have read the section on intermittent test failures
  • I have set my log level to debug and attached a log file showing the complete request/response cycle
  • For bonus points and virtual high fives, I have created a reproduceable git repository (see below) to illustrate the problem

Expected behaviour

Migration of consumer tests from pact 9 to pact 10 should be possible without major changes except the ones described in https://docs.pact.io/implementation_guides/javascript/docs/migrations/9-10.

The created pact file should still be valid.

Actual behaviour

After the update from pact 9 to pact 10, the pact mock response is wrong: It contains properties like 'pact:matcher:type': 'type', min: 1. A pact file is still created, but contains invalid responses.

Steps to reproduce

The created pact file looks like this:

 "response": {
        "body": {
          "result": {
            "min": 1,
            "pact:matcher:type": "type",
            "value": [
              {
                "cancelled": {
                  "pact:matcher:type": "type",
                  "value": false
                },
                "complexObject": {
                  "pact:matcher:type": "type",
                  "value": {
                    "value": {
                      "pact:matcher:type": "type",
                      "value": "value"
                    }
                  }
                },
                "descriptions": {
                  "pact:matcher:type": "type",
                  "value": {
                    "en": {
                      "pact:matcher:type": "type",
                      "value": "foo"
                    }
                  }
                },
                "name": {
                  "pact:matcher:type": "type",
                  "value": "name"
                }
              }
            ]
          }
        },

This seems to be a regression in pact 10: After changing the version to ^9.0.0 in the package.json the pact file is valid.

According to the logs, the issue seems to be with the header matcher:

2022-09-23T11:08:11.354110Z  WARN ThreadId(01) pact_models::content_types: Failed to parse '{"value":"application/json;charset=UTF-8","regex":"application/json(;\\s?charset=[\\w\\-]+)?","pact:matcher:type":"regex"}' as a content type: mime parse error: an invalid token was encountered, 7B at position 0
2022-09-23T11:08:11.354186Z  WARN ThreadId(01) pact_models::content_types: Failed to parse '{"value":"application/json;charset=UTF-8","regex":"application/json(;\\s?charset=[\\w\\-]+)?","pact:matcher:type":"regex"}' as a content type: mime parse error: an invalid token was encountered, 7B at position 0

removing the headers section from the willRespondWith part in the spec.js file and rerunning the test also results in a correct pact file

Relevant log files

log.log

@M-Landwehr M-Landwehr added the bug Indicates an unexpected problem or unintended behavior label Sep 23, 2022
@mefellows
Copy link
Member

Can you please share the pact test? I think you're right, the regex can now be used without the double escape (that the ruby core needed) and that might be the issue.

@M-Landwehr
Copy link
Author

@mefellows
Copy link
Member

Thanks. I think there must be a bug somewhere in the core, that is not happy with the JSON being sent to configure the application headers.

I do think now it's not necessary to apply the term regex any longer - I think any JSON charset will properly match now, so you could probably quickly experiment with dropping the matcher for a straight up application/json value and it should just work.

But the bug needs fixing.

Implementation notes to self - consider using the pactffi_with_header_v2 ffi function which properly supports multiple headers and check to see if the JSON structure is supported.

@M-Landwehr
Copy link
Author

Wow, thanks for the fast response! I changed it to

headers: {
    'Content-Type': 'application/json',
},

and it works fine 👍

@mefellows
Copy link
Member

Great thanks for letting me know, we'll fix the other one.

@jamesarosen
Copy link

I'm not sure whether this is related, but here are some notes from my debugging. I'm using Matchers (not MatchersV3) because my provider is Ruby and the pact-ruby client doesn't support v3.

If I specify a Matchers.string for headers:

.willRespondWith({
  body: Matchers.like({ some: 'object' }),
  headers: {
    'Content-Type': Matchers.regex({
      generate: "application/json; charset=utf-8",
      matcher: "application/json",
    }),
  }
})

then the Pact file has the generated value in response.headers.Content-Type but the matcher under response.matchingRules.header.$['Content-Type']:

"response": {
  "body": {…},
  "headers": {
    "Content-Type": "application/json; charset=utf-8"
  },
  "matchingRules": {
    "body": {…},
    "header": {
      "$['Content-Type']": {
        "combine": "AND",
        "matchers": [
          {
            "match": "regex",
            "regex": "application/json"
          }
        ]
      }
    }
  }
}

@mefellows
Copy link
Member

I believe this issue is now fixed, with the separate issue emerging #964 or #1058. Closing because we are using the new FFI method.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants