Error Checking for PDO Prepared Statements

Question

I'm trying to create proper error handling for queries on a MySQL database using PDO prepared statements. I want the program to exit the moment an error in the prepared statement process is detected. Taking advantage of the fact that each step in the PDO prepared statement process returns False on failure, I threw together this repugnant hack:

 global $allFields;
 global $db;
 global $app;
 //dynamically append all relevant fields to query using $allFields global
 $selectQuery = 'SELECT ' . implode($allFields, ', ') .
     ' FROM People WHERE ' . $fieldName . ' = :value';
 //prepared statement -- returns boolean false if failure running query; run success check
 $success = $selectQueryResult = $db->prepare($selectQuery);
     checkSuccess($success);
 $success = $selectQueryResult->bindParam(':value', $fieldValue, PDO::PARAM_STR);
     checkSuccess($success);
 $success = $selectQueryResult->execute();
     checkSuccess($success);

with checkSuccess() doing the following:

function checkSuccess($success) {
    if ($success == false) {
        //TODO: custom error page. 
        echo "Error connecting to database with this query.";
        die();
    }
}

Two things. First, this is horribly verbose and stupid. There must be a better way. Obviously I could store the booleans in an array or something to take out a line or 2 of code, but still.

Second, is it even necessary to check these values, or should I just check the result after I perform this line of code:

$result = $selectQueryResult->fetch(PDO::FETCH_ASSOC);

I already have code that does this:

if ($result) { //test if query generated results
    // do successful shit
}

else {
    echo "404";
    $app->response()->status(404); //create 404 response header if no results

As much as I try to break the prepared statement process by inserting weird, mismatched, or lengthy queries, my program always makes it to the $result assignment without returning false on any of the functions where I run checkSuccess(). So maybe I don't need to be checking the above logic at all? Keep in mind that I check for a successful database connection earlier in the program.

[scolding]DONT USE `global`s [/scolding] Check this out - http://stackoverflow.com/questions/1557787/are-global-variables-in-php-considered-bad-practice-if-so-why — ShuklaSannidhya, Mar 07 '13 at 16:45

mishu · Accepted Answer · 2013-03-07T16:14:16.277

24

I preffer setting the error mode to throwing exceptions like this:

$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);

right after I connect to the database. So every problem will throw an PDOException So your code would be:

$selectQuery = '
                SELECT 
                    ' . implode($allFields, ', ') . ' 
                FROM 
                    People 
                WHERE 
                    ' . $fieldName . ' = :value
';
try
{ 
    $selectQueryResult = $db->prepare($selectQuery);
    selectQueryResult->bindParam(':value', $fieldValue);
    $selectQueryResult->execute();
}
catch(PDOException $e)
{
    handle_sql_errors($selectQuery, $e->getMessage());
}

where the function would be:

function handle_sql_errors($query, $error_message)
{
    echo '<pre>';
    echo $query;
    echo '</pre>';
    echo $error_message;
    die;
}

In fact I am using a general function that also has something like

$debug = debug_backtrace();
echo 'Found in ' . $debug[0]['file'] . ' on line ' . $debug[0]['line'];

to tell me where was the problem if I am running multiple queries

edited Mar 07 '13 at 16:14

answered Mar 07 '13 at 16:00

mishu

5,197
1
21
37

You should also put the `prepare` statement in the `try` block. – jeroen Mar 07 '13 at 16:01
10

@YourCommonSense care to share more on that? also.. that is just an example that does a similar thing to what the initial code was trying.. it could as well log to a file, send a message or any type of error handling.. the question was about how to catch errors, not what to do with them – mishu Mar 07 '13 at 16:06
+1 to @YourCommonSense for reassuring me that if I'm doing a quick and dirty implementation, leaving out the exception handling will likely report the errors without any additional code. – BlueMonkMN Jan 27 '15 at 16:11
I disagree on the insecurity of try / catch blocks. It all depends upon how you handle them. Throw them out to the display, sure they are, log them some place.. in a file outside the web root, in a database thats not publicly accessible.. these things are more secure. – Chris Apr 29 '15 at 05:29
@YourCommonSense Your comment doesn't make any sense – Dr. Dan Jan 20 '20 at 12:07
@Dr.Dan as the author that he downvoted: I do understand what he means, it's not good practice to catch every exception in your app, maybe just the ones you can recover from, but in genral there should be a general handler. *BUT* I did not change the answer because it is in line with what was asked for, as it had to be the equivalent of an approach that used `die` in case of error. How to design your app architecture it's another issue, the answer covered what was asked. – mishu Jan 20 '20 at 12:29

score 5 · Answer 2 · answered Mar 07 '13 at 15:56

5

You have to catch PDOException:

try {
    //your code/query
} catch (PDOException $e) {
    //Do your error handling here
    $message = $e->getMessage();
}

PDOException

answered Mar 07 '13 at 15:56

Emery King

3,357
18
32

3

And make sure that PDO throws exceptions: `$db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);` – jeroen Mar 07 '13 at 15:58
Is this secure for code deployed to the public? Or does it risk displaying sensitive information about my tables? – user1427661 Mar 07 '13 at 15:59
@user1427661 You should never display technical error messages on a live site. Just make sure they are logged. – jeroen Mar 07 '13 at 16:00
1

So then could I still have the error reporting enabled, but just have the try ... catch block log the error and display a custom error page with a vague statement about database connection failure? – user1427661 Mar 07 '13 at 16:01
-1. NEVER use try..catch just to spit out an error. it is useless, redundant and insecure – Your Common Sense Mar 07 '13 at 16:02
4

@YourCommonSense That's a bit harsh, the code in the `catch` does not actually spit anything out: `//Do your error handling here` sounds about right to me. – jeroen Mar 07 '13 at 16:04

Error Checking for PDO Prepared Statements

2 Answers2

Linked