[#7187] - Add databasequeryerrors logging category
- Closed
- 27 Jun 2015
- Medium
- Build: staging
- Easy Test
- # 7187
- Diff
- Bakual:AddDatabasequeryerrorsLogger
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
Based on dicussions in #7183
Background
Currently, our database drivers are logging database query errors to the category databasequery.
However to actually see those appear in a log, you need to have several conditions met.
Rant
I'm not even sure exactly as I got confused a lot while testing. I think you need to have global debug enabled and in the debug plugin also the "log queries" option enabled. But today it didn't work anymore for me. So I'm not sure how I got them to appear yesterday.
Solution
With this PR the errros would be logged as a new category "databasequeryerror", which will be logged as soon as you have the "Log Almost Everything" parameter enabled in the debug plugin.
With global debug on, you will get the query errors logged twice, because debug will try to run the query again in "explain" mode. That should be fine.
This PR also removes the logging which we introduced with some recently merged PRs. This is no longer neeeded.
It also adjusts the tooltip for the category parameter in the plugin. I thought it may be worth to add the category to the list there.
Testing
- Change a query so it fails. I just renamed the table name in the admin stats module (https://github.com/joomla/joomla-cms/blob/staging/administrator/modules/mod_stats_admin/helper.php#L84) to
#__user. This module is loaded in the Control Panel by default if you have testing data installed so it was a simple test for me. But any query should work. - Enable logging in the debug plugin. The most important is the "Log Almost Everything".
- Load the page and see if any errors are logged in the Joomla logfiles.
- Apply PR and load the page again. You now should see the database query error logged in the "logs/everything.php" logfile.
@Bakual I'm gonna nitpick for a moment and suggest you just call the new category "databaseerror", the current choice is rather excessively verbose in my opinion.
@nikosdion IIRC those log statements were added when all the "handle DB error" PRs were merged, so it's all new stuff and I think based on my comment in #7183 they were added since there wasn't awareness of the existing logging in the database API. Plus contextually a lot of the categories are wrong anyway (why are modules and plugins logging to the controller category?).
@nikosdion These logging I removed only were merged yesterday. So there is no B/C yet with that.
See 5bbb0ed and 06b38e2
The controller category was probably not the best choice to begin with and using a specific category makes it easier to only filter for errors. Together with adding it to the tooltip it should be possible to figure out how to get them.
I'm gonna nitpick for a moment and suggest you just call the new category "databaseerror", the current choice is rather excessively verbose in my opinion.
True that. Going to change 
is it possible to change the name to add hyphenation to increase readbility
- it is always hard to read two words joined together especially when the joining letter is the same eg change to database-error from databaseerror
On 17 June 2015 at 14:06, Thomas Hunziker notifications@github.com wrote:
I'm gonna nitpick for a moment and suggest you just call the new category
"databaseerror", the current choice is rather excessively verbose in my
opinion.True that. Going to change [image:
—
Reply to this email directly or view it on GitHub
#7187 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
joomla-cms-bot
- | Labels |
Added:
?
?
|
||
Changed to database-error
Seemed to work fine for me. Logging to file as well as the display part (with filtering).
| Category | ⇒ | Libraries |
| Status | New | ⇒ | Pending |
| Easy | No | ⇒ | Yes |
| Milestone |
Added: |
||
tested renaming some of the table names involved and works as expected
mssql & postgresql
| Labels |
Added:
?
|
||
| Status | Pending | ⇒ | Ready to Commit |
RTC Thanks 
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/7187.
Ah we have already the label
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-06-27 00:12:23 |
| Closed_By | ⇒ | wilsonge |
Merged - thanks Thomas!
| Labels |
Removed:
?
|
||
I'm not sure if you should remove the extra logging. These messages were logged as "controller", not as "databasequery". It's possible that I'm debugging an issue with my controllers and since I don't want to sift through massive logs I may choose to only log "controller" messages. With the change I'd have to somehow know that I need to log both controller and databasequeryerror messages to debug the controller.
IMHO it's best to log the same message twice under two different contexts than requiring the developer to magically know that he should be looking for an irrelevant context to what they're currently debugging.