Add retry attempts to Hackney

benoitc / hackney

simple HTTP client in Erlang

Other

1.34k stars 427 forks source link

Add retry attempts to Hackney #161

Open novabyte opened 9 years ago

novabyte commented 9 years ago

I can't find support for retry attempts in Hackney. If it's not supported I'd like to propose it as a feature enhancement.

benoitc commented 9 years ago

Retrying should be an application logic (since it depends when you want to do it) which is why it is not supported right now in hackney but an explicit mechanism can be added.

I think a retry mechanisms should handled the following cases:

retry on connection error?
retry on POST/PUT error (obviously not when streaming) or only on GET (retry function rule?)

Also if the retry is requested the request should check if it's possible before starting anything (eg. when requested for a stream request).

Did I missed anything?

novabyte commented 9 years ago

Hi @benoitc I was thinking of a simple retry strategy like what's in Fusco: https://github.com/esl/fusco/blob/master/src/fusco.erl#L176

benoitc commented 9 years ago

i wll add these feature in the next release.

novabyte commented 9 years ago

@benoitc Thank you sir :smiley:

novabyte commented 9 years ago

Hi @benoitc have you had time to consider this further? Is it still on the roadmap? Let me know if I can help in some way.

benoitc commented 9 years ago

@novabyte yes it's definitely on the roadmap. I expect to release the 2.0.0 on 06/19 with it.

edgurgel commented 9 years ago

I think had an issue related to this. Making requests with a difference of 5 seconds I would get {error, closed}, but if I waited 6-7 seconds a new connection would be used and less than 5 seconds the same connection is reused. It would be useful to reconnect/retry in this case.

benoitc commented 9 years ago

I am lagging behind my schedule....

@edgurgel mmm not sure to follow. Do you mean that if you're connecting < 5 sec you actually get an {error, closed} error? What is the keepalive of the server?

Anyway the coming new connection pool should address this issue. I expect now to make a release before the end of the month with it now.

edgurgel commented 9 years ago

@benoitc , yeah I forgot to add that the server will keep-alive for exactly 5 seconds. I get a {error, closed} when the time between two requests is too close to 5. If it's 4 or 6 it's fine to reestablish the connection or reuse the previous.

flupke commented 9 years ago

I implemented retries in my download code, it's fairly complete (backoff factor + number of retries, or user-defined function), feel free to use it if you want: https://github.com/flupke/hackney_message_bug/blob/master/lib/hackney_message_bug.ex

edevil commented 8 years ago

@benoitc Did this feature ever get implemented? It seems it wasn't since the issue is open, but @edgurgel is referring to a bug introduced by it?

In another note, are there plans to include a feature like this?

benoitc commented 8 years ago

it's not implemented yet :) but any patch doing that is welcome :)