Bug 7184 - have mysql returning errors
Summary: have mysql returning errors
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: 3.8
Hardware: All All
: PATCH-Sent (DO NOT USE) enhancement (vote)
Target Milestone: ---
Assignee: Paul Poulain
QA Contact: Ian Walls
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 7119
  Show dependency treegraph
 
Reported: 2011-11-06 06:44 UTC by Paul Poulain
Modified: 2013-12-05 20:04 UTC (History)
0 users

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 7184 raise mySQL error if debug is set (1.67 KB, patch)
2011-11-06 08:37 UTC, Paul Poulain
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Poulain 2011-11-06 06:44:14 UTC
Koha staff interface doesn't complain when there is an SQL error, except in the logs. That can result in strange situations, where the user don't see there is a problem, even if there is.

There is an easy solution to this problem : RaiseError

in C4/Context.pm, the line 
    my $dbh= DBI->connect("DBI:$db_driver:dbname=$db_name;host=$db_host;port=$db_port",
	$db_user, $db_passwd) or die $DBI::errstr;

just has to be replaced by
    my $dbh= DBI->connect("DBI:$db_driver:dbname=$db_name;host=$db_host;port=$db_port",
	$db_user, $db_passwd, {RaiseError => 1}) or die $DBI::errstr;

We also could deal with the DebugLevel systempreference to raise the error only if the library has set it.
Something like: {'RaiseError' => C4::Context->preference("DebugLevel")?1:0}

My preference goes to the 2nd option = raise error only if DebugLevel is set
Comment 1 Paul Poulain 2011-11-06 07:08:22 UTC
silly me (& ian) : ->preference needs a db handler, that is not still open, so it calls new_dbh, that need ->preference ... => forever loop => boom !

We will have to use DEBUG pragma (in koha-httpd.conf)

Patch coming soon !
Comment 2 Paul Poulain 2011-11-06 08:37:32 UTC
Created attachment 6238 [details] [review]
Bug 7184 raise mySQL error if debug is set

Before this patch, if a mySQL occured in Koha (any error), then the user was seeing nothing.
With this patch, if DEBUG is set in Koha VirtualHost, any mySQL error will make Koha die and display the SQL error

Step to test:
=== BEFORE PATCH ===
go anywhere, and change a mySQL statement to make it invalid.
Launch a page using the SQL you've made wrong, you'll see nothing, except you won't see the expected results

=== AFTER PATCH ===
Modify koha-httpd.conf, and add the following line, in the virtualhost (if it does not already exist)
SetEnv DEBUG 1
restart/reload Apache
Launch the same page, you'll get a Perl statement saying something like:

DBD::mysql::st execute failed: <<blablabla SQL error>> at <<where the error was raised line <<error line>>
Comment 3 Ian Walls 2011-11-06 10:43:45 UTC
Simple patch, implements more feedback information using the Environment variable instead of the system preference.

Followup will become necessary: modify etc/koha-httpd.conf to include default values for the DEBUG environment variable.  The choice of this default value should be consistent with the level of debugging already defaulted to in Koha with the syspref.

This will come in very handy whenever atomic update files are not applied along with their companion code; a missing column or system preference value will become very clear very quickly.

Marking as Passed QA
Comment 4 Paul Poulain 2011-11-06 14:43:08 UTC
patch pushed, please test.
Comment 5 Paul Poulain 2011-11-06 14:56:42 UTC
QMaint: I think this patch should be kept for 3.8: if it's activated on 3.6, there may be unexpected errors. I'll propose on koha-devel to have all tester setting DEBUG to 1, but I think it should not be done for 3.6 yet.
Comment 6 Jared Camins-Esakov 2012-12-31 00:24:28 UTC
There have been no further reports of problems so I am marking this bug resolved.