gjbarnard / moodle-format_grid

Grid course format contributed by Gareth J Barnard originally created by Paul Krix
GNU General Public License v3.0
21 stars 55 forks source link

Image information is empty fatal upgrade error #143

Closed aspark21 closed 3 years ago

aspark21 commented 3 years ago

Moodle 3.9.7 Grid 3.9.1.2 cffbcd81ec95df96895a8f8013ed2abd87f40fc8

-->block_checklist ++ Success (0.54 seconds) ++ -->format_grid Database transaction aborted automatically in /var/www/site/admin/cli/upgrade.php Default exception handler: Image information is empty - itemid > 161039, filename > goi_UNO Logo klein.png, sectionimage_sectionid > 161039, sectionimage_image > goi_UNO Logo klein.png, sectionimage_newimage > goi_UNO Logo klein.png and sectionimage_displayedimageindex > 1. Please report error details and the information contained in the php.log file to developer. Debug: generate_image Error code: noimageinformation

!!! Image information is empty - itemid > 161039, filename > goi_UNO Logo klein.png, sectionimage_sectionid > 161039, sectionimage_image > goi_UNO Logo klein.png, sectionimage_newimage > goi_UNO Logo klein.png and sectionimage_displayedimageindex > 1. Please report error details and the information contained in the php.log file to developer. !!! !! generate_image Error code: noimageinformation !! !! Stack trace: * line 498 of /lib/setuplib.php: moodle_exception thrown

===== Exit Code ===== 0

This kinda looks related to some of the fixes you've been doing for #142

aspark21 commented 3 years ago

For reference, updating from 3c73a292767f015a3ef72d266d111c1cb4bb140f

I've looked through the commits in between but can't see what's changed in relation to this tbh

gjb2048 commented 3 years ago

@aspark21 Why do you think its related to #142? I'm intrigued by your thought, please explain your thinking? I'll also get back to you in a moment with another question pertaining to the actual image. Aside, any news on the outstanding Adaptable issues please?

aspark21 commented 3 years ago

I can see you were making more use of that update_displayed_images_callback callback e.g. in 871fcaf28d0d7ed339ff9ddd5f679c45c371a75d and changing the savepoint from which it gets triggered 34175eb2034521717cb14880092397151c945d83

gjb2048 commented 3 years ago

@aspark21 Is image with itemid '161039' corrupted in your MoodleData folder? i.e. use that 'itemid' in the files table to find the entry to then get the content hash to then find the file on disk, copy it and rename as a .png:

Screenshot 2021-05-19 132146

gjb2048 commented 3 years ago

@aspark21 https://github.com/gjb2048/moodle-format_grid/issues/143#issuecomment-844053921 is to do with the fact that the coloured background is no longer imposed on the generated image to allow the new summary image to be generated without it. Hence all of the images needing to be regenerated as a part of the upgrade. This issue stems from before that point when the original image information is being retrieved by PHP's 'getimagesize'.

aspark21 commented 3 years ago

Are you using the file API or going to disk directly?

reason I ask is site where the error occurred is in AWS and those files are likely in S3 if they haven't been served recently. Didn't get an error on the on-prem despite not having any files

gjb2048 commented 3 years ago

@aspark21 I'm using Moodle's file API.

gjb2048 commented 3 years ago

@aspark21 FYI: https://moodle.org/mod/forum/discuss.php?d=422456

gjb2048 commented 3 years ago

@aspark21 Additional, ok, I am using the Moodle file API even though I'm using PHP functions on a 'file' directly -> https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/lib.php#L2466-L2500 as I'm using Moodle's 'copy_content_to' method (https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/lib.php#L2479) to operate on a copy of the image in a temporary location, thus, could it be that is failing and causing https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/lib.php#L2758 to fall over on the filepath of that temporary file -> https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/lib.php#L2478-L2479 ?

mnods commented 3 years ago

FYI same issue, tried with 3.9.1.3 on mdl3.9.7

i expect it would work in prod as the images concerned will exist but the upgrade fails in stage environment as part of a pipeline, where the image doesn't exist (db copy with no files)

gjb2048 commented 3 years ago

@mnods That's really helpful, thank you! Now that I know that there is no files attached in MoodleData folder, then I can work on an alternative solution to making this work. @aspark21 Is this the case with you?

aspark21 commented 3 years ago

The file should exist in AWS S3 - https://moodle.org/plugins/tool_objectfs (non-prod envs can bring down a local copy of any prod file)

But as I said most likely wouldn't be in the moodledata directory.

gjb2048 commented 3 years ago

@aspark21 and @mnods Ok, this patch https://github.com/gjb2048/moodle-format_grid/commit/2e7c2bea1860b5f3adfcbadfbfc4caec6aa7f86e should fix it, as now the displayed image is only flagged in the DB as needing updating, and only actually updated when the course is accessed again for the first time.

mnods commented 3 years ago

thanks gareth, will have a look soon.

gjb2048 commented 3 years ago

Release candidate: https://moodle.org/mod/forum/discuss.php?d=422979.

katcher commented 3 years ago

Hello, I am having a similar issue moodle 3.9.7+ and it is preventing upgrade. thankfully it is on my development branch. this is urgent, any suggestions?

3.9.7+ (BUILD: 20210520) (2020061507.02)

Once you do this you can not go back again. Please note that this process can take a long time.

Are you sure you want to upgrade this server to this version?

type y (means yes) or n (means no) : y -->format_grid PHP Warning: copy(/public/moodledata/live/filedir/b8/e7/b8e77c24cb9583c3297f8b7f61fd92f44e1c06fb): failed to open stream : No such file or directory in /public/home/moodle/lib/filestorage/file_system_filedir.php on line 234 PHP Warning: getimagesize(/public/moodledata/live/temp/gridformatdisplayedimagecontainer/b8e77c24cb9583c3297f8b7f61fd92f 44e1c06fb): failed to open stream: No such file or directory in /public/home/moodle/course/format/grid/lib.php on line 27 58 PHP Warning: unlink(/public/moodledata/live/temp/gridformatdisplayedimagecontainer/b8e77c24cb9583c3297f8b7f61fd92f44e1c0 6fb): No such file or directory in /public/home/moodle/course/format/grid/lib.php on line 2761 Database transaction aborted automatically in /public/home/moodle/admin/cli/upgrade.php Default exception handler: Image information is empty - itemid > 1541773, filename > goiWelcome.png, sectionimage sectionid > 1541773, sectionimage_image > goi_Welcome.png, sectionimage_newimage > goi_Welcome.png and sectionim age_displayedimageindex > 1. Please report error details and the information contained in the php.log file to develop er. Debug: generate_image Error code: noimageinformation

!!! Image information is empty - itemid > 1541773, filename > goi_Welcome.png, sectionimage_sectionid > 1541773, sectionimage_image > goi_Welcome.png, sectionimage_newimage > goi_Welcome.png and sectionimage_displayedimageindex > 1. Please report error details and the information contained in the php.log file to developer. !!! !! generate_image Error code: noimageinformation !! !! Stack trace: * line 498 of /lib/setuplib.php: moodle_exception thrown

gjb2048 commented 3 years ago

@katcher I've never seen this upgrade process before:

type y (means yes) or n (means no) : y -->format_grid PHP Warning: copy(/public/moodledata/live/filedir/b8/e7/b8e77c24cb9583c3297f8b7f61fd92f44e1c06fb): failed to open stream : No such file or directory in /public/home/moodle/lib/filestorage/file_system_filedir.php on line 234

So:

How do you perform it? Does the file '/public/moodledata/live/filedir/b8/e7/b8e77c24cb9583c3297f8b7f61fd92f44e1c06fb' exist on your system?

please.

katcher commented 3 years ago

Hi Gareth, update is performed on my dev seerver which as mentionned above does not have the files - moodledata directory migrated from production. I was hoping there would only be a few items to copy over from production and rerun the upgrade (cmdline) but it seems like jumping down a rabbit hole.

Is there a query I can run to see the complete list of missing files? If so, and it is not too many, i can get them from prod and copy them over. It is not a perfect solution but i ineed to get my development server back up and running. Any suggestion?

Fairly urgent.

Eric

gjb2048 commented 3 years ago

@katcher

The grid_icon table has the file table ref as you can see in https://github.com/gjb2048/moodle-format_grid/issues/143#issuecomment-844055096.

I do have a possible code solution in M3.10 -> https://moodle.org/mod/forum/discuss.php?d=422979 - but as of yet, no feedback.

Why do you not have the associated moodledata folder when migrating a system from one server to another? This is outside of normal practices.

G

katcher commented 3 years ago

I do have a moodledata folder but, it is not a full copy. Our moodledata folder is over 10tb as a result, we cannot have a duplicate. So, we usually have a db backup from prod that we restore on our dev database and then perform the upgrade to ensure no issues ...

We hope. Since we are running moodle 3.9 I assume your patch will not work on version 3.9. Is that correct?

katcher commented 3 years ago

PS it is not outside of normal practices. I have a complete moodle development environment but not a complete moodledata folder!

katcher commented 3 years ago

if you have a few minutes we can discuss, I can post a zoom link?

gjb2048 commented 3 years ago

Having an incomplete Moodledata folder which means that the 'files' table has references to files that do not exist IS outside of what is recommended etc.

The patch should work, but the Moodle 3.9 version is a few commits behind the M3.10.

If you'd like a zoom call, then that's 'paid for support' time that I would charge for.

katcher commented 3 years ago

Hi Gareth, just to understand, I ran your upgrade code from moodle.org and because I did not run it on a complete moodle data directory it failed.

I have been managing our moodle installation for over 10 years now and have not encountered an error like this which broke the upgrade. Are you planning on backporting your update for 3.10 to LTS 3.9?

Please advise. If I can help in anyway.

Eric

katcher commented 3 years ago

ps, I am not migrating from one server to another. I am upgrtading a copy of my production moodle in development prior to attempting on production. This is not a move as you indicated.

We always run a test upgrade on our development server prior to upgrading on production.

gjb2048 commented 3 years ago

@katcher Ok:

I have been managing our moodle installation for over 10 years now and have not encountered an error like this which broke the upgrade. Are you planning on backporting your update for 3.10 to LTS 3.9?

Fair enough, but conceptually, as with all things a plugin given everything being equal, may want to update the files it manages. Just because you've not encountered it, does not mean that it cannot happen when you operate with an incomplete Moodle installation where 'referential integrity' (conceptual in this instance) has been broken between the files table in the DB and the files on the storage device.

I am planning a backport at some point in time.

I am not migrating from one server to another. I am upgrtading a copy of my production moodle in development prior to attempting on production. This is not a move as you indicated.

That's not what I mean. Test servers should always where possible be an identical replication of the production environment, thus this issue would not have happened and the upgrade would have worked as I tested it. Your 'test' server is not an 'identical' copy of the production, and therefore any upgrade procedure cannot honestly be 100% tested and certifiable as being safe before you try on production.

katcher commented 3 years ago

@gjb2048 It is often impossible to maintain an exact copy for financial reasons - 10 tb is too large to duplicate on our infrastructure.
Also, i have updated your plugin on many occasions and have not encountered any problems in the past.

In an attempt to bring my dev site back I downgraded the current version to 2020070701 and updated the mdl_config_plugins version number. Is that ok? Were there any additional db changes since that version?

Thanks again for your help.

gjb2048 commented 3 years ago

@katcher Ok:

Are you serious that your institution cannot afford a 10TB HD?

You've not encountered an issues because I've not needed to do anything like this in the past.

For DB info (to save me time), look in the db/upgrade.php file.

In an attempt to bring my dev site back I downgraded the current version to 2020070701 and updated the mdl_config_plugins version number. Is that ok?

Yes, but you'll need to clear the 'allversionshash' (from memory so might not be the exact name) too etc.

G

katcher commented 3 years ago

@gjb2048

I am not trying to be antagonistic but I would expect that as a developer - who has contributed so much to moodle, you would be conscious of other institutions realities. Infrastructure changes are not simply plug in a hard drive and away you go!
Also, I was not the only person who encountered this issue! I am just reporting.

Thanks for your kind assistance

gjb2048 commented 3 years ago

@katcher I am aware of how Moodle works and therefore do expect that others are using it as it should be. This issue was quite unexpected to me!

porcospino commented 3 years ago

Hi @gjb2048

We've found this isn't necessarily because the files aren't present on the filesystem anymore. There seem to be orphaned records in mdl_format_grid_icon. This seems to happen when a section is deleted. We are trying to upgrade from format_grid 3.5.0.6 to 3.9.1.3 as part of an upgrade from Moodle 3.5.16 to 3.9.7

Files that belong to course sections that have been deleted are revealed by this SQL:

select gi.*
from mdl_format_grid_icon gi
where gi.image is not null
and not exists (select 1 
                        from mdl_course_sections cs 
                        where cs.course = gi.courseid 
                        and cs.id = gi.sectionid);

We can work around this by deleting these records from mdl_format_grid_icon

gjb2048 commented 3 years ago

@porcospino The grid icon table is used to store the information about the uploaded image and its displayed image version - hence displayed image index. Thus it is more than likely that both images per record exist in the Moodle file system.

porcospino commented 3 years ago

@gjb2048 sorry I wasn't clear. The following SQL will get a similar (but at our site not identical) list of entries in mdl_format_grid_icon that don't have a corresponding entry in mdl_files

select gi.*
from mdl_format_grid_icon gi
inner join mdl_context cx on 
    (cx.instanceid = gi.courseid and cx.contextlevel = 50)
left join mdl_files f on 
    (f.contextid = cx.id and f.filename = gi.image 
     and f.component = 'course' and f.filearea = 'section')
where f.id is null
and gi.image is not null;

Each of these entries will cause a failure here:

https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/lib.php#L2468-L2469

because (I think) the files are removed from the mdl_files table when the section is deleted. Even if the files existed on the filesystem, we wouldn't know, because we don't have the hashes in mdl_files anymore.

gjb2048 commented 3 years ago

Oh, ok, I'd not considered a section deleted event -> https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/classes/observer.php

gjb2048 commented 3 years ago

Then again should happen here: https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/lib.php#L1872-L1882

gjb2048 commented 3 years ago

And: https://github.com/gjb2048/moodle-format_grid/blob/b4eee51fe40787f65693e45c641fca24e006f13e/lib.php#L1927-L1959

porcospino commented 3 years ago

OK, wow, that callback was introduced in 2.9.0.2. I think that means we've been sitting on this problem since we had Moodle 2.7 and it's only just surfaced...

gjb2048 commented 3 years ago

@porcospino To be honest, I'm now a tad confused. This issue problem or another?

gjb2048 commented 3 years ago

@porcospino Odd, as Grid reacts to sections being deleted and removes the image as needed.

porcospino commented 3 years ago

I get the same error message as @aspark21 when I try to upgrade. As you say, Grid correctly handles section deletions, but it did not before 97a001da7c24c43858a020a761097ad172bdcfd4. So I think we have entries in mdl_format_grid_icon that pre-date this change, and the orphaned records have only come to light because of a change made somewhere between V3.5.0.6 and V3.9.1.3. I know that's a big delta to be looking through, but we always upgrade between LTS releases of Moodle.

Gael-Mifsud commented 3 years ago

Hello,

As porcospino, we upgrade Moodle with LTS versions. We encountered the same error "noimageinformation" this morning when we tried to upgrade a pre-production Moodle from 3.5 to 3.9.

We hope there can be a workaround.

Thanks.

2021-06-25 - Moodle 3 9 7+ (Build 20210617)

Gael-Mifsud commented 3 years ago

We executed a new test.

On a freshly updated Moodle from 3.5 to 3.9 : We upgraded the grid plugin from 3.5 to 3.6, then 3.6 to 3.7, then 3.7 to 3.8 with no problem. And when we upgrade from 3.8 to 3.9 that triggered the same error as my last message.

Hope this helps.

gjb2048 commented 3 years ago

@Gael-Mifsud Thanks for the info, the patches https://github.com/gjb2048/moodle-format_grid/commit/2e7c2bea1860b5f3adfcbadfbfc4caec6aa7f86e and https://github.com/gjb2048/moodle-format_grid/commit/d5b9af8cb7d71e4c07ddf6909aa9465df4463b11 will fix this issue.

Gael-Mifsud commented 3 years ago

Hello.

We upgraded from Moodle 3.5 to Moodle 3.9 directly with the patched version (with number finishing with "705") and unfortunately, we have now another error : unknown column 'updatedisplayedimage'.

Capture web_30-6-2021_8394_192 168 0 15

We plan on testing to upgrade from 3.5 to 3.6 to 3.7 to 3.8 to 3.9 this afternoon, but we can't at this moment.

Thank you for your help.

gjb2048 commented 3 years ago

@Gael-Mifsud When did you get the code? As I patched that version a few days ago and it would not have had that issue unless you were already using the code from here, being the same version (as its my development area) and thus:

    if ($oldversion < 2020070705) {
        // Define field updatedisplayedimage to be added to format_grid_icon.
        $table = new xmldb_table('format_grid_icon');
        $field = new xmldb_field('updatedisplayedimage', XMLDB_TYPE_INTEGER, '1', null, XMLDB_NOTNULL, null, '0', 'displayedimageindex');

        // Conditionally launch add field updatedisplayedimage.
        if (!$dbman->field_exists($table, $field)) {
            $dbman->add_field($table, $field);
        }

        /* This is a new mechanism and thus we now use it for the previous two versions update of the images.
           Clearly this has to happen now that the new field is in the DB.
           Background was removed from images and transparent PNG's need regenerating. */
        global $CFG;
        require_once($CFG->dirroot.'/course/format/grid/lib.php'); // For format_grid.

        format_grid::update_displayed_images_callback();

        // Grid savepoint reached.
        upgrade_plugin_savepoint(true, 2020070705, 'format', 'grid');
    }

would not have been executed. So, what version of the format were you running before the upgrade please?

gjb2048 commented 3 years ago

P.S. Oh drat drat and double drat, development version (prior to release checking), has:

    if ($oldversion < 2020070703) {
        // Change in default name.
        $value = get_config('format_grid', 'defaultsection0ownpagenogridonesection');
        set_config('defaultsetsection0ownpagenogridonesection', $value, 'format_grid');

        global $CFG;
        require_once($CFG->dirroot.'/course/format/grid/lib.php'); // For format_grid.

        format_grid::update_displayed_images_callback();

        upgrade_plugin_savepoint(true, 2020070703, 'format', 'grid');
    }

Which would cause the problem.

Gael-Mifsud commented 3 years ago

Hello.

I'm not sure I understand but it seems good news (only development error), isn't it ? Anyway, here are the versions we used :

Version to upgrade : // Plugin version. $plugin->version = 2018052306;

// Required Moodle version. $plugin->requires = 2018051700.00; // 3.5 (Build: 20180517).

// Full name of the plugin. $plugin->component = 'format_grid';

// Software maturity level. $plugin->maturity = MATURITY_BETA;

// User-friendly version number. $plugin->release = '3.5.0.6';

Version we used for the UPGRADE // Plugin version. $plugin->version = 2020070705;

// Required Moodle version. $plugin->requires = 2020061500.00; // 3.9 (Build: 20200615). $plugin->supported = array(39, 39);

// Full name of the plugin. $plugin->component = 'format_grid';

// Software maturity level. $plugin->maturity = MATURITY_STABLE;

// User-friendly version number. $plugin->release = '3.9.1.4';

gjb2048 commented 3 years ago

@Gael-Mifsud Ok, are you installing (or intend to) the code from here on a production server? i.e. The code published here should only be used on a test server and is subject to change without notice or version bump.

Gael-Mifsud commented 3 years ago

We used it in a test-server for now. We wanted to use it on our production-servers when it will become official but not the development version.

Is it safe to upgrade to version 3.8 and wait for a 3.9 that won't trigger the error ?

gjb2048 commented 3 years ago

@Gael-Mifsud I've just patched the M3.9 version -> https://github.com/gjb2048/moodle-format_grid/commit/c2ed248858421b5ea546c3638ad44a63fe67812f. The M3.8 version is quite a way back in terms of functionality and I doubt I'll bring it forward unless I need to. The M3.9 version has better image generation, with smaller ones as the grid image background is not put into the actual image. In any event, I ask that you use the version of Grid designed for the major version of Moodle, i.e. as a matched pair.