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

Add more HTTP/1.1 timeout options (only idle_timeout missing in cowboy_http2) #1032

Closed
rossilazuardi opened this issue Oct 11, 2016 · 19 comments
Labels

Comments

@rossilazuardi
Copy link

Hi ..
I try eventsource example, and experiment trying to change value of timeout, and seems it doesn't work. Server still close connection every 5 seconds.

init(Req0, Opts) ->
    Req = cowboy_req:stream_reply(200, #{
        <<"content-type">> => <<"text/event-stream">>
    }, Req0),
    erlang:send_after(1000, self(), {message, "Tick"}),
    {cowboy_loop, Req, Opts, 5000}.
@essen
Copy link
Member

essen commented Oct 11, 2016

If the client does not send any data until timeout, then it is normal that the connection gets closed.

Note that due to underlying changes in master, the current code does not perform the detection of data sent by the client, and therefore the timeout is always triggered.

@rossilazuardi
Copy link
Author

Yes.. I have no issue with close connection if client not send any data,
but I need a little bit more time for the timeout.., I've been use cowboy v.1xx for sse, and it works with the timeout

{loop, Req4, undefined, 60*1000} 

@essen
Copy link
Member

essen commented Oct 11, 2016

Is the issue that if you set, say, 60s, that Cowboy closes the connection after 5s?

@rossilazuardi
Copy link
Author

Yes

@essen
Copy link
Member

essen commented Oct 11, 2016

Ah understood. I'll have to take a look.

@essen essen added the Bug label Oct 11, 2016
@Littlelies
Copy link

Maybe I misunderstood the issue, but I tried:

    cowboy:start_clear(http_listener, 100,
        [{port, 8080}],
        #{env => #{dispatch => Dispatch}, request_timeout => 60000}
    ).

And it worked using master, rev c528d9b.

@essen
Copy link
Member

essen commented Dec 30, 2016

This one is about loop handlers which have a different kind of timeout, that's not working on current master.

@essen essen added this to the 2.0 Breaking Changes milestone Feb 4, 2017
@essen
Copy link
Member

essen commented Feb 17, 2017

OK so the old loop handler option is not needed anymore. HTTP/2 has its own timeout mechanism so it's perfectly fine to have a loop handler running forever (until the connection closes).

We already have a timeout in HTTP/1.1 saying how long we have to wait between requests. But it's not working properly (it's closing the connection despite a request ongoing). What we will however need is a separate timeout in HTTP/1.1 to close the connection on client inactivity when there is a request ongoing.

@essen essen changed the title Set a timeout value in loop handlers not working Add more HTTP/1.1 timeout options Feb 18, 2017
benoitc added a commit to barrel-db/barrel-platform that referenced this issue Feb 23, 2017
Cowboy 2 timeout if nothing is done in 5s dor now. Let set a larger timeout waiting for a proper fix cowboy.

related issue:
ninenines/cowboy#1032 (comment)
@StartSWest
Copy link

StartSWest commented Apr 10, 2017

Well if loop the handler is a gen_server it has its default time out that can be configured in Erlang system, and I don't remember well but they can be also set to infinity in gen_server call arguments.

@bullno1
Copy link

bullno1 commented Apr 19, 2017

I'm bitten by this bug. I don't know enough about cowboy's internal but what do you think about an internal command like cancel_timeout that cowboy_loop can send to the connection process to cancel the request timeout? cowboy_websocket uses switch_protocol which achieves something similar in addition to switching protocol handler.

It's not very elegant but it's a quick fix for cowboy_loop.

@essen
Copy link
Member

essen commented Apr 19, 2017

Normal fix is simpler, just need to enable this timeout when there are no requests currently running.

@essen
Copy link
Member

essen commented May 3, 2017

I have a patch fixing request_timeout and adding idle_timeout that I will push soon after I investigate why the process may not always exit (#1156).

@essen
Copy link
Member

essen commented May 5, 2017

We now have request_timeout, idle_timeout and inactivity_timeout for HTTP/1.1. HTTP/2 is still missing idle_timeout.

@mm-tfx
Copy link

mm-tfx commented Jun 9, 2017

I am trying to understand SSE and Cowboy here.
My SSE handler would like to send 100 chunks. 1 chunk every second.
But the connection will be closed after 60 seconds (value of this idle_timeout).
I can increase this value but I am not sure if this is a way.
Considering SSE is a connection idle really? I mean after one request from the client, Cowboy produces data. Perhaps I am missing something from SSE principles.

Thanks.

@essen
Copy link
Member

essen commented Jun 9, 2017

You will need to tweak the timeout value for HTTP/1.1. It's not a problem for HTTP/2 as long as the client sends pings. The problem is that we can't reliably detect TCP connections being closed, so we need to rely on the data coming in + a timeout. If no data comes in (SSE) then we just close on timeout. Best to make sure your client reconnects.

@mm-tfx
Copy link

mm-tfx commented Jun 9, 2017

As usual I am using couple tools to test this. Web browsers have reconnect support implemented and I can even control reconnect time from SSE (retry parameter). Curl does not work so well here for me. Gun is really helpful as usual and I guess I would have to add some code for Gun to make it reconnect and listen again to SSE.
I cannot however let any SSE event be dropped so I would have to probably buffer some data somewhere.

One last question:
By writing "if no data comes in (SSE)" did you mean no data coming from the client/browser?

Thanks.

@essen
Copy link
Member

essen commented Jun 9, 2017

Yes, client.

About not losing any events, that's why you have the Last-Event-ID header. When you reconnect you are supposed to put the last known ID there and the server resumes from that point. If you can't allow anything to be dropped you'll most likely need a complex solution involving storing these events to disk somewhere so you can replay if the client or the node goes down.

@ninenines ninenines deleted a comment from mohammedmazharuddin Sep 14, 2017
@essen essen removed this from the 2.0 Breaking Changes milestone Oct 2, 2017
@essen
Copy link
Member

essen commented Nov 15, 2018

FYI: 2.6 will include a way to increase the idle_timeout of HTTP/1.1 connections on a per-request basis. See #1292 (comment)

@essen essen changed the title Add more HTTP/1.1 timeout options Add more HTTP/1.1 timeout options (only idle_timeout missing in cowboy_http2) Nov 15, 2018
@essen
Copy link
Member

essen commented Nov 16, 2018

I've now added the idle_timeout option to HTTP/2 as well and it'll be part of 2.6 to be released next week. Closing, thanks!

@essen essen closed this as completed Nov 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants