Joomla! Issue Tracker | Joomla! CMS #41367 - [4.3] Work around a successful upgrade with silent errors

? Language Change bug PR-4.3-dev

Fixed in Code Base
13 Sep 2023
Medium
Build: 4.3-dev
# 41367
Diff
Fedik:upd-fail-fix

Pending

User tests: Successful: Unsuccessful:

Fedik
14 Aug 2023

Summary of Changes

Kind of a fix for failed upgrade that report success.
I have added error collector, the most messages are loged now.
Also updated the "complete" layout, now it will show a meesage about error instead of "success".

Note: it is just a work around. A proper fix require more coding/re-coding of upgrade process.

Main issue that all messages like

joomla-cms/administrator/components/com_admin/script.php

Line 217 in 3d07930

    
           echo Text::sprintf('JLIB_DATABASE_ERROR_FUNCTION_FAILED', $e->getCode(), $e->getMessage()) . '<br>';

are lost, and after upgrade is finished, the complete upgrade page shows just a static "Success"

joomla-cms/administrator/components/com_joomlaupdate/tmpl/joomlaupdate/complete.php

Line 23 in e7f5cc1

    
           <?php echo Text::sprintf('COM_JOOMLAUPDATE_VIEW_COMPLETE_MESSAGE', '&#x200E;' . JVERSION); ?>

Testing Instructions

Download upgrade packge from the pr.
In joomla 3 in any Fields plugin add throw new Exception('Plugin test error'); in plugin constructor.
Run joomla upgrade, with manual upload of upgrade packge.

public function __construct(&$subject, $config)
{
  parent::__construct($subject, $config);
  throw new Exception('Error error, Fields plugin');
}

Addittionaly, try to add throw new Exception('Update test error'); in coupe of upgrade methods in administrator/components/com_admin/script.php

Actual result BEFORE applying this Pull Request

The finalise step will crash with messed up page.
Upgrade log reaport about "Files deleting blabla", but no error info.

Expected result AFTER applying this Pull Request

The finalise step are finished.The completion page show a message about unsuccessful update.
Upgrade log contains logs for error 'Plugin test error'.

3d74bcc 14 Aug 2023

Log all

3d07930 14 Aug 2023

Capture errors

a9058e3 14 Aug 2023

Log messages

joomla-cms-bot - change - 14 Aug 2023

Test 1: Cause database error in fixTemplateMode method of script.php so preflight fails with exception

Modification in the update packages in file "administrator/components/com_admin/script.php":

    protected function fixTemplateMode(): void
    {
        $db = Factory::getContainer()->get('DatabaseDriver');

        foreach (['atum', 'cassiopeia'] as $template) {
            $clientId = $template === 'atum' ? 1 : 0;
            $query    = $db->getQuery(true)
                ->update($db->quoteName('#__template_styles'))
                // Test PR #41367: Simulate database error
                // ->set($db->quoteName('inheritable') . ' = 1')
                ->set($db->quoteName('not_existing_column') . ' = 1')
                // Test PR #41367 end
                ->where($db->quoteName('template') . ' = ' . $db->quote($template))
                ->where($db->quoteName('client_id') . ' = ' . $clientId);

            try {
                $db->setQuery($query)->execute();
            } catch (Exception $e) {
                $this->collectError(__METHOD__, $e);
            }
        }
    }

This shall simulate any kind of database error happening at that place.

The above code is from the package with the PR applied, in the package without the PR it is done accordingly but of course without the changes from this PR.

Actual result without the PR applied

Custom update URL: obsolete link removed
Update package: obsolete link removed

Backend after the update:

The "joomla_update.php" log file doesn't show anything special, i.e. no differences to the log from a normal update without errors beside of the time stamps, the update zip name and the user ID. All steps of the update have been processed.

Expected result with the PR applied

Custom update obsolete link removed
Update package: obsolete link removed

Backend after the update:

The backend shows a page with is blank except of error messages or alerts, i.e. it is missing all backend menus and modules because using the system template.

Reloading the page doesn't change that.

Only using the "back" button of the browser 2 times or more or changing the URL in the browser's URL field e.g. to administrator/index.php without any options brings us back to the administrator template.

The "joomla_update.php" log file shows 2 errors as the error happens 2 times, one time for each core template. All steps of the update have been processed. See here the comparison, left side = with PR applied, right side = without PR applied:

Test 2: Let preflight method of script.php fail e.g. due to broken manifest cache and return false.

Modification in the update packages in file "administrator/components/com_admin/script.php":

    public function preflight($action, $installer)
    {
        if ($action === 'update') {
            // Get the version we are updating from
            /* Test PR 41367: Simulate missing or broken manifest
            if (!empty($installer->extension->manifest_cache)) {
                $manifestValues = json_decode($installer->extension->manifest_cache, true);

                if (array_key_exists('version', $manifestValues)) {
                    $this->fromVersion = $manifestValues['version'];

                    // Ensure templates are moved to the correct mode
                    $this->fixTemplateMode();

                    return true;
                }
            }
            Test PR 41367 end */

            return false;
        }

        return true;
    }

This shall simulate the preflight method failing due to a missing or incomplete manifest cache.

Actual result without the PR applied

Custom update URL: obsolete link removed
Update package: obsolete link removed

Backend after the update:

The backend shows an alert with an unspecific warning.

The "joomla_update.php" log file shows no warning or error. But compared to the log file from a successful update you can see that the SQL update scripts have not been run. See following comparison, left side = log from this test without PR applied, right side = log from a successful update:

Expected result with the PR applied

Custom update URL: obsolete link removed
Update package: obsolete link removed

Backend after the update:

The backend shows a page with is blank except of error messages or alerts, i.e. it is missing all backend menus and modules because using the system template.

Reloading the page doesn't change that except of closing the yellow alert, which is the same as shown without this PR applied.

The "joomla_update.php" log file shows 1 error and 1 warning. Same as without the PR applied, the SQL update scripts have been not processed. See here the comparison, left side = with PR applied, right side = without PR applied:

Test 3: Cause an SQL error in an update SQL script (new script 4.3.4-2023-08-20.sql).

New update SQL scripts "4.3.4-2023-08-20.sql" with an erroneous SQL statement in the update packages.

This shall simulate any kind of SQL or database error during the SQL updates.

Actual result without the PR applied

Custom update URL: obsolete link removed
Update package: obsolete link removed

Backend after the update:

The backend shows an alert about the SQL error.

The "joomla_update.php" log file shows the SQL error and reports the incomplete SQL updates.

Expected result with the PR applied

Custom update URL: obsolete link removed
Update package: obsolete link removed

Backend after the update:

The backend shows a page with is blank except of error messages or alerts, i.e. it is missing all backend menus and modules because using the system template.

Reloading the page doesn't change that except of closing the yellow alert, which is the same as shown without this PR applied.

The "joomla_update.php" log file shows 1 warning and 1 error, but they are some kind of redundant to the logs reported already without the PR applied and which are still there when the PR is applied. See here the comparison, left side = with PR applied, right side = without PR applied:

Summary

In Test 1, without this PR, neither the backend nor the update log file show anything about the error, while with this PR both do.

For Test 2 the same applies as for Test 1, except that without the PR you can see in the update log file that the SQL updates have not been executed if you are an expert on database update.

My thoughts:

I'm not sure if it's a good idea to always use the system template, except if we can extend it by a link to the administrator/index.php without any options, so we navigate to the home dashboard.
The logs in test 3 added by this PR seem to be redundant, but that might not be the case in every scenario, so I am ok with them. @Fedik But maybe you want to change something there since it seems that SQL errors in the update SQL scripts are already handled well without your PR.

@Fedik More tests will follow sooner or later, but maybe you want to change something after these first results, so I will wait a bit for your feedback.

Fedik - comment - 20 Aug 2023

@richard67 thanks, that a huge test ;)

I'm not sure if it's a good idea to always use the system template

This is in purpose, to increase a chance to see the result page of failed upgrade.
And to avoid modules rendering, in case some SQL fail to update one of them. Example fail of upgrade #__menu schema will lead to crash of the menu module.

The logs in test 3 added by this PR seem to be redundant,

I just replaced all echo to do log. However existing log to jerror category produce a message.
I tried to minimise changes in behavior. I would keep it as it is for 4.x.

It still would need a proper refactoring of whole upgrade code, there many strange pieces.
The upgrade process managed to survive all the way from Joomla 1.6 ?

Did you tried to test CLI update?

UPD. I will try to add "Dashboard button", or something like that.

richard67 - comment - 20 Aug 2023

Did you tried to test CLI update?

No, not yet. Meanwhile I have updated my custom update URLs so they can also be used for updating from 3.10.12. Will provide more test in the next days.

HLeithner - change - 20 Aug 2023

Title

…

Work around a successful upgrade with silent errors

[4.3] Work around a successful upgrade with silent errors

HLeithner - edited - 20 Aug 2023

bddafb2 22 Aug 2023

Merge branch '4.3-dev' into upd-fail-fix

8a159bb 22 Aug 2023

Back button and better text

Fedik - comment - 22 Aug 2023

I have added "back button", and changed message to be more helpfull.

@brianteeman please check the message text, thanks!

ec04fcf 22 Aug 2023

phpcs

2af0bec 22 Aug 2023

logs

brianteeman - comment - 22 Aug 2023

the english is fine if thats the message you want to say. i have no opinion on that

ed342c5 23 Aug 2023

Make sure logging is working before continue

Fedik - comment - 23 Aug 2023

I have added a test for logging before download/upload.
This will work for future updates 4+, but 3 => 4 still will fail when log is not writable.

569803b 25 Aug 2023

Merge branch '4.3-dev' into upd-fail-fix

richard67 - comment - 25 Aug 2023

@Fedik In the testing instructions you wrote:

In joomla 3 in any Fields plugin add throw new Exception('Plugin test error'); in plugin constructor.

But on a 3.10.12, none of the fields plugins has a constructor. Did you mean we should add a constructor with that code?

Or could we also modify e.g. a content plugin which has already a constructor?

Fedik - comment - 25 Aug 2023

Yes, I meant "add" :)
Here a code for copy/paste:

public function __construct(&$subject, $config)
{
  parent::__construct($subject, $config);
  throw new Exception('Error error, Fields plugin');
}

Added to description.

Or could we also modify e.g. a content plugin which has already a constructor?

No, the content plugin not loaded while upgrade, I already tested.

Fedik - change - 25 Aug 2023

The description was changed

Fedik - edited - 25 Aug 2023

dffd3cd 27 Aug 2023

Merge branch '4.3-dev' into upd-fail-fix