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

HTTP Trailers #230

Closed
wojtekmach opened this issue Aug 28, 2023 · 8 comments · Fixed by #233
Closed

HTTP Trailers #230

wojtekmach opened this issue Aug 28, 2023 · 8 comments · Fixed by #233

Comments

@wojtekmach
Copy link
Owner

wojtekmach commented Aug 28, 2023

Hey @whatyouhide @sneako @mtrudel, I have a favour to ask. I'd like some advice.

I was reading up on HTTP specs. In RFC 9110 section 6.5 we have:

Fields (Section 5) that are located within a "trailer section" are referred to as "trailer fields" (or just "trailers", colloquially). Trailer fields can be useful for supplying message integrity checks, digital signatures, delivery metrics, or post-processing status information.

Trailer fields ought to be processed and stored separately from the fields in the header section to avoid contradicting message semantics known at the time the header section was complete. The presence or absence of certain header fields might impact choices made for the routing or processing of the message as a whole before the trailers are received; those choices cannot be unmade by the later discovery of trailer fields.

RFC 9110 obsoletes RFC 7230 which Mint was referencing in their documentation. So we have clear nomenclature, we have HTTP "fields" which are either "header fields" (colloquially "headers") or "trailer fields" (colloquially "trailers").

I think it would be nice if this standardisation was reflected in libraries though there are of course concerns about backwards compatibility.

Some prior art:

  1. golang net/http.Response has a Trailer field with the same shape and semantics as their Header field for headers.
  2. Fetch API used to have atrailer promise on the response but they have since dropped it Remove trailer support whatwg/fetch#979, apparently due to lack of interest from the browser implementors. Down the stack trailers continue to be an important feature, for example gRPC uses it.

In our ecosystem we could:

  1. Mint.stream instead of returning a second {:headers, headers} response could return {:trailers, trailers}. This would be a breaking change so would have to be opted-in via an option.
  2. %Finch.Response{} could get a new :trailers struct field. Similarly, Finch.stream could call fun.({:trailers, trailers}, acc).
  3. Bandit is stuck with whatever Plug supports. FWIW there was some appetite for trailer support in Plug. I believe the way to support it would be with a new function, perhaps Plug.Conn.send_trailers(conn, trailers) so backwards compatible. We could add a conn.resp_trailers field too.

I'd love some advice whether this is worth pursuing. Mint is obviously post 1.0 but Req and Finch aren't. (but they are about to be!) Trailers seem to be rather rarely used feature so I think we could get away with some breakage in Finch and Req before they are 1.0. I'm happy to do the work on Mint and Finch. FWIW if it doesn't seem to be worth pursuing now, we are not painting ourselves into a corner. That is, Finch and Req continue with the resp.headers field and maaaaybe in the future we add a resp.trailers field an a way to opt-in to send trailers there instead of to the headers.

@wojtekmach wojtekmach changed the title Trailer headers HTTP Trailers Aug 28, 2023
@whatyouhide
Copy link
Contributor

Personally I don't think it's worth changing the name of :headers:trailers in the (post-1.0) Mint response, since the semantics are the same. It's just a "nomenclature" issue (as you put it).

Mint is low level and I don't expect a lot of folks to use it directly. I think documenting it clearly in Mint would be enough. Happy to hear @ericmj's take.

As for Finch/Req, yeah calling them "trailers" sounds good. Thoughts?

@wojtekmach
Copy link
Owner Author

Yeah just doing it in Finch or even just in Req is fine by me. I already go against the grain by introducing headers as maps so people have to adapt to that which is a good opportunity to make other changes like introducing the resp.trailers field. Thanks!

@sneako
Copy link
Contributor

sneako commented Aug 28, 2023

Nice find @wojtekmach, I am willing to have Finch follow the newer RFC and add :trailers as a new field on the response struct.

@wojtekmach
Copy link
Owner Author

Great I’ll send a PR to Finch soon.

@mtrudel
Copy link

mtrudel commented Aug 28, 2023

A path forward for implementing trailers in Plug was discussed here. This is a longstanding nit of mine and I'd be happy to work this to completion if there's interest.

IIRC, the lack of a clear need was the reason against implementing them within Plug previously

Notwithstanding this, I don't believe an implementation of gRPC would be possible given the current Plug API, as was also discussed in the linked issue.

@wojtekmach
Copy link
Owner Author

Thanks @mtrudel. Out of curiosity is there a way with Bandit to temporarily drop down to the socket and send trailers before finishing the req/resp cycle?

@mtrudel
Copy link

mtrudel commented Aug 28, 2023

Out of curiosity is there a way with Bandit to temporarily drop down to the socket and send trailers before finishing the req/resp cycle?

The plumbing is there (I've been expecting to add trailer support since I first built Bandit), but there's no way to expose it at the moment since the public interface into Bandit is Plug; there's definitionally no other aspect to Bandit's interface.

@whatyouhide
Copy link
Contributor

@wojtekmach FWIW, at least documentation now calls them "trailer headers": elixir-mint/mint@2ce7593

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants