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.
Created attachment 17041 [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.
Created attachment 17070 [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>
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.
Created attachment 17228 [details] [review] Bug 9941: Follow up to make the error message print nicely
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>
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>
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?
(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
(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 :)
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.
(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?
Since this is a Plack-only patch, yes, we should wait until someone contributes one that works for Plack.
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.
Setting severity to normal - this is not a release blocker.
In 2021, with dev_install=0, if the DB does not exist the client get a standard 500 "Internal Server Error". No stacktrace displayed. Closing.