fearless359 / simpleinvoices

Fearless359 SimpleInvoices beyond great beginnings
https://simpleinvoices.group
GNU General Public License v3.0
28 stars 7 forks source link

Use built-in error-reporting to improve installation and DB connection errors. #111

Closed redcuillin closed 1 year ago

redcuillin commented 1 year ago

After a recent server change I was confused by the message I received when a database connection failed. The message implied I needed to install custom.config.inc, but it already existed.

I have modified Config.php and Setup.php to:

  1. Catch missing custom.config.inc files and throw appropriate error.
  2. Simplify Setup.php and remove inline HTML in the class while adding correct use of SiError.php class to show appropriate helpful messages to user if DB connections fail.
redcuillin commented 1 year ago

In case it's not clear why I thought a change was needed, here's a screenshot of what I was shown when a database connection error happened: Firefox_Screenshot_2022-12-19T05-18-08 409Z

This didn't immediately suggest a database connection problem to me, while confusing me because custom.config.inc already existed. As a result it slowed down my ability to diagnose the correct problem (the MySQL user did not have permission to connect from the new server's IP).

The message shown also didn't use the built-in error handling class in SI Invoices.

An ini parse error was thrown, rather than a file not found error, if custom.config.inc really did not exist.

fearless359 commented 1 year ago

I don't agree with the change you made. I've supplemented the instructions to include the error you encountered and believe that is sufficient.