`Req.Request`, `Req.Response`: Make headers opaque

[wojtekmach]

In Req v0.4 I changed headers from the usual lists of tuples to map of lists.

In hindsight it probably wasn't worth it since every other client, as well as Plug, uses lists of tuples. My rationale was describe here https://hexdocs.pm/req/changelog.html#change-headers-to-be-maps and I think the change does not really live up to the expectations. I suppose pattern matching is kinda nice but headers[name] is still unergonomic because it returns [binary], what we IMHO really want is headers[name] :: String.t | nil (give me first header value or nil). If, which is a big IF (though probably a NO), Access is ever extended to lists of two element tuples, resp.headers["content-type"] #=> "text/plain" | nil would JUST WORK though there are considerations with that mentioned next.

As an aside, I'm planning to add Req.get_header(req | resp, name) :: String.t | nil and Req.get_header_values(req | resp, name) :: [binary()]. The former would be used vast majority of the time, the latter would be for the odd Set-Cookie, see https://www.rfc-editor.org/rfc/rfc9110.html#section-5.3-4.1.

What we additionally did is, per RFC 9110, made headers access case-insensitive, all the accessor functions like Req.Request.put_header, Req.Request.get_header (or the new Req.*header* functions mentioned earlier), etc, downcase under the hood. I think this might be another reason to make headers opaque because otherwise when people access them, they don't necessarily preserve the case insensitiveness.

Making headers opaque would allow us to change the internal representation and frankly it probably would go back to [{binary(), binary()}] which would avoid conversions back'n'forth. (It would be my personal L too but that wouldn't be the first one.) A potentially interesting approach is for well known header names, https://www.rfc-editor.org/rfc/rfc9110.html#section-18.4, represent them as atoms which I believe would use less memory and result in faster comparisons and all the other ones use string name for. And then API functions would use atoms and strings. Such design, if it turned out to be worth it performance wise, would only be sensible with opaque type and forcing people to use the public API.

With opaque type we'd forego pattern matching but I think that's OK after all.

If we do this we should also create functions like Req.get_headers(req | req) :: [{binary, binary}] which would serve as compatibility with the rest of the ecosystem which would again admit the whole headers maps was a mistake.

[wojtekmach]

Btw since Req does downcasing anyway I can probably use elixir-mint/mint#399 to avoid yet another downcasing but I haven't thought about the implications.

[wojtekmach]

If we make this opaque now we could in the future even add some kind of %Req.Headers{} that implements Access handling case insensitive keys which would make the API even simpler, eg:

resp.headers["content-type"]

put_in(req.headers["authorization"], "bearer secret")

But that’d be again diverging from Plug and friends. But I think it has some things going for it.

[Shakadak]

If we make this opaque now we could in the future even add some kind of %Req.Headers{} that implements Access handling case insensitive keys which would make the API even simpler, eg:

resp.headers["content-type"]

put_in(req.headers["authorization"], "bearer secret")

But that’d be again diverging from Plug and friends. But I think it has some things going for it.

Why not use a struct was my original question when I first read the Headers section on hexdocs [1]. Especially because of that part:

While we can ensure case-insensitive handling in the former case, we can't in the latter.

Obviously, I'm lacking the reasoning you used to go first for a list of tuples, then to a map. And what are the implications of any solution. But I've found this issue through link surfing and I'm interested in this reflection. I've read the rationale you linked about the change from list to map. So here are my gathering:

• Other libraries have converged to a list of tuple. Probably as an heritage from Erlang libraries.
• Using a map instead opened the possibility to use the access syntax.
• Using a map also allowed to use pattern matching.
• When using a map, only dedicated access functions allows working with headers in a case insensitive manner.
• Using a struct would allow to use the access syntax (and the *_in operators) with insensitivity.
• Using a struct would allow using Enum:
  • Enum.to_list/1 would remove the need for Req.get_headers_list/1.
  • Enum.member? wouldn't be really helpful.
  • Enum.find* could be an awkward substitute for List.keyfind, but we already have the access syntax anyway.
• Using a struct loses the ability to use pattern matching.
  • Personally I think this is only helpful when we want to check for the presence of two or more interdependent headers and get their values. (We do sort of have that use case at work, but that's part of a plug, so serving HTTP requests instead of dealing with a request/response workflow. Here it's okay to have more verbosity.)

I think having a clear idea about how to handle duplicate headers should be the crux of the problem. Everything else is pretty much reduced to using a key-value container. If duplicate headers are considered rare but expected when they happen, I think having a dedicated function for this case (like you proposed) and the access syntax for the rest of the time is the better compromise. In that case, using the access syntax on duplicate headers should return the values as comma separated.

Aside from that, from experience at work, dealing with mixed type keys in key-value containers has always been a problem long term, so I kind of have some reserve regarding giving special cases to known headers.

[1]: https://hexdocs.pm/req/Req.html#module-headers

PS: I wouldn't talk about mistakes, but rather exploring the possibilities :P Maps didn't come that early in the history of Elixir, and even less so in Erlang's one ^^