payload changed by pact between consumer test and provider test

ajoanny commented 5 months ago

Thank you for reporting a bug! We appreciate it very much. Issues are a big input into the priorities for Pact-JS development

All italic text in this template is safe to remove before submitting

Thanks again!

Software versions

Please provide at least OS and version of pact-js

OS: MacOS Sonoma v14.4.1
Consumer Pact library: v14.3.3
Provider Pact library: v14.3.3
Node Version: v20.11.1

Issue Checklist

Please confirm the following:

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

Hello !

Expected behaviour

When this request is defined in the consumer test

.withRequest('POST', '/api/test', (builder) => {
  builder.headers({
    'Content-Type': 'application/vnd.api+json',
  });
  builder.jsonBody({ data: { attributes: { name: 'name' } } });
})

Then the request body used in the provider test should be

  { data: { attributes: { name: 'name' } } }

Actual behaviour

The body used in the request for the provider is

{"body":{"type":"Buffer","data":[123,34,100,97,116,97,34,58,123,34,97,116,116,114,105,98,117,116,101,115,34,58,123,34,110,97,109,101,34,58,34,110,97,109,101,34,125,125,125]}

Steps to reproduce

Consumer test :

const provider = new PactV4({
  provider: 'provider',
  consumer: 'consumer',
  dir: path.resolve(process.cwd(), 'pacts'),
  spec: 4,
  logLevel: 'debug',
});

describe('POST /api/test', () => {
  it('send a request', () => {
    const body = { data: { attributes: { name: 'name' } } };
    return provider
      .addInteraction()
      .uponReceiving('json api content type')
      .withRequest('POST', '/api/test', (builder) => {
        builder.headers({
          'Content-Type': 'application/vnd.api+json',
        });
        builder.jsonBody(body);
      })
      .willRespondWith(200, () => {})
      .executeTest(async (mockserver) => {
        const response = await axios.request({
          baseURL: mockserver.url,
          headers: {
            'Content-Type': 'application/vnd.api+json',
          },
          method: 'POST',
          url: `/api/test`,
          data: body,
        });

        expect(response.status).toEqual(200);
      });
  });
});

Provider test:

describe('Order-API contracts in order that', () => {
  it('comply with consumers expectations', () => {
    return new Verifier({
      provider: 'provider',
      providerVersion: '1.0',
      providerBaseUrl: 'http://localhost:8080',
      pactBrokerUrl: getPactBrokerURL(),
      consumerVersionSelectors: [{ branch: 'fix/pact' }],
      publishVerificationResult: true,
      failIfNoPactsFound: false,
      enablePending: true,
      logLevel: 'debug',
    })
      .verifyProvider()
      .then(() => {
        console.log('Successfully verified Pact contracts!');
      });
  });
});

The request handler :

const schema = z.object({
  data: z.object({
    attributes: z.object({
      name: z.string(),
    }),
  }),
});

routes.post('/api/test', async (req, res) => {
  const result = schema.safeParse(req.body);
  if (result.success) {
    res.status(200).send();
  } else {
    res.status(400).send();
  }
});

I have a workaround which is to use the content-type 'application/json' to keep the body as it was defined in the consumer test. I dont know if it should be flagged as a bug, but it's really surprising that the request was not exactly the same on both side of participants.

If I have enough time today I'll use the template to create a repository to reproduce the behavior.

Have a nice day 👋🏽 !

Provide a Minimal, Reproducible Example. You can create your own repository, or use this template that's already hooked up to CI and everything.

Relevant log files

consumer-provider.json provider.log consumer.log

Please ensure you set logging to DEBUG and attach any relevant log files here (or link to a gist).

mefellows commented 5 months ago

This seems like a bug in the core. I'll investigate later and raise an upstream issue if that's the case.

It should be treated as a JSON media type.

Thanks for the detailed report.

ajoanny commented 5 months ago

Thanks a lot, if needed you can reproduce the behavior using this repository https://github.com/ajoanny/pact-issue-1210.

mefellows commented 5 months ago

I think it is bug in Pact JS, as the CLI verifier was correctly returning string data

Likely in the proxy. Adding the custom type in there yields a change from:

[09:46:45.411] DEBUG (37365): pact@12.0.0: incoming request: {"body":{"type":"Buffer","data":[123,34,100,97,116,97,34,58,123,34,97,116,116,114,105,98,117,116,101,115,34,58,123,34,110,97,109,101,34,58,34,110,97,109,101,34,125,125,125]},"headers":{"content-type":"application/vnd.api+json","accept":"*/*","accept-encoding":"gzip, deflate","host":"127.0.0.1:49895","content-length":"39"},"method":"POST","path":"/"}

to

[09:47:25.306] DEBUG (37590): pact@12.0.0: incoming request: {"body":{"data":{"attributes":{"name":"name"}}},"headers":{"content-type":"application/vnd.api+json","accept":"*/*","accept-encoding":"gzip, deflate","host":"127.0.0.1:49901","content-length":"39"},"method":"POST","path":"/"}

I suspect the proxy is not writing the body back correctly, it may be as a result of https://github.com/pact-foundation/pact-js/pull/1199, possibly https://github.com/pact-foundation/pact-js/blob/40c424c5b8ea5f242b2fcec5007a8ce66937dfb1/src/dsl/verifier/proxy/proxyRequest.ts#L25.

mefellows commented 5 months ago

OK, I think the problem is in your express server. At least, the repro can be fixed by changing the express.json() usage to the following:

server.use(express.json({
  type: [
      'application/vnd.api+json', // uncomment to fix
  ],
}));

I've tested this on the latest Pact JS and it seems to work OK.

The bodyParser documentation referenced in express is not entirely clear on the behaviour, but I think the real issue is that +json types are not included by default. See this issue: https://github.com/expressjs/body-parser/issues/519.

ajoanny commented 5 months ago

It works fine ! Thank you 🙏🏽 🙏🏽 🙏🏽 ! I will fix the code on my server. Sorry for the inconvenience.

Have a nice day ! Thanks again for the quick reply !

mefellows commented 5 months ago

No worries!

pact-foundation / pact-js