rubygems / gemstash

A RubyGems.org cache and private gem server
MIT License
754 stars 121 forks source link

Faraday::ConnectionFailed #79

Closed midwire closed 8 years ago

midwire commented 8 years ago

Howdy, and thanks for this great gem caching service!

Part of the reason I decided to try gemstash is because I'm on a slow T1 connection. So when I try to pull some larger gems I sometimes get:

ERROR - Connection failure - execution expired (Faraday::ConnectionFailed)

It would be great to be able to set an increased timeout value for Faraday. If I get a few mins. I'll drop in a PR here.

Thanks again!

smellsblue commented 8 years ago

Thanks so much for trying it out and the feedback! I'd be happy to merge it in when you have a PR ready! Would it help to get a patch release of the gem, or are you running Gemstash directly from a git clone?

midwire commented 8 years ago

I'm running from a git clone at the moment. Just working on the config setup portion now, as I've already got the patch itself working and passed all specs. Calling the option config[:fetch_timeout], but please let me know if you want to change it, or feel free to.

pcarranza commented 8 years ago

I wonder,

If we catch the error as a timeout and not a general connection error, does it make sense to have some form of exponential retry? that would adjust itself when we have a gem that is too big and we are in a slooooow connection.

Something like leaving the default timeout configured, no idea how much is it, say 5 seconds; catch the timeout error, double the timeout to 10 seconds and retry. If it fails again this second time we could just give up and let the error bubble up or double and try again one more time.

That would be a bit more dynamic and will allow us to not think so much about the connection we are dealing with. And I love not having to think at all.

Just a thought, the PR is a good solution also.

midwire commented 8 years ago

I usually prefer exponential backoff as the solution for things like this. The issue that I foresee is what if the 3 backoffs isn't enough and it still times out? Do we do 4? And then what if someone is on dial-up... LOL. I jest but, also like the ability to just set it and forget it. I could be on the beach somewhere with horrible bandwidth, set it to 999, and have some drinks while I wait for it to cache up 'nokogiri' and all the other gems for my app, and not have to worry about it timing out. :watch:

Re: not having to think about it, we could make it a hidden config option and not ask the question during 'setup'?

I'm willing to try exponential if you'd rather have that. It may take me some time to get to it as I've got to wrap up for tonight. Let me know, and I'll pursue it this coming week.

pcarranza commented 8 years ago

Oh no, I think that having the ability to set it up as a general timeout is great. Do not ditch that.

In a case like this a backoff makes sense to only have a second try, not more than that (just honouring the lazy in me so I don't have to setup anything at all), but that said, simplicity beats complexity.

I was just wondering if that could ever be implemented based on the type of error we get.

midwire commented 8 years ago

Ya, it definitely raises on timeout. We could catch it and do EB. I'll add that as a second try thing, like you are thinking. :+1:

pcarranza commented 8 years ago

@midwire does it make sense to close this one since the branch is merged in?

midwire commented 8 years ago

Yes indeed. Closing.