Add chunked response body fun #439

Merged
merged 1 commit into from Apr 28, 2013

Conversation

Projects
None yet
2 participants
Contributor

fishcakez commented Feb 18, 2013

Extends cowboy:set_resp_body_fun/3 to allow chunked responses in streaming response funs. This is a backwards compatible change. Useful when the body length is unknown and you want to keep the connection alive.

StreamFun = fun(SendChunk) -> lists:foreach(SendChunk, ["ChunkA", "ChunkB"]) end,
Req2 = cowboy_req:set_resp_body_fun(chunked, StreamFun, Req),
cowboy_req:reply(Status, Req2),

The code attached is (currently) a proof of concept with a (passing) testcase.

Owner

essen commented Mar 1, 2013

And now for the hard question: why?

I don't see how this is any different than doing a chunked_reply followed by many chunks. Am I missing something?

Contributor

fishcakez commented Mar 7, 2013

What is the intended way to do a chunked response with a REST handler? Currently you have to use chunked_reply and chunk inside a callback and once done return {halt, Req, State}.

Use case:
Send a body of unknown length and keep the connection open for subsequent requests.

Owner

essen commented Apr 12, 2013

I'm OK in principle but you need to rebase/cleanup (there's a ct:log still) first.

Owner

essen commented Apr 25, 2013

For info: test handlers have moved in the suite's data directory and their name has been shortened.

Contributor

fishcakez commented Apr 25, 2013

Ok, rebased/removed ct:log.

@essen essen and 1 other commented on an outdated diff Apr 25, 2013

src/cowboy_req.erl
@@ -959,6 +967,18 @@ reply(Status, Headers, Body, Req=#http_req{
true -> ok
end,
Req2#http_req{connection=RespConn};
+ {chunked, BodyFun} ->
+ %% We stream the response body in chunks.
+ {RespType, Req2} = chunked_response(Status, Headers, Req),
+ if RespType =/= hook, Method =/= <<"HEAD">> ->
+ ChunkFun = fun(IoData) -> chunk(IoData, Req2) end,
+ BodyFun(ChunkFun),
+ %% Empty chunk signals end of body.
+ _ = chunk(<<>>, Req2),
@essen

essen Apr 25, 2013

Owner

You don't need to do that, Cowboy will do it later.

@fishcakez

fishcakez Apr 25, 2013

Contributor

No it won't, reply/4 sets the resp_state to done, so ensure_response won't finish it off.

@essen essen and 1 other commented on an outdated diff Apr 25, 2013

src/cowboy_req.erl
-> Req when Req::req().
set_resp_body_fun(StreamLen, StreamFun, Req)
when is_integer(StreamLen), is_function(StreamFun) ->
- Req#http_req{resp_body={StreamLen, StreamFun}}.
+ Req#http_req{resp_body={StreamLen, StreamFun}};
+set_resp_body_fun(chunked, StreamFun, Req) ->
@essen

essen Apr 25, 2013

Owner

Missing an is_function.

@fishcakez

fishcakez Apr 25, 2013

Contributor

ok.

Owner

essen commented Apr 25, 2013

Can you explicitly send the empty chunk like in ensure_response then, will be clearer.

Contributor

fishcakez commented Apr 26, 2013

Guilty as charged re your last inline comment ;). I've done as you said and added a test for 1.0.

Owner

essen commented Apr 26, 2013

+-type resp_chunked_fun() :: fun((fun((iodata()) -> ok | {error, atom})) -> ok).
atom()

@fishcakez fishcakez Add chunked response body fun
Adds a new type of streaming response fun. It can be set in a similar
way to a streaming body fun with known length:

Req2 = cowboy_req:set_resp_body_fun(chunked, StreamFun, Req)

The fun, StreamFun, should accept a fun as its single argument. This
fun, ChunkFun, is used to send chunks of iodata:

ok = ChunkFun(IoData)

ChunkFun should not be called with an empty binary or iolist as this
will cause HTTP 1.1 clients to believe the stream is over. The final (0
length) chunk will be sent automatically - even if it has already been
sent - assuming no exception is raised.

Also note that the connection will close after the last chunk for HTTP
1.0 clients.
c8242ab

essen merged commit c8242ab into ninenines:master Apr 28, 2013

Owner

essen commented Apr 28, 2013

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment