fix(json): do not override custom content-type request header

[arcanemachine]

Preamble

I am working with Elasticsearch, which uses a bulk endpoint that uses NDJSON, and expects a content-type: application/x-ndjson header.

However, Tesla's JSON encoder middleware does not allow the header to be overridden, so I have to use workarounds to set the header when sending the request:

defp client(opts \\ []) do
  request_content_type = Keyword.get(opts, :request_content_type, "application/json")

  base_headers = [{"accept", "application/json"}]

  # Set custom headers and middleware based on the payload type. (This is a workaround to fix
  # Tesla's lack of support for Elasticsearch's bulk NDJSON endpoint)
  {headers, tesla_json_middleware} =
    case request_content_type do
      "application/json" ->
        {base_headers, {Tesla.Middleware.JSON, engine: JSON}}

      "application/x-ndjson" ->
        headers = base_headers ++ [{"content-type", "application/x-ndjson"}]

        {headers, {Tesla.Middleware.DecodeJson, engine: JSON}}
    end

  middleware =
    [
      {Tesla.Middleware.BaseUrl, base_url},
      {Tesla.Middleware.Headers, headers},
      tesla_json_middleware,
      # Other middleware excluded for brevity...
    ]

  Tesla.client(middleware)
end

Solution

This pull request resolves the issue by allowing the content-type header to be overridden, but falls back to the default application/json if no custom content-type header is present in the request.

This allows me to remove my workaround logic and just use plain old Tesla.Middleware.JSON in my client with the custom header that is required by Elasticsearch.

[arcanemachine]

I think the "Semantic PR Title" check failed... I believe my PR title is in line with the requirements.

Error message:

Error: Resource not accessible by integration

[yordis]

is the downcase actually required? under what conditions (adapter specific I am guessing)?

[yordis]

should we use Tesla.get_header?

[arcanemachine]

Re: Downcase - I figured that would be best in the event that some misconfigured server demands Content-Type or similar instead of complying with the (case-insensitive) spec.

Re: Use Tesla.get_header - I was not aware of that function. To resolve the issue an idiomatic manner, I have force-pushed a change to my branch to use that function instead.

The reason I did that extra downcasing work in my initial commit was to accommodate a future situation where poorly-written server demands e.g. a case-sensitive Content-Type header instead of complying with the spec. For my needs, though, Tesla.get_header/2 works fine.

[yordis]

Should this be something explicit as part of the middleware?

{Tesla.Middleware.JSON, content_type: "application/x-ndjson"}

[arcanemachine]

I just realized I had a total XY problem issue.

I was having issues with the headers, but my solution fixed the problem for the wrong reason.

The JSON middleware already exposes options to set the request headers, which weren't working because my payload was technically a binary (NDJSON is a list of JSON objects, so not "encodable" per JSON middleware).

Long story short: My PR is not needed, I was able to set the headers correctly, and was getting errors from another endpoint which was misconfigured and causing errors for an unrelated reason, which I thought was related to the same issue, but was actually something else.

Sorry for wasting your time. 😅