No Code Attached Yet
avatar nikosdion
nikosdion
1 Oct 2021

I bumped into this interesting bug when one of my database tables became corrupt.

Steps to reproduce the issue

Install Joomla 4 on a PHP 8.0 host. The PHP version is important.

(optional) Set Site Debug to Yes and Error Reporting to Maximum.

Pick a view which uses Empty State, e.g. articles.

Sabotage your database to cause a SQL error — this emulates the effects of having a corrupt table or a Joomla update failing to apply schema changes. For articles, sabotage the #__content table, e.g. rename a column which is used in the view.

Expected result

You get a Joomla error page about a SQL error which helps you identify and fix the root cause.

Actual result

You get a PHP error (InvalidArgumentException).

If you turned on Site Debug you will see that it points out to this code:

if (!\count($this->items) && $this->isEmptyState = $this->get('IsEmptyState'))
{
	$this->setLayout('emptystate');
}

It complains that $this->items is a boolean, not countable.

System information (as much as possible)

PHP 8.0 is the only relevant piece of system information.

Additional comments

When a SQL error occurs, ListModel::getItems returns boolean false. In PHP 7 trying to count() a non–countable scalar would result in a warning. On PHP 8 this is an InvalidArgumentException which causes a stop error.

There are two ways to deal with it.

The architecturally correct solution is to change the first line of that if–block to read

if (\is_countable($this->items) && !\count($this->items) && $this->isEmptyState = $this->get('IsEmptyState'))
{
	$this->setLayout('emptystate');
}

However, it's not viable for Joomla 4 since is_countable is only available on PHP 7.3 and later when the Countable interface was added to the language. Joomla requires PHP 7.2 or later. If we insist on supporting PHP 7.2 this won't work.

This brings us to the second solution. All HtmlView classes which implement Empty State also have this piece of code to check for errors:

// Check for errors.
if (\count($errors = $this->get('Errors')) || $this->transitions === false)
{
	throw new GenericDataException(implode("\n", $errors), 500);
}

Moving that if–block above the Empty State if–block resolves the problem.

Pinging @PhilETaylor since he added the Empty State code to Joomla.

avatar nikosdion nikosdion - open - 1 Oct 2021
avatar joomla-cms-bot joomla-cms-bot - change - 1 Oct 2021
Labels Added: No Code Attached Yet
avatar joomla-cms-bot joomla-cms-bot - labeled - 1 Oct 2021
avatar PhilETaylor
PhilETaylor - comment - 1 Oct 2021

well, welcome to the hell that is Joomla where a method has a mixed return, and can return a An array of data items on success, false on failure, instead of just an empty array or property handled exception :)

avatar nikosdion
nikosdion - comment - 1 Oct 2021

To be honest, null would be a reasonable response type. An empty array could be confused with the valid result of a query which returns no data. That's why PHP allows nullable types.

An even better approach would be the Model, Table and whatever just throw the exception on error instead of setting a property so we could use proper exception handling with any unhandled exception bubbling up and ultimately get caught by Joomla's exception handler. Something for 5.0, I guess ?

Anyway, the reason I pinged you is that you contributed this feature and I wanted you to know I found a way it breaks on PHP 8 which wasn't obvious at the time of contribution.

avatar PhilETaylor
PhilETaylor - comment - 1 Oct 2021

Anyway, the reason I pinged you is that you contributed this feature and I wanted you to know I found a way it breaks on PHP 8 which wasn't obvious at the time of contribution.

Thanks. It was developed on PHP 8 (as all my code nowadays) but I guess I never checked all return types that the method could return

Although as this only happens in extreme edge cases where there are bigger underlying issues (like you say, a missing column) Im not going to drop everything and rework it today :)

avatar nikosdion
nikosdion - comment - 1 Oct 2021

Of course, no problem. This is more of a warning to 3PDs copying what Joomla is doing. Our extensions are far more likely to suffer from a failed schema update than core Joomla :)

avatar nikosdion nikosdion - change - 9 Aug 2022
Status New Closed
Closed_Date 0000-00-00 00:00:00 2022-08-09 16:42:37
Closed_By nikosdion
avatar nikosdion nikosdion - close - 9 Aug 2022

Add a Comment

Login with GitHub to post a comment