Upgrade.php not handling errors correctly.

[akc42]

I hesitate to post here, because the problems I have been having (see http://www.simplemachines.org/community/index.php?topic=496753.0) seem so likely to have been encountered before that I can't see how they have escaped detection before now.  However, if I give as full details as possible so someone with more knowledge than me can determine if its my setup, or similar issue that is causing the problem.

Firstly, I have to say my setup is slightly strange, in that I do not manage the relationship between the upgrade process through forum admin, but rather through the use of git branches.  I have one branch that is the raw distribution, a master branch which is a version with all my mods and themes installed, and a site branch, which is derived from master, but has different settings so it works in my production context.  I have been running upgrade.php in my master branch, having recently merged the latest distribution (2.0.4) and english-utf8 language packs into it.

I have checked that the code in question does not seem to have any differences between the distribution and my master branch.

The upgrade process is between version 2.0RC5 and 2.0.4.  This probably means (I am not sure it applies to the entire file) that the modifications to the database that are described in upgrade_2-0_mysql.sql have already been applied to my database.  In particular, the area that has been causing me problems is the

Code Select Expand


ALTER TABLE {$db-prefix}boards ADD COLUMN redirect VARCHAR(255)

As this column already existed in my database.

The symptoms I was seeing was that the upgrade process hung, and after 30 seconds I would receive a message saying one of the steps had timed out.  Looking behind the scenes (firebug in Firefox), I could see that the response to the Ajax request to conduct the particular substep that did the alter table was the raw text "Duplicate column 'redirect'".  Since this was not valid xml, the ajax response function was ignoring it - and thus the request never seemed to complete.

Fortunately I have been able to get an Eclipse/XDebug combination working to put breakpoints into the code of the upgrade.php script to work out where the "Duplicate column 'redirect'" was being produced.  The answer to this is on line 569 of Sources/Subs-Db-mysql.php

Code Select Expand


// Nothing's defined yet... just die with it.
if (empty($context) || empty($txt))
die($query_error);


There are two issues here.

• $context is never even setup within the upgrade.php script (nor for that matter $txt['database_error'] which is in index.English file rather than Install.english language file)
• the call to $smcFunc['db_query'] is not using db_error_skip to avoid an internal use of smf_db_error, when just after it, if it sees the error it calls it directly ( approx line 2519 - I don't have the exact line number right now because I have added some code to over come these issues - see below)

I added some code further up the upgrade.php script to set up the $context variable and ran into two more problems, namely that functions allowedTo and fatal_error were not defined.  I wrote a couple of noddy functions to simulate these.  I also added a change to the call of $smcFunc['db_query'] in the upgrade_query

Following code inserted at line 643 of upgrade.php

Code Select Expand


if(!isset($context)) {
$context = Array();
$context['error'] = '';
if(!isset($txt['database_error'])) $txt['database_error'] = 'Database Error';
}
if(!function_exists('allowedTo')) {
function allowedTo($p,$b){
return True;
}
}

if(!function_exists('fatal_error')) {
function fatal_error($error,$log){
return($error);
}
}


and the following at what used to be around line 2500 - but I can't tell you the exact place because of the code inserted above has moved it all down.  update_query now starts at line 2527

Code Select Expand


function upgrade_query($string, $unbuffered = false)
{
global $db_connection, $db_server, $db_user, $db_passwd, $db_type, $command_line, $upcontext, $upgradeurl, $modSettings;
global $db_name, $db_unbuffered, $smcFunc;

// Get the query result - working around some SMF specific security - just this once!
$modSettings['disableQueryCheck'] = true;
$db_unbuffered = $unbuffered;
$result = $smcFunc['db_query']('', $string, Array('security_override'=>1,'db_error_skip'=>1));
$db_unbuffered = false;


With all these changes in place, my upgrade script now runs through to successful completion.

But as I said at the beginning - these are quite large changes to be made to code that MUST have been run before in this situation.  This means I may have missed something significant.  I just don't know what.

[emanuele]

But as I said at the beginning - these are quite large changes to be made to code that MUST have been run before in this situation.  This means I may have missed something significant.  I just don't know what.

Don't be so sure...
I fixed a bug in the upgrader while releasing 2.0.3 (i.e. upgrade fails if the themes directory is not in $boarddir), I hope I didn't break anything related to that...

[akc42]

I just tried to upgrade another forum from 1.1.11 to 2.0.4 and ran into exactly the same problems.  The same fix sorted them

[krussell]

Just confirming that the same fix solved my upgrade problem:
http://www.simplemachines.org/community/index.php?action=post;topic=497181.0;last_msg=3492606

[emanuele]

Digging my open tabs I found a couple of topics I was keeping there to further investigations:
http://www.simplemachines.org/community/index.php?topic=489671.0
http://www.simplemachines.org/community/index.php?topic=489393.0
and I think they are exactly the same issue since the failure is in the query just before the one you found.
And these are just two, but I'm pretty sure there is more around.

Now it would be funny to understand why it has worked for some time and now it doesn't, but better concentrate on the fix.
If you want to apply it and send a pull request at github feel free to, otherwise I'll commit it "in few days".
It may even be worth fixing it in SVN and refresh the current release...

And thanks for the debugging!

[Gaucho1962]

akc42 ypu are a super star, spent two days looking for a solution and this was it

[emanuele]

Committed to SVN (revision 11050).
Will be included in the upcoming security release.

Thanks akc42!

[Bartabat]

Thanks akc42, your fix works like a charm