User tests: Successful: Unsuccessful:
Pull Request for Issue #7428 .
see #7428 (comment)
partial reverted #8130 following the new class JDatabaseExceptionExecuting
and for #10801
simulate a db error
for example change this line
https://github.com/joomla/joomla-cms/blob/staging/components/com_content/models/articles.php#L253
with this
->join('LEFT', '#__ZZZcategories as parent ON parent.id = c.parent_id');
in fronted click on :
Status | New | ⇒ | Pending |
Labels |
Added:
?
|
Category | ⇒ | Components |
I'd suggest keeping the error message & code that the database layer sets versus using a generic error just as @sovainfo suggested. Otherwise it makes the entire Exception thrown here unusable for debugging. The main issue that needs addressing is the arbitrary appending of the SQL query to the error string which was done inline previously then changed to be done in the getErrorMessage()
method of each driver after the stripping of table prefixes. Removing the query from the displayed error message 100% negates the need for said getErrorMessage()
methods.
...maybe we can undiscolse the dbname ...
It's a step forward.
JDatabaseExceptionExecuting
(or its parent RuntimeException
for 3.5 compatibility) should be caught by whatever's making the query and handling the error. In a lot of Joomla code those errors are going uncaught and bubbling up to the top level error handler (JErrorPage::render()
). There isn't a simple fix to address that instead of going through each class/method, deciding what should catch Exceptions and what should allow Exceptions to bubble up, and handling that.
The thing about the getErrorMessage()
methods is that they're really just a big bandage and a lazy way to deal with a partial issue. In well designed code no database error should get caught all the way at the top of the stack, some intermediary should be catching it even if it's just to give a more contextually appropriate error message/code.
For me the first priority is getting the SQL query out of the Exception message, your change in the MySQLi driver accomplishes just that. That same change should be applied to all the drivers and that change merged to core ASAP.
A longer project, or something you could dedicate resources for a 2-3 day code sprint with a handful of folks to to make a dent in the work, is reviewing all the database interactions in core. The MVC layer interactions (components) should all probably be catching errors no matter what. Sorting out what "helper" APIs should be catching and should be bubbling comes next. For example, JInstaller::abort() should probably be catching database exceptions, especially since that's an error handling routine that's trying to gracefully back out a partial extension installation. On the other hand, JPluginHelper::load() should probably catch the database exception and throw a new exception with a more localized error message (Failed to load plugin data) and the database exception chained to the new object so as to "mask" the database error with a contextually appropriate error string but still make the source error available for debugging. Lastly, you've got something like JHtmlAccess::level() which as a pure HTML helper might be better off just letting the Exception bubble and an error handler in whatever is using that JHtml
helper (presumably we're talking about a JForm
or JFormField
object or something in the MVC layer with this specific one) handle catching errors at a higher level and "translate" them as need be.
One last idea (not related to this RFC but should be implemented), in our error.php templates with debugging enabled we're only showing the error details of the "top" exception object. It might be a good idea to add handling to loop while Throwable::getPrevious()
returns an object so that each object in the trace is displayed to help with debugging.
Dislike very much that it says it can't find the page. Don't expect this page in this situation. Assuming it is a component problem expect the template and modules and a message the content is not available. In the case it is a problem with a module it should produce the page. It should be configurable whether to report anything on the page, for both situations. Probably at the menu item level for the component and at the module level for each module.
so let's go step by step
first step :
getting the SQL query out of the Exception message
please some test...
Title |
|
Title |
|
I have tested this item
Check the test result below.
Note that the error message replaced but it is showing two times.
@hardiktailored then test should be unsuccessful as if error message comes two times.
I have tested this item
Changed to unsuccessful as suggested due to error message showing two times.
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-12-30 16:59:53 |
Closed_By | ⇒ | alikon |
Category | Components | ⇒ | Libraries Postgresql MS SQL Components |
Consider the error to generic. It should read something like:
An error has occurred while trying to get the information from the database.
It is up to the application to catch this and turn it into something like:
No content available