Raise timeout for Fetcher to work on slow internet connections

[small1]

The timeout in the appstore fetcher is to low. So i raised it to 30 seconds.

Closes #14926

[kesselb]

Thank you and welcome to the club 🎉

Looks good to me because the result is cached. We could consider to increase the lifetime of the cached value to a higher value.

server/lib/private/App/AppStore/Fetcher/Fetcher.php

Line 42 in 3a75ea7

const INVALIDATE_AFTER_SECONDS = 300;

Would you mind to sign off your commits? https://github.com/nextcloud/server/pull/16149/checks?check_run_id=157989419

[small1]

Thank you and welcome to the club

Looks good to me because the result is cached. We could consider to increase the lifetime of the cached value to a higher value.

server/lib/private/App/AppStore/Fetcher/Fetcher.php

Line 42 in 3a75ea7

const INVALIDATE_AFTER_SECONDS = 300;

Would you mind to sign off your commits? https://github.com/nextcloud/server/pull/16149/checks?check_run_id=157989419

I thought i did that ... seems like i did not do the amend right in some way.

[small1]

I'm going to stop and try to fix that mistake. It was to long since i used git :P

[kesselb]

I'm going to stop and try to fix that mistake. It was to long since i used git :P

https://github.com/nextcloud/server/pull/16149/checks?check_run_id=158029855 it should be enough to follow this two commands.

[jancborchardt]

The commits should probably be squashed before merging. Not sure if we do this automatically or there’s a command for it @kesselb @rullzer?

[small1]

The commits should probably be squashed before merging. Not sure if we do this automatically or there’s a command for it @kesselb @rullzer?

@jancborchardt now i think i got my git commands in the right order. I did remove all the merging stuff that should not be in the commit and squashed my own.

[MorrisJobke]

Also the 30 seconds there has some side effects: most web servers have a default timeout of 30 seconds - so if the timeout hits in it is 30s + the request timeout. Which then is over 30s and thus the response is always a 50x (most likely bad gateway) error. Thus we set this to 10 seconds. The better approach would be to fetch this in a background job so it is not bound to a user facing request and also makes the app management a lot more snappy.

[small1]

Also the 30 seconds there has some side effects: most web servers have a default timeout of 30 seconds - so if the timeout hits in it is 30s + the request timeout. Which then is over 30s and thus the response is always a 50x (most likely bad gateway) error. Thus we set this to 10 seconds. The better approach would be to fetch this in a background job so it is not bound to a user facing request and also makes the app management a lot more snappy.

php has an execution timeout of 30 seconds as default yes that can be changed with set_time_limit to increase with a few seconds if needed. Apache and nginx has a timeout of requests for 60 seconds if im not mistaken so setting this would not really be a problem. I never tested with a smaller timeout than 30 for my clients that are on slow connections. But perhaps 20 or 25 would work as well. 10 is to low.

But yes the better way would be to have a background job for this. It would make everything work much smoother. But also a bigger rewrite.

[MorrisJobke]

php has an execution timeout of 30 seconds as default yes that can be changed with set_time_limit to increase with a few seconds if needed. Apache and nginx has a timeout of requests for 60 seconds if im not mistaken so setting this would not really be a problem. I never tested with a smaller timeout than 30 for my clients that are on slow connections. But perhaps 20 or 25 would work as well. 10 is to low.

Yep something below 30 is fine, but 30 is extremely bad, because it interferes with the default setup (and to be honest a lot of instances out there run with a default setup ;)).

I still fear, that the app management is then super slow to use and just gives a bad user experience. To really tackle this we should fix it properly and not with yet another workaround. Sorry for the bad news here. You could use your patch in the meantime to avoid issues on your instance, but we have thousands of other instances out there about which we also need to think.

[small1]

Yeah breaking stuff is not the motive here. Only fixing broken stuff. What it says right now is that an app cant be downloaded. Or you get an empty app store in some cases.

But i agree. Breaking installations is not an option. AS this is a small patch i can manage to work around with fixing this manually.

[small1]

And actually when i tinkered around now ani found that sometimes the problem occurs in guzzles curl factory. So more than this was needed anyway.

[small1]

I'm closing this.

[MorrisJobke]

Thanks for your help anyways 👍

[andrewperry]

This suggests that the Nextcloud team may be able to help resolve the underlying issue by increasing the responsiveness of the apps server: https://developers.google.com/speed/pagespeed/insights/?url=https%3A%2F%2Fapps.nextcloud.com%2Fapi%2Fv1%2Fapps.json

[fwolfst]

@kesselb I understand you locking #14926, but maybe you could include this one-liner in the "solution". I will apply it after every update and it might be worth sharing for people with automated setups.

sed -i "s/'timeout' => 10,/'timeout' => 90/" lib/private/App/AppStore/Fetcher/Fetcher.php #as of NC 16.0.6