Skip to content

Conversation

@frranck
Copy link

@frranck frranck commented Jul 3, 2015

No description provided.

@shivanraptor
Copy link
Collaborator

Increasing timeout duration for each retry is useful, but I think value of 1.5 is a bit too long.
Assume we have 3 retries & the default timeout value 60s is used, the 1st loading timeout is 60s, 2nd retry is 90s and the 3rd retry is 135s. I think no mobile user will wait for more than 2 minutes for timeout.

Since this library is preparing its support for AFNetworking v3, we'll consider merging your codes to v3. Be sure to view the v3support branch for latest updates.

@frranck
Copy link
Author

frranck commented Feb 1, 2016

@shivanraptor: You're right, I changed the default timeout to 15 secs so the 1.5 ratio works well for me. What is still annoying is that it will do 2 useless retries on a 404...

@jold
Copy link
Collaborator

jold commented Feb 1, 2016

@shivanraptor we found 12 seconds as best timeout and my team is using that for very long time. We have also usability study based on that. The 60 seconds is too much to wait for user. Usually he needs to know immediately that server is not responding or in case of slow connection the 30 seconds is the maximum usable waiting time.

I suggest to use 12 or 15 seconds as initial time-out, otherwise the retry does not make sense.

The current retry based on 12 seconds is 12s 1st, 18s 2nd and 27s 3rd.

@jold
Copy link
Collaborator

jold commented Feb 1, 2016

@frranck @shivanraptor just found that we have already added this support 6 days ago parallely :) see line 135 in AFHTTPRequestOperationManager+AutoRetry.m and 142 in AFHTTPSessionManager+AutoRetry.m

@frranck thank you for your contribution.

Now we have to enable user to set the nextRetryTimeoutMultiplier using the property. It should be initialized by default with 1.5f (from predefined constant). @frranck can you contribute on that?

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

Successfully merging this pull request may close these issues.

3 participants