Bug 9941 - with Plack, Web installer should not die with stacktrace when database does not exist
Summary: with Plack, Web installer should not die with stacktrace when database does n...
Status: In Discussion
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (web-based installer) (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Robin Sheat
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-28 02:40 UTC by Jared Camins-Esakov
Modified: 2014-05-06 17:21 UTC (History)
7 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 9941: Web installer should give error message when db connection fails (1.14 KB, patch)
2013-03-30 13:33 UTC, Jared Camins-Esakov
Details | Diff | Splinter Review
Bug 9941: Web installer should give error message when db connection fails (1.19 KB, patch)
2013-03-31 02:29 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9941: Follow up to make the error message print nicely (641 bytes, patch)
2013-04-07 03:53 UTC, Chris Cormack
Details | Diff | Splinter Review
Bug 9941: Web installer should give error message when db connection fails (1.26 KB, patch)
2013-04-12 11:45 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 9941: Follow up to make the error message print nicely (713 bytes, patch)
2013-04-12 11:45 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jared Camins-Esakov 2013-03-28 02:40:09 UTC
When running under Plack with $ENV{DEBUG} set, the installer dies with a stacktrace when checking if the database exists, rather than simply reporting it does not exist. This is not good.
Comment 1 Jared Camins-Esakov 2013-03-30 13:33:20 UTC Comment hidden (obsolete)
Comment 2 Chris Cormack 2013-03-31 02:29:44 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2013-03-31 12:33:36 UTC
I can't get this to work for me, maybe I am missing something:

- Changed the database name in my koha-conf.xml file to a non existing one.
- Tried to access http://localhost:8080/cgi-bin/koha/installer/install.pl and got a perl error message.
- Reinstated correct database name and page can be accessed correctly.

I am not running Plack.

Switching to "Failed QA" for now, waiting for fix/ideas/instructions.
Comment 4 Chris Cormack 2013-04-07 03:53:54 UTC Comment hidden (obsolete)
Comment 5 Kyle M Hall 2013-04-12 11:45:11 UTC
Created attachment 17380 [details] [review]
Bug 9941: Web installer should give error message when db connection fails

To test:
1) Point Koha at a database that does not exist in koha-conf.xml.
2) Start up web installer.
3) Watch web installer crash and burn.
4) Apply patch.
5) Start up web installer.
6) Watch the web installer tell you it failed to connect without any
   sort of horrible error message.

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 6 Kyle M Hall 2013-04-12 11:45:21 UTC
Created attachment 17381 [details] [review]
Bug 9941: Follow up to make the error message print nicely

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall 2013-04-12 11:47:07 UTC
One question. I normally access the web installer by just browsing to the root of the Koha intranet site. If I do that I get the following error:

Software error:

Unknown database 'koha3' at /home/koha/kohaclone/C4/Context.pm line 803.
BEGIN failed--compilation aborted at /home/koha/kohaclone/C4/Auth.pm line 63.
Compilation failed in require at /home/koha/kohaclone/mainpage.pl line 25.
BEGIN failed--compilation aborted at /home/koha/kohaclone/mainpage.pl line 25.
For help, please send mail to the webmaster (webmaster@kohadev), giving this error message and the time and date of the error.

Is it possible to display the same "Unable to connect to database. Please check your database configuration in koha-conf.xml" message for this case?
Comment 8 Chris Cormack 2013-04-13 04:09:11 UTC
(In reply to comment #7)
> One question. I normally access the web installer by just browsing to the
> root of the Koha intranet site. If I do that I get the following error:
> 
> Software error:
> 
> Unknown database 'koha3' at /home/koha/kohaclone/C4/Context.pm line 803.
> BEGIN failed--compilation aborted at /home/koha/kohaclone/C4/Auth.pm line 63.
> Compilation failed in require at /home/koha/kohaclone/mainpage.pl line 25.
> BEGIN failed--compilation aborted at /home/koha/kohaclone/mainpage.pl line
> 25.
> For help, please send mail to the webmaster (webmaster@kohadev), giving this
> error message and the time and date of the error.
> 
> Is it possible to display the same "Unable to connect to database. Please
> check your database configuration in koha-conf.xml" message for this case?

It is, but that isn't the webinstaller throughing that error :) So that might be better on it's bug.

Something like make C4::Context handle dbh errors more nicely
Comment 9 Chris Cormack 2013-04-13 04:09:50 UTC
(In reply to comment #8)

> 
> It is, but that isn't the webinstaller throughing that error :) So that
> might be better on it's bug.

Throughing??? I meant throwing :)
Comment 10 Jonathan Druart 2013-04-19 12:09:48 UTC
Jared, could you confirm this patch is for Plack?

The error message appears without Plack and patches work as expected.

But under Plack, I got a ugly 'An Error Occurred 500 Internal Server Error', without anything in the log files. Same behavior with and without patches.
Comment 11 Mason James 2013-04-27 13:28:08 UTC
(In reply to comment #10)
> Jared, could you confirm this patch is for Plack?
> 
> The error message appears without Plack and patches work as expected.
> 
> But under Plack, I got a ugly 'An Error Occurred 500 Internal Server Error',
> without anything in the log files. Same behavior with and without patches.


hmm, the patch fails for me using plack, too

Jared, shall we hold back for a patch that works with plack too?
Comment 12 Jared Camins-Esakov 2013-04-27 13:30:21 UTC
Since this is a Plack-only patch, yes, we should wait until someone contributes one that works for Plack.
Comment 13 Robin Sheat 2014-04-22 04:09:27 UTC
I did some experimenting with this, and it seems pretty difficult, given that we do so much in BEGIN and I don't see a nice way we can return a meaningful exception from it. If we die() we get a stacktrace.

The long-term best way is to move things more toward lazy initialisation, but in most cases that ship has long sailed.
Comment 14 Galen Charlton 2014-05-06 17:21:19 UTC
Setting severity to normal - this is not a release blocker.