JavaScript updater confusing when update path has bugs/warnings

I, too, had the upgrade script 'hang' using Firefox. As far as I can tell, the Javascript isn't 100%.

I've had troubles with the JS upgrade method in the past (including earlier today). Annoyingly, I've just run a couple of updates on my local machine from 4.6.5 to 4.7.0b4 and then up to HEAD - both without any trouble.

If I have time today then I'll make a patch to strip out all of the JS-related code from updates. Ideally I'd prefer to find what is making things hang or create errors though.

If we can't fix this timely (before the next beta/RC), we'll take out the Javascript bit.

Simpler than I thought. Attached patch to update.php, and of course you could then remove update.js from misc.

Unless someone comes up with a fix, I'll commit Thox' patch before I roll the next RC/beta.

With respect to the menu-related issues mentioned:

- Should update really be telling the user to disable contrib themes? The primary links update requires the phptemplate variables to be available so it can import the old links.

- I don't see that checking for the existence of a "Primary links" menu to be particularly useful. The update script should only be run once... I think it explicitly warns about only running once. So if a user wants to run it twice they can keep both pieces.

Does the JS error reporting patch fix this issue, or at least partially fix it? Can this be split into more specific issues if not? I agree with Ber about the need to keep everything functioning with and without JS. I just don't like the scorched earth remove feature solutions.

There was some off-list discussion of this. I'll just paste my message here:

I've developed several (multi-part) updates, each time using the Javascript version to develop and test. I've never encountered a hang that was not caused by faulty code in my particular update. And since then I've committed a patch which fixes the lack of feedback when the update polling fails, whether it be due to PHP errors or transfer errors. So you will never get an indefinite hang (at most until the PHP time limit, or the TCP/IP timeout).

The JS update path is not voodoo magic. It simply makes the update polling asynchronous for the end-user. If the JS version hangs, then the non-JS version hangs too. Thox's removal patch shows how little effect the JS has on the update system.

As far as testing goes, you can do short test runs if you add some bogus updates that consist of sleep statements (to ensure multiple requests are needed to finish). You can manually force updates to run again using the schema dropdown in update.php. Unless you actually want to test the 4.6-4.7 updates themselves, then it doesn't matter because, whether Javascript is used or not, the updates are performed exactly the same way.

To be honest, I'm sick and tired of anti-JS FUD. Like with any other feature in Drupal, we can only fix problems if there are proper bug reports which allow reproducing the bug. If we cannot reproduce, then we should "won'tfix", and not "ripoutthefeaturebecauseit'seviljs". Anecdotal reports of "having the update system hang" are useless.

Ber, what Steven says is that JS is *not* the cause of update problems which I tend to believe.

I am no JS guru. But I was writing a chat program for my own needs with xmlhttprequest years before anyone has called this ajax. I do not pretend to be able to write the nice behavioral js Steven & Thox produced. But I understand it pretty well, and it would come to me a big surprise if it would cause "hang". Do not strip the JS as it's very useable.

removing the submitter's proposed (non) resolution from the bug report. a description of the bug is all we need there.

Ber: could you provide a copy of your database/site for Thox/Steven/anyone to play with? Looks like you have a backup that allows us to recreate the problem. Anything particular in your watchdog? Any 'headers have already been sent' messages? Can you try again with full error logging? Looks like you're the key to have us investigate this some more.

I don't understand why you would have a problem by running update.php twice though. After each update, the following code is run:

function update_data($module, $number) {
  ...
  if ($finished == 1) {
    drupal_set_installed_schema_version($module, $number);
  }
}

The menu update is not a multi-part one, so it will always finish in one call. Running update.php again should not mess things up, unless you manually force the schema version to an old one.

I've got a 4.6 database sitting around that I've been meaning to update. It's backed up, so I can attempt upgrades, botch, and restore endlessly if necessary.

At present, I get a cascade of errors just hitting update.php.

Warning: Illegal mix of collations (utf8_general_ci,COERCIBLE) and (latin1_swedish_ci,IMPLICIT) for operation 'like' query: SELECT * FROM access WHERE status = 1 AND type = 'host' AND LOWER('69.209.91.222') LIKE LOWER(mask) in /home/viaposit/public_html/includes/database.mysql.inc on line 120

Warning: Illegal mix of collations (utf8_general_ci,COERCIBLE) and (latin1_swedish_ci,IMPLICIT) for operation 'like' query: SELECT * FROM access WHERE status = 0 AND type = 'host' AND LOWER('69.209.91.222') LIKE LOWER(mask) in /home/viaposit/public_html/includes/database.mysql.inc on line 120

... are the important ones, everything else is just cascading 'headers already sent' stuff. If I persist in upgrading, it tells me to start at System update 129. After a few seconds, a Javascript window pops up with an impressive list of errors also revolving around those same two queries. Rinse, repeat. I'm sure others have had the same problem, but I wanted to chip in and see if there's anything I can do to help.

...And, I ran the patch and the update process works. It's not as pretty as the JS version, but it does work. And that makes me happy.

Eaton: so the JS upgrade system doesn't work and the non-JS one works?

Dries: Just spent some quality time restoring and upgrading a 4.6 database over and over. What I've found is:

For MY database (can't speak of any others) the abovementioned SQL errors appear as soon as I hit update.php. If I persist and launch the update operation using the current cvs version of update.php, it chugs along for a bit, then throws a javascript dialog up with a page of SQL and output errors relating to collation problems.

IF I go back to update.php from scratch, it starts where it left off. It continues throwing javascript popup errors, though, and I'm not sure if it's actually making progress.

If I use the patched non-Javascript version, the same underlying errors seem to happen, I just never get the Javasript popup window. When the non-JS version 'halts' (around update 146 in this case) it just sits there with two SQL errors on the page and doesn't continue reloading/executing subsequent updates.

It appears that going back and starting the update again -- and ultimately succeeding -- works with the non-JS version. I have to go through a few more restore/update/restore/update/etc cycles to be sure.

I've attached a full backup of the pre-update DB for others' troubleshooting/testing fun.

eaton: does this happen always on update 146?

After speaking with eaton on IRC, we figure out his problem is an update issue, and the JS error reporting seems to be functioning properly in his case.

I would say for the most part none of the issues in this thread are directly javascript related. Javascript/Ajax hinders debugging, and is far less straight forward than the typical GET/POST http scenario.

I think the new javascript error reporting will ease a lot of the hinderances AJAX causes with the debugging process.

Nearly all of these errors I think are directly related to the update system, and errors in the system_update_n function.

The system_update_n functions are atomic per update_n hook, but what about within the update. I see multiple alter queries in a many system_update_N functions. In a case where the first few queries suceed and a latter fails the update won't be marked as complete and will be re-attempted... re running some updates could have destructive effects if an early "transaction" wasn't completed.

I think this js issue should be closed outright, and proper issues against the update system be opened.

killes: after investigation, it appears dopry's analysis is correct. Javascript isn't the source of any of these issues, it just muddied the waters a bit.

Even if the analysis is correct, and the JS isn't the source of the problem, it managed to confuse you more than it should. It took you a few attempts to realize and validate that everything is OK. Something you, as a developer, can do. Certainly not something Joe Average will manage to investigate and grok.

Soon there will be up to 50.000 Drupal websites that might attempt to upgrade; the process should not be confusing, nor frustrating. Let's try to fix the broken update so it's no longer confusing. We now have a test database to play with.

If we can't fix the broken upgrade, we might still have to remove the JS for reasons explained in the first line of my comment.

I tried to upgrde eaton's database and got this error:

Failed: ALTER TABLE {vocabulary} ADD tags tinyint(3) unsigned default '0' NOT NULL

Everything else went fine.

I think I hate backported features with db changes.

Killes: did that cause your Javascript upgrade to hang/choke? What did you experience?

I had tried without JS. Now tried with JS and it went through.

I got "user warning: Duplicate column name 'tags' query: ALTER TABLE vocabulary ADD tags tinyint(3) unsigned default '0' NOT NULL in /home/killes/checkouts/drupal/includes/database.mysql.inc on line 120."

Printed on the screen, but this might be due to the debugging uptions in settings.php that I used.

The only problem I have with updating the database dump higher up is when PHP's display_errors is set to TRUE. In that case, the UTF-8 Collation warning messes things up for JS, because it is output before update_do_update_page() can surpress all error reporting.

IMO we should simply always hide all errors (by moving the ini_set('display_errors', FALSE) to the beginning of update.php:

• Without it, the utf-8 collation and watchdog referrer errors show up on the update.php Access Denied page, possibly confusing users (no update_fix_foo() is run in this case, and even if they would run, the errors still show up the first time).
• All errors are logged to the watchdog and shown at the end of the update process anyway.

Patch attached for this. Though it also hides the collation error, I added an explicit fix for it anyway, as this is the best solution. I can complete the update process on the faith.gz dump, though there is a message at the end about a failed update.

Now because the main issue here is user confusion, I took a closer look at what happens if a fatal error occurs. Neither the JS nor non-JS version is clear: an alert box in the former, a white page with error message in the latter (but only if php.ini error reporting was set to true, which is not the default, otherwise you get a blank page). So I altered the JS so it redirects to an error page, and I used an output buffering trick to achieve the same in non-JS.

So now, they both do exactly the same and recover from fatal PHP errors in a user friendly manner.

I like UnConeD's approach to fixing this issue.

Committed to HEAD.

So this evening, Matt and I were updating Participate.net when we got the dreaded infinite database loop. Nothing seems to happen, never gets to 1%. If you look at the database, access table is updated to utf8, but that is it.

You can start and stop but nothing. Then it was going the 2nd time and we got this error after a few min:

'An HTTP error 404 occured'
update.php?op=do_update

Strange. So then we apply Thox's patch and Matt modifies it slighty to apply to HEAD and magically, everything works. But strangely enough, I *didn't* refresh my browser cache, but instead reran the page, and the JS updater worked great this time around.

*Very* strange. Was it the patch that did it? If so, I didn't refresh my cache so none of those JS updates were ever applied, only the ones to update.php were.

If not, was it just 3rd time is a charm type luck?

Who knows, but upgrading is a bit of a headache. However, I've encountered this infinite loop before, but no corrupt data so far, mostly just puzzlement. And then it just works.

However, Steven, great patch, no more long lists of collation errors :-) That's a great start!

Oh wait, this patch never touched update.js. Hmm, perhaps the key to fixing this infinite loops lies somewhere in this attached patch, because everything did work after applying it.

Much too tired to analyze it anymore tonight though.

Restoring old status.

I agree with Ber. It seems to be the number one thing people are complaining about at http://drupal.org/drupal-4.7.0-beta5. Until we can easily reproduce the problem, there is little we can do. We'll need someone to deliver a (new) database dump that we can use to reproduce the problem.

I suspect that browser caching of js files from the older version has something to do with this. Please try to collect the following information from anyone with these problems:

-Browser/OS version
-PHP Version
-PHP error handling settings (listed at http://www.php.net/manual/en/ref.errorfunc.php#errorfunc.configuration)
-Drupal error handling setting
-Try clearing your browser's cache, going back to a backup if one exists, and trying again.

Remember that the update script changes the internal version number after each update it performs, so it is harder to perform the same update twice.

It would be easy to attach a time() timestamp to the javasript incldue in the updater just to make sure that everyone has the latest. this is an acceptable performance hit IMO because updating is so rare.

I meant append a timestamp on the querystring thus forcing redownload of the js

Bèr: I disagree that the JS adds complexity. It simply adds a different, more elegant polling mechanism (ajax with HTTP POST) in place of a very outdated and inappropriate one (meta refresh with HTTP GET). All the complicated actions happen on the PHP side. The obscuring is very, very minimal. Anyone who knows PHP should be able to make sense of JS code. You know Ruby as well, so you have an extra edge.

The main problem with the JS was strange error reporting in the update system. Fine, I fixed that. All it took was someone to analyse the PHP error reporting from a neutral perspective. Your last comment seems to indicate that you already knew of these issues before. If you then chose "remove the update JS" as the answer, then you are not thinking from a "I have nothing against JS" perspective IMO. Especially considering that the non-JS version used to have much worse usability issues than the JS version (blank screen if display_errors was set to Off and a fatal error occurred).

As far as your list of methods of 'breaking the update' go, the update system in HEAD recovers gracefully for all of them (so you are definitely not using the latest). Still, I think that any update which calls non-existant functions or prints out crap is broken and needs to be fixed. The only issue is if ini_set does not work, but in that case settings.php will throw errors anyway. It is IMO an unsupported configuration.

The update.php is not properly reporting errors anymore. I spent 45 minutes debugging a simple problem.

Update.php said: 'The update process did not complete. All errors have been logged. You may need to check the watchdog table manually.' but no errors were logged.

Updating from 4.7beta4 to 4.7beta5 the updater javascript seems to hang..

The FireBug extension reports the following responseText came back
{ status: 1, percentage: 100, message: "Updating complete" }
But immideately after that it reports "parseJson is not defined" in progress.js on line 89.

This only happens when I upgrade on a server that is far away (as in not on my local network) .. on the local network it just works(TM)

By the way, to debug AJAX Firebug is invaluable.

Might be related or similar to a problem I had: http://drupal.org/node/13148#comment-79277. Javascript reported an "unspecified error" because of PHP error that was surpressed. Removing ini_set('display_errors', FALSE) from update.php helped me debug the problem (but might screw up the updater). Removing that line worked for me.

If you get a parseJson not defined, then your browser is definitely caching an old JS file. That function is defined in drupal.js, which all other JS files depend on, including all the code before that in progress.js.

Summary:

1. errors in update hooks play havoc with the upgrade process.
2. Javascript makes debugging difficult for some developers.
3. old javascipt source files are cached on the browser
4. ini_set(display_errors, 0) messes with the Javascript and hides errors.

Suggested Solutions:

1. file bugs against the update system
2. use firebug, or read the error message and goto 1
3. add a ?$timestamp to the url for the javascript in update.php
4. Our Ajax library needs to report that php execution failed due to a fatal error, and possibly report that error in place of ini_set(display_errors, 0).

I still don't see that JS is a problem. I think the remaining bugs are still primarily in the update system, political/personal, with our Ajax implementation(particularly the error reporting), and good old browser caching which has plagued web developers for all time.

I'm going to split this issue into several distinct issues once I get to my office. And maybe we can close some parts of it instead of having several issues buried under the rather abstract title of "Javascript is Confusing".

.darrel.

Good summary, Darrel. Good idea to split the issue in smaller (critical and non-critical) problems.

I wish there was a split to other issues status, but fixed will have to do.