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

V2RequestBuilder methods adding Content-Type headers by default #263

Open
will-banked opened this issue Jan 24, 2023 · 4 comments
Open

V2RequestBuilder methods adding Content-Type headers by default #263

will-banked opened this issue Jan 24, 2023 · 4 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@will-banked
Copy link

Software versions

  • OS: macOS 13.1
  • Consumer Pact library: e.g. pact-go
  • Provider Pact library: e.g. v2.0.0-beta.17.0.20230119022944-5eda04090104
  • Golang Version: go1.19.4 darwin/arm64
  • Golang environment: Provide output of go env

Actual behaviour

Pact .withRequest() seems to always add a Content-Type to requests, even if you don't specify one. Example:

WithRequest("POST", "/internal/v1/payouts", func(b *consumer.V2RequestBuilder) {
	b.BinaryBody(createPayoutBytes)
})

In the logs for the actual request, it's now added on an expected Content-Type header, even though I've not specified it in code.

2023-01-24T14:24:49.940971Z  INFO tokio-runtime-worker pact_mock_server::hyper_server: Received request HTTP Request ( method: POST, path: /internal/v1/payouts, query: None, headers: Some({"host": ["127.0.0.1:55532"], "x-b3-sampled": ["0"], "content-length": ["322"], "accept-encoding": ["gzip"], "user-agent": ["Go-http-client/1.1"]}), body: Present(322 bytes) )

2023-01-24T14:24:49.941956Z  INFO tokio-runtime-worker pact_matching: comparing to expected HTTP Request ( method: POST, path: /internal/v1/payouts, query: None, headers: Some({"Content-Type": ["application/octet-stream"]}), body: Present(322 bytes) )

I've used the BinaryBody() function, but I've also seen the same for BodyMatch() where I've passed in an empty struct, it also added the same Content-Type expected header.

Expected behaviour

I'd expect to be required to specify the any headers myself.

Steps to reproduce

I can provide a repo if needed, but should be pretty straightforward to update the rawTest() function to remove setting the header for Content-Type, and then to see that the tests fail because Pact just by default expects that header to exist.

Relevent log files

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

@mefellows
Copy link
Member

Thanks for raising.

The underlying core function requires a content-type to be set at the time of the body, so we have a few choices (keen for your thoughts) but I think the best is f or BinaryBody to take a new first argument specifying the content type (first because it's consistent with the other methods that also accept that). This is a breaking change, but I think now would be the time to do it before it's out of beta

FWIW I believe you can also separately set the header afterwards, and it will override what the framework defaulted it to anyway. But this is not resilient. That is if you do b.Header("content-type", "...") and then b.BinaryBody(...) the second call would override it.

@mefellows mefellows added the bug Indicates an unexpected problem or unintended behavior label Jan 25, 2023
@will-banked
Copy link
Author

Hmmm interesting, so in my case above, the consumer actually doesn't send Content-Type headers in the request. Is there anything that would cover for this scenario?

@mefellows
Copy link
Member

@uglyog thoughts on the above?

@rholshausen
Copy link
Contributor

Looks like we need a mechanism to not set the content type header, or to remove it afterwards

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