PHP MySQLi Connection File

Question

I have recently adjusted the way I create my mysqli connections. Initially I would include the connection file and use if($mysqli->connect_error)... in my main script. Now I check if there is an issue with the connection within the connection file. If there isn't an issue then I just check that the variable name for the connection is set. Before I continue I'm curious if there are any issues with the way I am doing this and if it goes against any 'best-practices'.

The contents of my connection file: (connect_db_login.php)

<?php
mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
try{
    $test = new mysqli('localhost:3306','login','password','testdb');
    $test->set_charset('utf8');
}catch(Exception $e){ //$e not used; dead code?
    echo('<script> alert("Connection to database failed!");</script>');
    header("refresh:0; url=../login.php"); //using refresh because location doesnt wait for script
}

Checking that the connection has been made in main script:

<?php
    include('../connect_db_login.php'); // using include not require
    if(isset($test)){
    ...

Answer 1

First of all, there are many things in this code which you did right. Setting the proper error reporting mode is brilliant! And always setting the port explicitly is also a very good thing, saving you from many confusions, on Mac especially. Setting the charset is also often forgotten while being very important.

However, too many things are wrong as well - on the conceptual, technological and the implementation levels. To highlight the main problems:

• if an error occurs, a web page should never return an OK signal. It should return an error signal instead. There is a thing called HTTP response code and your page should return different codes in different situations. In case of error it's 500, no 200 which your code currently returns.
• with this code you just left yourself without the explanation, why the error occurred. The error message is invaluable source of information that could help you to fix the issue, but you are simply ditching it!
• writing a dedicated code to handle every distinct error in your script is a waste. Or, to put the other way - why so much attention to the failed database connection? An include could fail as well. Just like there is no special error reporting code for the every include, there shouldn't be dedicated error reporting code for the DB connection as well. If every module would report its errors in its own manner the code would become a complete mess!
• redirecting a user to the login page in case of error just makes no sense. What would they do there? An error page should stay where it is.
• testing for the connection right after include is simply redundant. Can't your connection code already handle the failure? It can.
• database credentials are hardcoded which makes the code less portable

To solve these problems you must separate the error handling from the rest of your code and make it uniform, providing equal treatment for all errors in your application, no matter in which module or in which situation they occur.

Reporting database errors

Do not report them

I do understand where it's coming from. Every [insert your favorite obscene emphasis word here] example on the Net tells you to test for the connection success. So people just habitually reproduce this behavior. Which - if you think of it - is a nonsense. A database error is no different from any other error. If your script says "Oh my, headers already sent!" you don't wrap a header() function call in a try catch. But for some reason a database connection is followed by this honor guard everywhere. Well, just drop this bad habit. Make your database errors equal to all other errors.

Reporting errors in general

I do also understand why the error reporting is so much confused: every PHP apprentice profusely confuses themselves with a site user. No wonder why: during the development, they are the only site user for the long time, and just cannot imagine other people working with the site. But if you just think of it, the difference become apparent - you just need to distinguish a programmer from a site user, as they both require the completely different treatment. A site user doesn't care whether your database connection failed or a hard disk went nuts. All they need to know is that something went wrong but you are already working on it. Then tell them exactly this.

On the other hand, when your site goes live, you don't sit behind every PC browsing your site. This means any specific error information you are displaying will go for naught. On a live site an error must be logged.

Setting the proper charset.

The older default for MySQL, utf8 charset implements only a limited subset of the original UTF-8 standard. Hence, for the full Unicode support, and to avoid "Incorrect string value" errors caused by a mere emoji, always use utf8mb4 charset.

Storing database credentials

Let's digress for a while from the main subject of error reporting and look at the database credentials. The present code is again the direct product of the "I am working alone on my home PC and it's going to be so forever" mindset. When the time comes, your site will go live. And a live site wold definitely have different credentials so you will need to rewrite them. And when you will need to work a bit more on the code, you will have to rewrite them back... and so on. Which is far from being convenient.

There are many advanced techniques for providing the settings but the simplest one would be just storing them in a separate file. This way you'll be able to keep different credentials files on different servers. So instead of hardcoding the credentials, just use variables defined in a separate file:

<?php
$host = '127.0.0.1';
$port = 3306;
$db   = 'test';
$user = 'root';
$pass = '';
$charset = 'utf8mb4';

and then just include this file in your DB connection script

Implementing all the above.

In theory, given the connection shouldn't report errors by itself, only the following lines should be enough:

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
require __DIR__.'/db_credentials.php';
$mysqli = new mysqli($host, $user, $pass, $db, $port);
$mysqli->set_charset($charset);
unset($host, $db, $user, $pass, $charset); // we don't need them anymore

It will raise an error that would be handled elsewhere.

But given we are employing exceptions here, and every exception contains a stack trace, and a stack trace contains every function parameter used, there is a slight chance that database credentials could be leaked. To prevent even such a small chance, it's better to catch the exception and then throw a brand new one, which will have no stack trace and hence no database credentials to leak:

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
try {
    require __DIR__.'/db_credentials.php';
    $mysqli = new mysqli($host, $user, $pass, $db, $port);
    $mysqli->set_charset($charset);
} catch (\mysqli_sql_exception $e) {
     throw new \mysqli_sql_exception($e->getMessage(), $e->getCode());
} finally {
    unset($host, $db, $user, $pass, $charset);
}

Now if an en error occurs, the exception will contain only the error message but not a stack trace. Note that stack traces are extremely helpful for debugging and here we are ditching it only as a tradeoff between the security and usability. But as a rule you should preserve as much debugging information as possible.

So this is it for the connection. What about error reporting in general?

Everything said above is a gist of my article on PHP error reporting, where I have a complete basic universal error handling solution. This code does everything what was said above: in case of error it sets the appropriate HTTP response code, and then changes its behavior depends on the user's type: for a site user it logs the error while showing only a generic text, but for a programmer it will show the error message on-screen.

I wouldn't copy the exact code here because it constantly gets improved over time and better to get the latest version.

How to actually use it?

Just have three files somewhere in your codebase, error_handler.php, mysqli.php, db_credentials.php. Then require the first two in your application.

Answer 2

The second approach i.e. return early with the exit in a conditional block is a good approach -

if(isset($test)===false){
    exit()
}

This keeps nesting levels down for the rest of the code

You shouldn't need to use else but it doesn't hurt. The condition isset($test)===false could be simplified to !isset($test) as well.

While I don't completely follow it, PSR-12 prescribes:

There MUST be one space after the control structure keyword

For the sake of readability please add spaces after if keywords. This also applies to try, catch, finaly.

---

}catch (Exception $e){ //$e not used; dead code?

Perhaps the exception should be logged - e.g. in an error log file, logging service, etc.