Stream output is not working on master

[sigsergv]

How to reproduce: create sample project with controller like this:

-module(rahm_greeting_controller, [Req]).
-compile(export_all).

hello('GET', []) ->

    V = "/etc/passwd",

    case file:open(V, [read]) of
        {ok, Fo} ->
            Generator = fun(Fp) ->
                case file:read(Fp, 100) of
                    eof -> file:close(Fp), done;
                    {ok, Data} -> {output, Data, Fp}
                end
            end,
            {stream, Generator, Fo, [
                 {"Content-Type", "text/plain"}
            ]};
        _ ->
            {output, "404 Not Found"}
    end.

It fails in cowboy/mochiweb backends with different reasons:

mochiweb

10:20:13.482 [error] error:undef: [{ok,build_response,[],[]},{boss_simple_bridge_handler,run,1,[{file,"src/boss/boss_simple_bridge_handler.erl"},{line,27}]},{mochiweb_simple_bridge_anchor,loop,1,[{file,"src/mochiweb_bridge_modules/mochiweb_simple_bridge_anchor.erl"},{line,27}]},{mochiweb_http,headers,6,[{file,"src/mochiweb_http.erl"},{line,122}]},{proc_lib,init_p_do_apply,3,[{file,"proc_lib.erl"},{line,247}]}]
10:20:13.482 [error] Error: exit:"Error building response"

cowboy

10:23:24.560 [error] Unhandled Error: error:function_clause Stacktrace: [{gen_tcp,send,[undefined,
              [["64",13,10],
               "root:x:0:0:root:/root:/bin/bash\ndaemon:x:1:1:daemon:/usr/sbin:/usr/sbin/nologin\nbin:x:2:2:bin:/bin:/",
               <<"\r\n">>]],[{file,"gen_tcp.erl"},{line,265}]},{boss_web_controller_handle_request,process_stream_generator,5,[{file,"src/boss/boss_web_controller_handle_request.erl"},{line,201}]},{boss_web_controller_handle_request,handle_request,2,[{file,"src/boss/boss_web_controller_handle_request.erl"},{line,35}]},{boss_simple_bridge_handler,run,1,[{file,"src/boss/boss_simple_bridge_handler.erl"},{line,26}]},{cowboy_simple_bridge_anchor,handle,2,[{file,"src/cowboy_bridge_modules/cowboy_simple_bridge_anchor.erl"},{line,34}]},{cowboy_handler,handler_handle,4,[{file,"src/cowboy_handler.erl"},{line,111}]},{cowboy_protocol,execute,4,[{file,"src/cowboy_protocol.erl"},{line,442}]}]

[brigadier]

I confirm

[brigadier]

The problem is in this call: mochiweb_socket:send(Req:socket(), Chunk) in the boss_web_controller_handle_request.erl

Implementation of the socket() function in cowboy_simple_bridge.erl:

socket(_ReqKey) ->
    undefined.

[sigsergv]

Also this code fragment looks suspicious: https://github.com/ChicagoBoss/ChicagoBoss/blob/master/src/boss/boss_web_controller_handle_request.erl#L172

handle_response(Bridge, _Payload = {stream, Generator, Acc0}, RequestMethod) ->
    Protocol        = Bridge:protocol_version(),
    TransferEncoding    = handle_protocol(Protocol),
    Response3        = Bridge:set_response_data(chunked),
    Response3,
    process_stream_generator(Bridge, TransferEncoding, RequestMethod, Generator, Acc0);
handle_response(Bridge, Payload , _RequestMethod) ->
    Bridge:set_response_data(Payload).

[choptastic]

Minor correction, but set_response_data/1 has always actually been a /2
due to the pmod syntax.

Bridge:set_response_data(Data) is exactly the same as
sbw:set_response_data(Data, Bridge) - the former is just a hack on the
language that previously relied on native pmods (then pmods via parse
transform), but is now just explicitly spelled out.

However, it is backwards compatible (Bridge:set_response_data(Data) is
still valid and works).

That said, I'm open to some some ideas to make it work better with Cowboy.

I have an undocumented hack in the cowboy bridge to support streaming, if
you use Bridge:set_response_data({stream, Size, StreamFun}), where
StreamFun is a function that mimics sendfile (
https://github.com/nitrogen/simple_bridge/blob/master/src/cowboy_bridge_modules/cowboy_simple_bridge.erl#L215-L226
)

That functionality currently only works with Cowboy and simple_bridge
(hence being undocumented), and if folks like it, I'd be happy to try to
extend it to the rest of the bridges.

On Thu, Sep 1, 2016 at 8:37 AM, Evgeny M. notifications@github.com wrote:

set_response_data/1 also doen't exist any more. It is now /2.
Looks like anything in this function was written for some very old version
of simple_bridge. 2013 year or before.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#655 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AALYnCUvyBBVVRyjVH71oB8hMi4Xa3GAks5qltUogaJpZM4JsqMf
.

Jesse Gumm
Owner, Sigma Star Systems
414.940.4866 || sigma-star.com || @jessegumm

[brigadier]

yeah i already realised that /1 and /2 are related to pmod, but deleted the message too late :)
Tbh I don't like Size requirement in set_response_data.

[choptastic]

I don't like the Size requirement either, and I believe it used to be part of the cowboy specs, but it seems to be fixed in 1.1.x (https://github.com/ninenines/cowboy/blob/1.1.x/src/cowboy_rest.erl#L863-L871)

I'll have to tweak it accordingly to allow {steam, StreamFun} as well as {chunked, StreamFun}

Or, as always, I'm open to pull requests :)

[brigadier]

The code below in boss_web_controller_handle_request makes chunked streaming work, for cowboy only:

handle_response(Bridge, _Payload = {stream, Generator, Acc0}, RequestMethod) ->
    Bridge:set_response_data({chunked,
                              fun(SendChunked) ->
                                    process_stream_generator(SendChunked, RequestMethod, Generator, Acc0)
                              end
                             });

process_stream_generator(_SendChunked, 'HEAD', _Generator, _Acc) ->
    ok;
process_stream_generator(SendChunked, Method, Generator, Acc) ->
    case Generator(Acc) of
        {output, Data, Acc1} ->
            SendChunked(Data),
            process_stream_generator(SendChunked, Method, Generator, Acc1);
        done -> ok
    end.

Requires this nitrogen/simple_bridge#59 pull for simple_bridge and cowboy 1.1.x (currently 1.0.3 in rebar.config)

[sigsergv]

The code below in boss_web_controller_handle_request makes chunked streaming work, for cowboy only

It doesn't work unfortunately: server returns chunks without headers and then throws Bad value on output port 'tcp_inet'.

updated Aww, forget about this, it was my fault — passed Content-length as integer and got a bunch of weird errors. It's working fine now