Backup error

musicpanda commented

2017-09-22 15:37:49 -05:00

(Migrated from github.com)

While updating on my Windows localhost the software ends with the following error message when it starts making a backup:
Error when trying to add C:xampphtdocs_webshop4AdapterAdapter_AddressFactory.php to archive C:xampphtdocs_webshop4AdapterAdapter_AddressFactory.php.

For some reason the slashes between "xampp" and "htdocs" and between the directory and the filename have disappeared.

This failed upgrade left my webshop with a new very big backup table. I saw no alternative but to delete it before trying upgrading with manual backup only.

While updating on my Windows localhost the software ends with the following error message when it starts making a backup: Error when trying to add C:xampphtdocs\_webshop4AdapterAdapter_AddressFactory.php to archive C:xampphtdocs\_webshop4AdapterAdapter_AddressFactory.php. For some reason the slashes between "xampp" and "htdocs" and between the directory and the filename have disappeared. This failed upgrade left my webshop with a new very big backup table. I saw no alternative but to delete it before trying upgrading with manual backup only.

musicpanda commented

2017-10-17 08:14:50 -05:00

(Migrated from github.com)

After some research I came to the conclusion that very likely the entry into the database is at least part of the problem.

Tbupdater first collects the filenames (with path) that need to be backed up. It them puts them into the database in the table tb_tbupdater_files_for_backup. And it then retrieves them from that table for the actual archiving..

My suspicion is that those filenames with path are not escaped and that as a consequence those backslashes are interpreted as escapes by MySQL. In the array $files in Backup::addFiles the backslashes are still present. Inside the database they are no longer there.

Unfortunately the code is too cryptic for me to find a remedy.

After some research I came to the conclusion that very likely the entry into the database is at least part of the problem. Tbupdater first collects the filenames (with path) that need to be backed up. It them puts them into the database in the table tb_tbupdater_files_for_backup. And it then retrieves them from that table for the actual archiving.. My suspicion is that those filenames with path are not escaped and that as a consequence those backslashes are interpreted as escapes by MySQL. In the array $files in Backup::addFiles the backslashes are still present. Inside the database they are no longer there. Unfortunately the code is too cryptic for me to find a remedy.

firstred commented

2017-10-17 16:56:19 -05:00

(Migrated from github.com)

Thanks for the extra info. Those are some great clues we can use to figure it out. Thanks for your help!

musicpanda commented

2017-10-19 11:29:43 -05:00

(Migrated from github.com)

I noted that I had put my next comments in another thread (https://github.com/thirtybees/thirtybees/issues/374#issuecomment-337756137).

The counter is now on 5 bugs in backup of which I solved four. My next one has to do with the database backup (so file backup works). Database backup stops due to a javascript error: Uncaught TypeError: Cannot read property 'nextQuickInfo' of null at Object.success (upgrader.js:484)

The improved code for Backup::addFiles is
` public static function addFiles(array $files)
{
foreach ($files as &$item) {
$item = ['file' => Db::getInstance()->escape($item)];
}

    $success = true;

    foreach (array_chunk($files, static::CHUNK_SIZE) as $chunk) {
        $success &= Db::getInstance()->insert(
            static::TABLE,
            $chunk,
            false,
            false
        );
    }

    return $success;
}`

Note that this includes two bug fixes. The escape of the filename and sending $chunk instead of $files to the database.

The other two fixed bugs are in AjaxProcessor.php. I will list them later:

the test for when all files are processed doesn't work. It should use backup:count() and be placed later in the code.
There was that PS_BASE_URI was not defined

I noted that I had put my next comments in another thread (https://github.com/thirtybees/thirtybees/issues/374#issuecomment-337756137). The counter is now on 5 bugs in backup of which I solved four. My next one has to do with the database backup (so file backup works). Database backup stops due to a javascript error: Uncaught TypeError: Cannot read property 'nextQuickInfo' of null at Object.success (upgrader.js:484) The improved code for Backup::addFiles is ` public static function addFiles(array $files) { foreach ($files as &$item) { $item = ['file' => Db::getInstance()->escape($item)]; } $success = true; foreach (array_chunk($files, static::CHUNK_SIZE) as $chunk) { $success &= Db::getInstance()->insert( static::TABLE, $chunk, false, false ); } return $success; }` Note that this includes two bug fixes. The escape of the filename and sending $chunk instead of $files to the database. The other two fixed bugs are in AjaxProcessor.php. I will list them later: - the test for when all files are processed doesn't work. It should use backup:count() and be placed later in the code. - There was that __PS_BASE_URI__ was not defined

musicpanda commented

2017-10-19 17:41:30 -05:00

(Migrated from github.com)

The database backup error gives me a headache. Maybe someone else knows more about what is happening here.

The javascript error is caused by a kind of crash. That is: the ajax ends returning nothing - yet that is interpreted by upgrader.js as a success. The error follows when it cannot find the fields that it expects.

The silent end happens during the query in AjaxProcessor.php that starts on line 731:
$data = $this->db->executeS( (new DbQuery()) ->select('*') ->from($table) ->limit((int) $backupLoopLimit, 200), true, false );

To find out what goes wrong here I put some debug code in Db.php in the function Db::executeS(). To prevent it from being triggered during earlier calls to this function I used a global flag that I only set just before the relevant code.

A var_dump of the query delivered the following:
object(TbUpdaterModule\DbQuery)#69 (1) { ["query":protected]=> array(8) { ["select"]=> array(1) { [0]=> string(1) "*" } ["from"]=> array(1) { [0]=> string(14) "tb_tb_access" } ["join"]=> array(0) { } ["where"]=> array(0) { } ["group"]=> array(0) { } ["having"]=> array(0) { } ["order"]=> array(0) { } ["limit"]=> array(2) { ["offset"]=> int(200) ["limit"]=> int(0) } }}

What looks interesting here is "tb_tb_access". It looks like an extra prefix is set before the table name. Yet the query in AjaxProcessor.php is called with the correct "tb_access".

Does anyone who knows the database functions what is going wrong here?

The database backup error gives me a headache. Maybe someone else knows more about what is happening here. The javascript error is caused by a kind of crash. That is: the ajax ends returning nothing - yet that is interpreted by upgrader.js as a success. The error follows when it cannot find the fields that it expects. The silent end happens during the query in AjaxProcessor.php that starts on line 731: `$data = $this->db->executeS( (new DbQuery()) ->select('*') ->from($table) ->limit((int) $backupLoopLimit, 200), true, false );` To find out what goes wrong here I put some debug code in Db.php in the function Db::executeS(). To prevent it from being triggered during earlier calls to this function I used a global flag that I only set just before the relevant code. A var_dump of the query delivered the following: `object(TbUpdaterModule\DbQuery)#69 (1) { ["query":protected]=> array(8) { ["select"]=> array(1) { [0]=> string(1) "*" } ["from"]=> array(1) { [0]=> string(14) "`tb_tb_access`" } ["join"]=> array(0) { } ["where"]=> array(0) { } ["group"]=> array(0) { } ["having"]=> array(0) { } ["order"]=> array(0) { } ["limit"]=> array(2) { ["offset"]=> int(200) ["limit"]=> int(0) } }}` What looks interesting here is "`tb_tb_access`". It looks like an extra prefix is set before the table name. Yet the query in AjaxProcessor.php is called with the correct "tb_access". Does anyone who knows the database functions what is going wrong here?

musicpanda commented

2017-10-20 10:46:27 -05:00

(Migrated from github.com)

How stupid can you get?

The function Db::displayError() in the tbupdater module displays its error messages with Tools::displayError(). Unfortunately this function does nothing to display the messages so that they disappear in the void.

It is already very puzzling that this module contains its own version of the TB classes. That these have such a low quality is even more puzzling. Not being able to display error messages is about as low as you can get.

It would seem to me logical to at least keep things parallel. So for all those classes that the TBupdater contains it should always use a copy of the latest version of TB itself.

How stupid can you get? The function Db::displayError() in the tbupdater module displays its error messages with Tools::displayError(). Unfortunately this function does nothing to display the messages so that they disappear in the void. It is already very puzzling that this module contains its own version of the TB classes. That these have such a low quality is even more puzzling. Not being able to display error messages is about as low as you can get. It would seem to me logical to at least keep things parallel. So for all those classes that the TBupdater contains it should always use a copy of the latest version of TB itself.

musicpanda commented

2017-10-22 15:35:52 -05:00

(Migrated from github.com)

I wrote a little report of what should be changed in the tbupdater module. I need to stop working on it as other issues need my attention. Due to the way Github maltreats code inside the text I have attached a text file.

If the proposed modifications are implemented the module should be able to finish when you don't disable the backup option. However, there is still a bug in the backup database section as it will overwrite itself. The code should be stored in filename.sql.bz2, filename-2.sql.bz2, filename-3.sql.bz2, etc. However, there is a bug so that it always uses the same filename so that it overwrites previous parts and only the last part of the backup is saved.

Another issue that needs attention is that this is the only module that is Composer based. As a consequence it uses old versions of the main TB classes. I believe this Composer shell should be removed so that we don't have duplicate code that is hard to maintain.

tpupdater-changes.txt

I wrote a little report of what should be changed in the tbupdater module. I need to stop working on it as other issues need my attention. Due to the way Github maltreats code inside the text I have attached a text file. If the proposed modifications are implemented the module should be able to finish when you don't disable the backup option. However, there is still a bug in the backup database section as it will overwrite itself. The code should be stored in filename.sql.bz2, filename-2.sql.bz2, filename-3.sql.bz2, etc. However, there is a bug so that it always uses the same filename so that it overwrites previous parts and only the last part of the backup is saved. Another issue that needs attention is that this is the only module that is Composer based. As a consequence it uses old versions of the main TB classes. I believe this Composer shell should be removed so that we don't have duplicate code that is hard to maintain. [tpupdater-changes.txt](https://github.com/thirtybees/tbupdater/files/1405320/tpupdater-changes.txt)

musicpanda commented

2017-12-17 12:45:59 -06:00

(Migrated from github.com)

The problem with the database backup files is in the function initializeFiles() in AjaxPrrocessor.php on line 2560.

You find there the line
$this->backupDbFilename = 'auto-backupdb_'.$this->backupName.'.sql';
This should be
$this->backupDbFilename = 'auto-backupdb_XXXXXX_'.$this->backupName.'.sql';

The missing of the XXXX part causes that on 645 no file numbers are inserted and the files overwrite each other.

The problem with the database backup files is in the function initializeFiles() in AjaxPrrocessor.php on line 2560. You find there the line `$this->backupDbFilename = 'auto-backupdb_'.$this->backupName.'.sql';` This should be `$this->backupDbFilename = 'auto-backupdb_XXXXXX_'.$this->backupName.'.sql';` The missing of the XXXX part causes that on 645 no file numbers are inserted and the files overwrite each other.

firstred commented

2017-12-17 12:48:08 -06:00

(Migrated from github.com)

Ahhh great find. Thanks!

musicpanda commented

2017-12-19 07:58:33 -06:00

(Migrated from github.com)

Something has gone wrong. In version 1.25 all the improvements of 1.24 have been discarded.

Traumflug commented

2019-04-04 17:30:43 -05:00

(Migrated from github.com)

Well, module tbupdater is in the process of getting abandoned in favor of Core Updater. Currently needed only for updating modules.

Which means, tbupdater won't receive bug fixes any more.

Well, module _tbupdater_ is in the process of getting abandoned in favor of Core Updater. Currently needed only for updating modules. Which means, _tbupdater_ won't receive bug fixes any more.

Rows
Columns

Backup error #9