bigpresh/Dancer-Plugin-Database

$dbh->ping may raise exception, causing crash.

jamesrusso opened this issue · 5 comments

Hello,

I am working with DBI::SQLAnywhere and it's ping method simply calls $dbh->prepare("select 1"). If the database connection has gone away due to timeout, etc, then this will raise an exception during the ping call and will crash that request. This is not the desired result. We should try and re-connect and crash if re-connection fails.

I supposed it really could be done either location, but I see no reason why it shouldn't be done in Dancer-Plugin-Database since it would be beneficial to other DBIs.

thoughts?

-jr

On Wednesday 13 May 2015 11:15:58 James Russo wrote:

Hello,

I am working with DBI::SQLAnywhere and it's ping method simply calls
$dbh->prepare("select 1"). If the database connection has gone away due to
timeout, etc, then this will raise an exception [...]

That seems like odd behaviour, from a method explicitly designed to be called
to find out if the database connection is still responsive; raising an
exception if not seems completely insane behaviour there, to me.

I checked the DBI documentation to see if it offers guidance on what a ping()
method should do, but it is very vague:

"The current default implementation always returns true without actually doing
anything. Actually, it returns "0 but true" which is true but zero. That way
you can tell if the return value is genuine or just the default. Drivers
should override this method with one that does the right thing for their type
of database."

I would prefer to see this fixed in DBD::SQLAnywhere instead, but may consider
catching exceptions within D::P::D for convenience in case any other DBI
drivers act similarly.

I've raised a ticket against DBD::SQLAnywhere for this:
https://rt.cpan.org/Ticket/Display.html?id=104410

If the maintainers of DBD::SQLAnywhere are willing to fix it upstream, then
result, problem solved; if not, I'll work round it in D::P::D for you :)

Ok, sounds good. I threw a quick pull request up for you to consider. I'll follow up with the SQLAnywhere guys.

-jr

I haven't completely tracked it down, but I think my problem is that both an exception is raised and then it is also printed. So, even when I eval and catch the exception in the DBD::SQLAnywhere module, the request still crashes because the error was printed? Still need to do more tracking down. After spending some more time on this, I do think the correct place to fix this is in the DBD::SQLAnywhere module.

So, this is not that simple. The error handling in DBI is executed after the return from the dispatched function (ie: ping). So, you really cannot eval the prepare statement in the ping method because the prepare statement will just save the error until the return of the ping method. The only thing I've found to work for me is to disable RaiseError on the DBH which I really don't want to do for my entire application.

Also interesting is that doing a local $dbh->{RaiseError} = 0 in the ping method still causes does the die due to the prepare failure, not sure why this isn't working..

ambs commented

Merged your solution, for now. If needed, we will readdress this problem.
Thanks.