[5.4] Change backend views to exceptions instead of legacy error handling

[Hackwar]

Summary of Changes

With the new flag in the LegacyErrorHandlingTrait to check if exceptions should be used, we now can finally move away from the legacy setError()/getError() to a proper error handling with exceptions. This PR converts the backend views to use this new flag. When set, instead of using the legacy methods, the model code should throw exceptions instead.

Testing Instructions

1. Go into every backend view (list and edit views) and check if they still display as before. Since we are only changing the views right now, nothing else has to be checked.

2. To check the new behavior in case of an error, we have to artificially trigger an error.
   You can do so in the list views by for example editing the database query in the getListQuery() method of the respective model.
   You can use any of the models tourched by this PR except of com_contact ArticlesModel, thats a special case tested in the next test3.
   For example, go to administrator/components/com_contacts/src/Model/ContactsModel.php and add a column to the select statement which doesn't exist in the database.
   Then visit the article list view in the backend and see the error.
   Before applying the patch, you get a strange error. Example for com_contact list view:

An error has occurred.
    0 count(): Argument #1 ($value) must be of type Countable|array, false given 

After applying the error should be more descriptive. Example for com_contact list view:

An error has occurred.
    500 Unknown column 'a.blablubb' in 'field list' 

And you should of course still get an error.

3. Same as test 2, but this time for administrator/components/com_content/src/Model/ArticlesModel.php.
   This is a special case as the strange error has been already fixed with PR [5.4] Error handling: Adding new shouldUseException() #44098 .
   Here you just check that with this PR applied the error is shown in the same way as without this PR.

Link to documentations

Please select:

• Documentation link for docs.joomla.org:

• No documentation changes for docs.joomla.org needed

• Pull Request link for manual.joomla.org:

• No documentation changes for manual.joomla.org needed

[exlemor]

Hi @Hackwar, testing this PR, applied Patch, went to Content --> Articles, crash:

An error has occurred.
0 Call to undefined method Joomla\Component\Content\Administrator\Model\ArticlesModel::setUseExceptions()

same at Content --> Categories, Featured Articles, Field Groups, Site Modules, Administrator Modules, etc

(sorry)

[richard67]

@exlemor How have you applied the PR? With patchtester? Or have you used the download from drone?

It can be that something is missing because the PR is not up to date with the base branch.

I will trigger a branch update now.

If you have used patchtester, revert the patch, refresh the pulls list and then apply again.

If you have used the download, please wait until a new download is available.

Thanks in advance.

[richard67]

@Hackwar Could you add the PHPStan errors to the phpstan-baseline.neon file? After the branch update from the 5.4-dev branch, where PHPStan is meanwhile mandatory, we have lots of errors Error: Call to deprecated method setUseExceptions() of class Joomla\CMS\MVC\Model\BaseModel:.

[richard67]

@exlemor P.S. to my previous comment: It seems your 5.4 testing environment did not include the merged PR #44098 . Maybe it's because you are testing with 5.4.0-alpha2? The PR has been merged after that alpha, so you have to use a current 5.4-dev branch or a recent 5.4-dev nightly build.

[brianteeman]

@exlemor you must always test with the current branch or at a minimum the nightly release. Anything else is not suitable for testing

[exlemor]

@Hackwar before I validate this as a successful test...

I re-installed clean environment and took about 30 minutes to check every backend view (list and edit views) - and everything worked..

BUT, when I did the error triggering, I still got a crash in both cases just with the Patch applied it was considerably shorter - not sure I would call it more descriptive though - (for someone who isn't a coder I mean) so if you meant a shorter stack trace that doesn't include legacy setError()/getError() - then that matches your expectations and I can validate it as successful.

[hans2103]

@Hackwar tested option two:

To check the new behavior in case of an error, we have to artificially trigger an error. You can do so in the list views by for example editing the database query in the getListQuery() method of the respective model. For example, go to administrator/components/com_content/src/Model/ArticlesModel.php and add a column to the select statement which doesn't exist in the database. Then visit the article list view in the backend and see the error. Before applying the patch, you get a strange error, after applying the error should be more descriptive. And you should of course still get an error.

I have opened the suggested file and added a non existing column to the select list in function getListQuery()

$db->quoteName('a.whatever'),

Before and after applying the patch I get the same error.

the error after applying the patch is not stranger then before. :-) It is the same.

[richard67]

@Hackwar Can it be that the testing instructions do not match anymore because your other PR #44098 has been merged meanwhile, so @hans2103 got the right error message in both cases, without and with this PR applied because that was fixed with the other PR?

[Hackwar]

Indeed. 😑

[richard67]

Indeed. 😑

@Hackwar Maybe you can adjust the testing instructions just to check that it works as before?

[richard67]

@Hackwar P.S.: It seems we have to do something to calm down PHPstan as the setUseExceptions is deprecated for 7.0, so we get a lot of new PHPstan messages.

[richard67]

@Hackwar I've allowed myself to update your testing instructions.

@exlemor @hans2103 Could you test again with the updated instructions?

[richard67]

I have tested this item ✅ successfully on 8089eec

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44828.}

[rbuelund]

I have tested this item ✅ successfully on 8089eec

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44828.}

[alikon]

RTC

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/44828.}

[muhme]

✅ Final test before merge, using JBT:

• Checked ugly errors with 'hacking' select in contacts, users and tags models
• Installed PR with graft
• Enabled 'Debug System' and 'Log Almost Everything'
• Tested nice! exceptions with 'hacking' select in contacts, users and tags models again
• Opened all backend views, added articles etc. to have list view, opened object view, opened tabs if multiple existing, all with quick visual check
• Additional test with articles and 'hacking' administrator/components/com_content/src/Model/ArticlesModel.php also with nice exception
• Checked administrator/logs/everything.php only CRITICALS with the 'hacked' exceptions, and the already known mail ERRORs
• Checked no PHP errors and warnings

[muhme]

Thank you @Hackwar for contribution. Thank you @brianteeman and @richard67 fort supporting. Thank you @hans2103, @exlemor, @rbuelund and @richard67 for testing.