Hmerman6006
Hmerman6006

Reputation: 1913

PHP not throwing specified error message in Class

I have three classes each handling a different functionality when connecting to my database.
Class 1 with name ConnectDB() which handles MySqli() connection; Class 2 with name MessageOut() which gives error and success messages via a $_SESSION['message'] key to the user UI and Class 3 with name WebApp() uses the MySqli connection to retrieve the data from the database and pass it to the UI.

This differs from how I normally do database connection, collection and display. Normally I just created a lot of functions in three files and called them if I needed them. But my site is getting bigger and more complicated so I am re-organising everything.
My current problem is when I create the db connection within my 3rd class it is not throwing the error I programmed.

class ConnectDB {
    //Connecting to database
    public function connect() {
        //connecting to mysql
        @$conn = new mysqli(DB_HOST, DB_USERRW, DB_PASSWRW, DB_DBASE);
        // check validity of database connection
        if (mysqli_connect_errno()) {
            return false;
            exit();
        }
        // select the database to use
        return $conn;
    }
}

ini_set('display_errors', 1); 
    ini_set('log_errors',1); 
    error_reporting(E_ALL); 
    mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
class WebApp {
    protected $db;
    function __construct(){
        $this->connector = new ConnectDB();
        $this->messager = new MessageOut();
        try {
            $this->db = $this->connector->connect();
            if(!$this->db) {
                throw new Exception(' Database connection is currently unavailable.');
            }
        } catch (Exception $e) {
            mysqli_report(MYSQLI_REPORT_OFF);
            $errors = array();
            $message = $e->getMessage();
            //populate array with message and divert output to error class functions
            array_push($errors, $message);
            $this->messager->erroutput($errors);
        }
    }
    public function selectIdata() {
        //select data
        try {
            $query = "SELECT *
                        FROM thetable";
            $stmt = $this->db->prepare($query);
            $stmt->execute();
            $stmt->store_result();

            $stmt->bind_result($idata);
            $result = [];
            if ($stmt->num_rows > 0) {
                while ($stmt->fetch()) {
                    $result[] = $idata;
                }
                return $result;
            } else {
                return false;
            }
        } catch (Exception $e) {
            mysqli_report(MYSQLI_REPORT_OFF);
            $errors = array();
            $message = $e->getMessage();
            //populate array with message and divert output to error class functions
            array_push($errors, $message);
            $this->messager->erroutput($errors);
        }
    }
}

I changed my localhost defined password to the wrong one and loaded the file, but the error is given on line 10 within my 1st class even though the error is suppressed. The idea is to check the connection before using it.
How do I get the error thrown with in my 3rd class giving me the 'Database connection is currently unavailable.' message?

Edit
So I re-assessed what I did and set a try-catch block within the constructor of my 3rd class, but know I get an error: Fatal error: Uncaught Error: Call to a member function prepare() on null in the 3rd class on line 41 where the db connection is used.

Upvotes: 0

Views: 427

Answers (1)

Dharman
Dharman

Reputation: 33237

It looks like you misunderstand the idea of error reporting. I will try to clear up the general concept to you.

Error reporting is to inform the developer of bad coding, bugs and other potential issues which need to be fixed. Error reporting is not for the users of the product. When you are developing you can enable display_errors for yourself, but you should never leave it in the code. You should however always log the errors. PHP has a very good error logger, but I can understand how it might not be enough for you and you would like to log more information together with the error message.

You can write a generic error handler and catch all errors and exceptions thrown by your application and log it to a secure location on the server using your own logger software. Don't catch the exceptions in the middle of your code. Such logger needs to be central to your application and in a separate file to be able to catch all errors.

For this reason, your try-catch is not very useful because you have it in more than one place and it is intertwined with your application code. Also, you are only catching exceptions and disregard errors. You should catch both. Use something like catch(\Throwable $e) to catch both.

@ is an error suppression operator. It should be avoided at all cost. If you can't avoid it then you need to rewrite the code to avoid it. What you are doing there with the mysqli connection you are actually ignoring the error twice. First, you use @ and then you kill your script. Do not kill the script. Do not silence errors. Let your errors bubble up and let them be caught by your error handler.

If you think about it, your class ConnectDB is pretty useless. To connect to the database you always need the same 3 lines. mysqli is already a class so wrapping 3 lines in another class is pointless. The correct code should be nothing more than:

mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
$conn = new mysqli(DB_HOST, DB_USERRW, DB_PASSWRW, DB_DBASE);
$conn->set_charset('utf8mb4');

Your current script exits when mysqli can't connect, but even if you don't silence the errors and you have error reporting switched on, then there is no chance the variable will be empty. if(!$this->db) becomes another fallacy. Besides, why would you want to throw an exception when you just have silenced one? This bring me to another point. Why throw an exception when you immediately catch it? Surely, the whole logic there is nothing more than a simple if statement:

if(!$this->db) {
    $this->messager->erroutput([' Database connection is currently unavailable.']);
}

I see you have named your class MessageOut and I really hope you do not expose the error messages to the user. This is not only terrible user experience, but also a security risk. You should instead implement a nice HTTP 500 error page or if your application is sophisticated enough, your own error page which will be displayed when your error handler catches an error.

Switching off mysqli error reporting once you caught some error serves no purpose. Just remove mysqli_report(MYSQLI_REPORT_OFF); from your code.

To visualise what I am describing consider this code:

<?php

// ini_set('display_errors', 1);
ini_set('log_errors', 1);
error_reporting(E_ALL);

class WebApp {
    protected $db;

    function __construct() {
        $this->messager = new MessageOut();

        mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT); // it can also be at the top of the script, but it makes more sense to put it together with the rest of mysqli code
        $this->db = new \mysqli(DB_HOST, DB_USERRW, DB_PASSWRW, DB_DBASE);
        $this->db->set_charset('utf8mb4');
    }

    public function selectIdata() {
        //select data
        $query = "SELECT *
                    FROM thetable";
        $stmt = $this->db->prepare($query);
        $stmt->execute();
        $stmt->store_result();

        $stmt->bind_result($idata);
        $result = [];
        while ($stmt->fetch()) {
            $result[] = $idata;
        }
        return $result;
    }
}

try {
    $app = new \WebApp();
} catch (\Throwable $e) {
    // log your errors here.
}

It's not the perfect code because the error handler is not there, and it should also be in a separate file, but the general idea is to avoid unnecessary code in your application logic. Do not try-catch. Do not silence the errors and do not add code which serves no purpose. Keep it simple.

Upvotes: 2

Related Questions