Update status module can cause white screen if the check for updates takes too long

#232041: Fatal error: Maximum execution time of 60 seconds exceeded (when fetching update status data)
there's a patch there that needs review. you can apply it to the D6 version of update.fetch.inc and it should work. Please report back to the other issue if so.

I don't think having the page hang for over half an hour is an *ideal* solution...

Also strictly speaking these are separate projects now...

Good points, thanks. ;) See #232041-5: Fatal error: Maximum execution time of 60 seconds exceeded (when fetching update status data)

Marked a few newer issues duplicate with this:
#414820: Unavailable drupal.org leads to crashed cron
#416066: Can't access Administer section while drupal.org is down

With today's d.o outage, I was only able to repro this on brand new 6.x sites where cron hadn't ever been run. The two 6.x sites I tested where cron had been run worked fine, even with update status on.

This is a minor issue now with D6.11+, but here is a patch to tell update status to give up if it fails to fetch anything for two modules in a row-- this prevents hanging / timeout / whitescreen when your server or d.o are not connected.

^{benjamin, Agaric Design Collective}

This is still an important issue, for example, when *.d.o is down for maintenance. Looks like the behavior of drupal_http_request() changed a few times over at #245990: HTTP request status fails, which is why we need a patch like this now. Mostly, this looks good. However, a few problems I ran into while testing it...

I'm not sure how to handle partial results. For example, if your net connection or updates.d.o is being flakey, you might get a handful that work while the rest fail. Also, not all projects necessarily use the same URL to fetch updates (a project might be hosted off d.o and set the 'project status url' field in its .info files). So, if you have a project on your site pointing to another update status server, and updates.d.o is down but the other server is up, you're only going to get data for that other server (assuming you were lucky and your other project was early in the list and $fail didn't abort the fetch since it's global). Or, if that server is down and updates.d.o is up, vice versa.

Currently with this patch, we save any partial results we get, display those, and grey-out the rest (see attached "partial-results" screenie). To generate that, I put this in settings.php:

And I put this in my codefilter.info file:

Note, that only works since codefilter was the first contrib update.module is trying to check, so $fail wasn't over the limit yet, and it reset the count...

However, $fail is just a single variable across all sites. So, it's pretty unreliable as it currently stands, and much is left to chance in cases where not all projects are pointing to updates.d.o...

So, some concrete suggestions/questions:

A) Should we display partial results or not? If so, "No available releases found" isn't really accurate. :( I think we'd need a new string there, but that'd break the string freeze. :( It might be safer to just drop all update information if we fail to fetch some of it, so that we at least get this text on the available updates report:

No information is available about potential new releases for currently installed modules and themes. To check for updates, you may need to run cron or you can check manually. Please note that checking for available updates can take a long time, so please be patient.

If you try to fetch manually, you also get a nice error message (see the "no-results" screenie).

B) If we're going to keep partial results, seems like we need to make $fail be an array keyed by the hostname we're fetching updates from. That means we have to parse the results of _update_build_fetch_url() with parse_url(), which is a slight performance hit, but compared to the actual HTTP requests themselves, it's insignificant.

C) Not necessarily new problem here, but it can be made worse by this patch depending on what we do with (A) and (B). The 'update_last_check' variable, which holds the timestamp when we last fetched data, is only set when we successfully fetched something. :( So, it can lead to slightly weird looking results when you're in a failure case. For example, if you click on "Check manually", and you get an error that says "Unable to fetch any information about available new releases and updates", it might still say: "Last checked: 6 hours 23 minutes ago". :( We should probably move the line where we reset that variable to right before we return, so we mark that we "checked", whether the check resulted in useful results or not.

D) Should we use a variable to control UPDATE_TRIES so you can override that in settings.php if you really want/need to?

I think dww's suggestions make sense.

A) I think partial results might be helpful. I could see installing a one-off module that needs to talk to update.somecompany.com and I would be annoyed if somecompany.com went bankrupt and the government seized their update server, and now I could no longer get updates from d.o. I think it makes sense to fix the status string so that it accurately reflects something to the effect of "the server was unreachable." Will need to ask Gábor if this would be acceptable in 6.x. It could probably go either way, but at least this isn't a screen that a typical "end user" will ever see.

B) Yeah, let's do that.

C) That sounds good, but can we (do we) keep separate statuses for last checked / last successfully checked?

D) That seems a bit overkill to me, but I suppose it doesn't hurt anything. Sure.

@webchick: thanks.

Re: (C): We don't keep separate timestamps, but we certainly could. However, with (B), this gets more complicated. We'd really need separate "last successfully fetched" timestamps for each potential fetch base URL. :( I lean towards having 'update_last_check' just record and mean the last time we tried to check (anywhere) for update data. And the message at the top of the report doesn't need to change at all, since "Last checked:" is still exactly accurate...

ok cool. that works.

Here's a (probably not going to fly) patch for D6. The big problem is that it breaks a few strings, but I'm not sure how to fix this bug in a reasonable way without either breaking strings or printing misleading stuff. :(

It also moves a little shared code into a helper function. Basically, we need both the full fetch URL (what _update_build_fetch_url() returns) and the "base" of that URL without the project info/version/etc. So, I added a _update_get_fetch_url_base() and that's now shared in two places. The alternative would be for _update_build_fetch_url() to return an array of 2 values, or something. I dunno, I'm not thrilled with this solution, but it works. If someone has a better suggestion, I'm all for it.

Otherwise, this is tested and working nicely.

This would need to be ported to D7, e.g. to use drupal_static(), etc.

Ported #11 to D7.

With a nod towards #253501: Tests for update.module this is RTBC.

Adding or changing any function with a leading _ shoudl not be considered an API change - and we have broekn D6 admin-facing strings before if needed.

I looked this over and it looks okay. Committed to HEAD.

I will say though that I am getting more and more nervous about making changes to this module without #253501: Tests for update.module having some (any!) code behind it. If the changes get any more extensive than this, we're probably going to have to postpone them until we have some baseline latticework of tests that assures us that things are working okay. Really, this one should probably be postponed for tests too, but I realize that this is causing headaches with 6.x right now.

Moving to 6.x for consideration.

Seems to work as advertised. I don't think there's any other way to deal with this other than the string change and the changes listed here so I think I agree with pwolanin that this is an acceptable set of changes.

Agreed that this is a good improvement. I also think that the string changes are worth it. Committed to Drupal 6!

Good to see the developments to address the issue on HEAD as there are several duplicated issues to submitted on this, and this one seems to work on patches, I am not a coder but I thought of few ideas comes to my mind a solution around filtering modules with various methods than running the update in order to reduce the consumption.

i thought i should point on that i submitted on the another post on the same issue -they might be feasible at that point or looked into the D7
- apologies if i got it anything wrong as non developer

http://drupal.org/node/238950#comment-1597750
http://drupal.org/node/238950#comment-1681950

@Gabor: Yay, thanks!!! Now I've just got to backport, test and commit to D5--2, then I can finally deal with #478928: Release update_status 5.x-2.4. ;)

@ica: Yes, we're working on it. ;) Please stay on topic in each issue, instead of introducing new feature requests inside other bug reports, etc. Some issues you might be interested in:

#395472: Plugin Manager in Core: Part 1 (backend)
#395474: Plugin Manager in Core: Part 2 (integration with update status)
#162788: Include modules that aren't enabled
...

Untested backport, if anyone wants to review/test...

Tested #20 and committed it to update_status DRUPAL-5--2. Moving this issue back to core for posterity.

Automatically closed -- issue fixed for 2 weeks with no activity.