Question restore failing with TypeError in PHP 8

[schach]

We are currently using STACK with Moodle 4.2 running on PHP 8.1. We have noticed that restores are failing with Type Error "Unsupported operand types: string + int".

Stack trace: line 303 of /question/type/stack/backup/moodle2/restore_qtype_stack_plugin.class.php: TypeError thrown line 98 of /backup/moodle2/restore_plugin.class.php: call to restore_qtype_stack_plugin->after_execute_question() line 405 of /backup/util/plan/restore_structure_step.class.php: call to restore_plugin->launch_after_execute_methods() line 113 of /backup/util/plan/restore_structure_step.class.php: call to restore_structure_step->launch_after_execute_methods() line 186 of /backup/util/plan/base_task.class.php: call to restore_structure_step->execute() line 191 of /backup/util/plan/base_plan.class.php: call to base_task->execute() line 168 of /backup/util/plan/restore_plan.class.php: call to base_plan->execute() line 411 of /backup/controller/restore_controller.class.php: call to restore_plan->execute() line 193 of /backup/import.php: call to restore_controller->execute_plan()

Moodle's restore process fails in backup/moodle2/restore_qtype_stack_plugin.class.php when adding the value 1 to the string $node->truenextnode. The is probably due to stricter type handling in PHP 8. As a workaround, typecasting variables $node->truenextnode, $node->falsenextnode and $node->nodename to int fixes the problems with restore.

[sangwinc]

@EJMFarrow could you please look into this? Thanks, Chris

[EJMFarrow]

This is odd. The code as it stands should still work in PHP 8.1:

An error would get thrown only if $truenextnode can't be evaluated as a number i.e. '1', '-1' and NULL are fine but '' or 'a' are not. NULL is the database default.

Looks like a data issue that PHP below 8 just skated over by evaluating to 0 (as does casting to int). Will need to investigate further to find out how we can end up in this situation. Setting the result to NULL might be a better longterm fix but casting to int restores the behaviour of PHP 7. I'll also need to check this isn't just shunting the problem further down the line.

[wilenius]

Also perhaps worth adding to @schach's original bug report is that we had been running the new PHP version for a couple of months before the problem surfaced, and after that, all restores/imports across the site failed – even with empty course areas. So we're not sure what initially triggered the problem.

[EJMFarrow]

Hi @wilenius. Thanks for the additional information. Are you saying that the STACK issue caused restores of even courses without STACK questions to fail?

[wilenius]

Yes! The stack trace on botched restore attempts was always the same I think. Restores were broken on all course areas. And the fix documented above fixed all restores...

[EJMFarrow]

OK, thanks for the confirmation. I will try to get to the bottom of this!

[wilenius]

We tried looking for similar issues in Moodle's tracker, but all we found was this: https://tracker.moodle.org/browse/MDL-75381.

[EJMFarrow]

I think this was triggered by an attempt to restore a really old and/or broken STACK question that was missing some fields. Previously, blank fields would have been evaluated to 0 but PHP 8 is more picky so the tidy up after importing such a question now fails. That tidy up is run on all restores so it keeps failing.

Apologies! I'll add the fix to cast to int for the next release and that will revert behaviour to be as before.

[schach]

Thanks for the quick investigation. Here's a couple of rows from our database where truenextnode or falsenextnode are blank, might help to locate the problem. In our database we have 28 such rows out of a total of 237742 rows.

# SELECT * FROM mdl_qtype_stack_prt_nodes WHERE truenextnode = '' OR falsenextnode = '' ORDER BY id;
   id   | questionid |  prtname  | nodename | answertest | sans | tans | testoptions | quiet | truescoremode | truescore | truepenalty | truenextnode | trueanswernote | truefeedback | truefeedbackformat | falsescoremode | falsescore | falsepenalty | falsenextnode | falseans
wernote | falsefeedback | falsefeedbackformat | description 
--------+------------+-----------+----------+------------+------+------+-------------+-------+---------------+-----------+-------------+--------------+----------------+--------------+--------------------+----------------+------------+--------------+---------------+---------
--------+---------------+---------------------+-------------
 216518 |    5742260 | prtResult | 4        |            |      |      |             |     0 | =             | 1.0000000 | 0.0000000   |              |                |              |                  0 | =              | 0.0000000  | 0.0000000    |               |         
        |               |                   0 | 
 231703 |    5996570 | prtResult | 4        |            |      |      |             |     0 | =             | 1.0000000 | 0           |              |                |              |                  0 | =              | 0.0000000  | 0            |               |         
        |               |                   0 |