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/2 ability to send few parts of headers in the same stream. #1017

Closed
IgorKarymov opened this issue Aug 21, 2016 · 12 comments
Closed

HTTP/2 ability to send few parts of headers in the same stream. #1017

IgorKarymov opened this issue Aug 21, 2016 · 12 comments

Comments

@IgorKarymov
Copy link

Hello essen! My best regards to you and your awesome cowboy!
I playing with the master version in an attempt to create google backed RPC server implementation.
I was able to receive request but faced with issue when trying to format reply, - google invent pretty tricky, but I believe correct way to frame replies:
http://www.grpc.io/docs/guides/wire.html#example
Can you advise me some way to send a similar sequence from "HEADERS BODY HEADERS" to the client with the current interface? I tried the combination of cowboy_req:stream_reply/3 :stream_body/3 but looks like it's explicitly not allowed to call stream_reply/3 more than once here:
https://github.com/ninenines/cowboy/blob/master/src/cowboy_req.erl#L633

@essen
Copy link
Member

essen commented Aug 21, 2016

Those are trailer headers, something not specific to HTTP/2 that Cowboy never supported so far. But we definitely should. I will have to think a bit about the interface, probably something like a stream_trailers function that always terminates the body streaming (and so must be used last).

@IgorKarymov
Copy link
Author

I have a set of different clients implementation, with implemented grpc support (unfortunately no erlang client because a lack of trustable http2 clients), so I can help with initial manual testing of this feature.

@essen
Copy link
Member

essen commented Aug 22, 2016

Here's what I believe we need:

  • A stream_trailers function that will send trailer headers and terminate sending the body (you never call stream_body with the 'fin' flag)
  • The function is basically ignored if the protocol used, server configuration or request headers (lacking TE: trailers when HTTP/1.1) prevents us from sending trailers
  • A well documented checklist of "why my client doesn't receive trailers"

We ignore instead of crashing because it's not possible to ensure trailers ever reach the client. For example a proxy could remove them.

In your case HTTP/2 would always allow and send trailers so it would work it nicely I think.

Thoughts?

@IgorKarymov
Copy link
Author

It sounds reasonable. Few additional things:

  • Trailers support for simple non-streaming API responses with cowboy_req:reply ?
  • looks like trailers support for not HTTP/2 endpoints restricted to HTTP/1 "Chunked Transfer Coding" https://www.w3.org/Protocols/rfc2616/rfc2616-sec3.html#sec3.6.1
  • To be honest, I didn't saw HTTP/1 "Chunked Transfer Coding" trailers in the wild. They can be usable in theory (calculating ETAG on the fly, the indication of the error in the middle of chunked transfer...), but I personally don't know any API that relies on trailers support (some of them reinvented custom solutions to achieve similar functionality. Probably because there are no clients that support "trailers" (including major browsers).

@essen
Copy link
Member

essen commented Aug 23, 2016

Nothing should rely on trailers, just about anything could discard them on the fly. This is also true for HTTP/2 during conversion from HTTP/2 to HTTP/1.1. Still, they're a part of the spec, so it doesn't hurt to implement it.

Trailers make no sense in cowboy_req:reply. Trailers are meant to be "headers we didn't know when sending the initial response" and when they are received, they are simply added to the list of headers. They have the same semantics, their sending is just delayed. Therefore if using the normal reply you should just send them in headers.

@IgorKarymov
Copy link
Author

Agree about cowboy_req:reply semantic. This case will make sense only for grpc, just because I will have a "trailers" (rpc status code) exactly at the same moment when I will have a body. So it probably not makes sense to change generic interface because of this (I can easily write my own wrapper).
For http/1.1 we have to check "Chunked Transfer Coding" + TE header in the request.
Can I send PR? Or you want to implement it by yourself?

@ferd
Copy link
Contributor

ferd commented Aug 23, 2016

I've seen one use of trailers in the wild with HTTP/1.1, as a checksum for the transferred chunked body appended after the transfer as part of some specific protocol built on top of HTTP. Never seen any other of the apps on our platform rely on it. YMMV.

@essen
Copy link
Member

essen commented Aug 24, 2016

Thanks for the feedback. One is actually more than I expected, to be honest. :-)

A PR is welcome of course but it needs tests in the req_SUITE. I haven't updated this part of the documentation yet so I can do that later.

@IgorKarymov
Copy link
Author

Just another API idea to discuss:
Can we remove isfin flag from stream_body and introduce instead something like:

-spec finish_stream(iodata() | http_headers(), req()) -> ok.

Not sure about function name, a little bit out of name system with stream_reply (start_stream?), and stream_body (continue_stream?).

@essen
Copy link
Member

essen commented Aug 24, 2016

I don't really want to have a function take two completely different inputs.

@IgorKarymov
Copy link
Author

Ok, let's stick to stream_trailers variant.

@IgorKarymov
Copy link
Author

superseded by #1020

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

No branches or pull requests

3 participants