Empty body should not result in Content-Type header being set

[goce-cz]

Description

Any HTTP method is free to choose whether it sends a payload or not (GET might be debatable, but that's not important). When it sends one, it should declare what type it is, by providing Content-Type header. When it doesn't send one, it MUST NOT declare the header.

The current implementation declares application/json for every call, even GET without specifying any body:

We are currently using a workaround through a middleware:

client.use({
  onRequest: req => {
    if (req.body === null) {
      req.headers.delete('Content-Type')
    }
    return req
  },
})

I know that there are at least two closed issues related to this, but I want to emphasize that this indeed is a bug which makes the library non-compliant with rules of HTTP protocol. I really like the library and I would like to remove this little stain to make it shine even more.

Reproduction

Just fire a POST request against a strict API server, such as Fastify.

await client.POST('/some-endpoint')

and see it respond with 400 Bad Request:

Body cannot be empty when content-type is set to 'application/json'

Fastify rightfully complains about the malformed incoming request.

Expected result

There should be no Content-Type header present, when we're not sending any body - regardless of the HTTP method.

Checklist

• [x] I’m willing to open a PR (see CONTRIBUTING.md)

[eduardohilariodev]

This also makes test fail when mocking a GET request

[drwpow]

I want to emphasize that this indeed is a bug which makes the library non-compliant with rules of HTTP protocol.

Thanks for raising this. Ideally this follows the fetch implementation as closely as possible, so that it has interop everywhere. It’s why despite requests to throw error on non-2XX status codes, this library won’t because that deviates from the spec.

Some QoL improvements were added around the native fetch implementation, of course, but if there’s ever a departure between spec-compliance and opinion, this library will choose spec compliance every time. In that regard, I do think you make a good argument that this is a bug, and not intentional behavior. I don’t think changing this default behavior would significantly negatively impact any consumers, but removing it would yield benefits.

The closed issues you’re referring to hopefully were regarding other aspects of the behavior (but if they made the same point, apologies if I missed something or failed to understand before).

So all that to say I’d welcome a PR to change this behavior, and it’ll get released as a breaking change just for safety.

[drwpow]

Also as a temporary workaround, setting createClient({ headers: { "Content-Type": null } }) should remove any default headers being sent, as an easier alternative to middleware, as a temporary workaround.

[goce-cz]

but if they made the same point, apologies if I missed something or failed to understand before

They never claimed that this is against the HTTP specs, that's why I tried again and I'm happy that successfully.

I’d welcome a PR to change this behavior

Great, give me some time and I'll whip a simple PR when I get some slack.

{ "Content-Type": null }

Sadly this disables the content type header completely, so I still need the middleware to add it when I'm sending a payload - relying on providing it with each request separately kinda goes against the fool-proof mentality of this library.