? Pending

User tests: Successful: Unsuccessful:

avatar mbabker
mbabker
9 Jan 2017

Partial Pull Request for #13357

Summary of Changes

In a lot of places in the code, failing to read from the cache store should not result in an unrecoverable error state. This PR makes JMenuSite::load() resilient to cache related errors by catching exceptions implementing the new JCacheException interface and will try to still execute the database query to load the data instead.

The JCacheException interface is added to make it easier to implement exception handling for these classes. I elected an interface to not bind all exceptions explicitly to being subclasses of RuntimeException. So instead of having (currently) two catch blocks if you don't care the type of error you can just have this single block.

I explicitly catch the cache exceptions so we are explicitly aware the error condition is related to the cache failing to connect and not some other RuntimeException that gets thrown in the process (i.e. the database exceptions). I also only explicitly catch JDatabaseExceptionExecuting for the database side of the error handling since it is probably an unrecoverable error condition if the menus can't be used and frankly if any other type of RuntimeException is getting thrown at this point then there is a part of me that'd be surprised the code ran that far in normal use and there is a part of me that says this method isn't trying to handle all possible error conditions so only handle what it knows how to recover from.

Testing Instructions

If the cache store fails to connect or the cache configuration is bad, the exceptions thrown by the cache API should be caught and JMenuSite::load() try to manually execute the lambda function to load the data. A database query failure here should still result in the same error message being displayed. Other types of RuntimeException should now go uncaught and bubble up as this method isn't adequately suited to be a general error handler and now explicitly only deals with error types it can respond to.

Documentation Changes Required

N/A

avatar mbabker mbabker - change - 9 Jan 2017
Status New Pending
avatar mbabker mbabker - open - 9 Jan 2017
avatar joomla-cms-bot joomla-cms-bot - change - 9 Jan 2017
Category Libraries
avatar mbabker
mbabker - comment - 29 Jan 2017

Would like some tests here. Getting 3.7 in a state where some of the core API is resilient to cache store errors would be a huge improvement for user experience.

avatar alikon
alikon - comment - 1 Feb 2017

I have tested this item successfully on 8f8ba27


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13524.

avatar alikon alikon - test_item - 1 Feb 2017 - Tested successfully
avatar csthomas
csthomas - comment - 1 Feb 2017

I have tested this item successfully on 8f8ba27


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13524.

avatar csthomas csthomas - test_item - 1 Feb 2017 - Tested successfully
avatar jeckodevelopment jeckodevelopment - change - 3 Feb 2017
Milestone Added:
Status Pending Ready to Commit
avatar jeckodevelopment
jeckodevelopment - comment - 3 Feb 2017

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13524.

avatar wilsonge wilsonge - change - 4 Feb 2017
Status Ready to Commit Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 2017-02-04 17:44:55
Closed_By wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 4 Feb 2017
avatar wilsonge wilsonge - merge - 4 Feb 2017

Add a Comment

Login with GitHub to post a comment