Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
jamesrusso opened this issue May 13, 2015 · 5 comments
Closed

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

jamesrusso opened this issue May 13, 2015 · 5 comments

Comments

@jamesrusso
Copy link
Contributor

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

@bigpresh
Copy link
Owner

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 :)

@jamesrusso
Copy link
Contributor Author

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

-jr

@jamesrusso
Copy link
Contributor Author

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.

@jamesrusso
Copy link
Contributor Author

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
Copy link
Collaborator

ambs commented Jan 24, 2016

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

@ambs ambs closed this as completed Jan 24, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants