4

I'm about to implement transactions in my php scripts and I'm doing some testing to help myself understand exactly how they work. I have the following code snippet:

try{
    $db->beginTransaction();

    $update = "UPDATE persons SET first_name = 'Adam' WHERE person_id = 4";
    $stmt = $db->exec($update);


    $select = "SELECT person_id, column_that_doesnt_exist FROM persons";

    try{
        $stmt = $db->prepare($select);
        $stmt->execute();
        $rows = $stmt->fetchAll(PDO::FETCH_ASSOC);
        echo json_encode('success');
    } catch (PDOException $e) {
        echo 'execute failure';
    }

    echo ' code gets here';

    $db->commit();
} catch(PDOException $e){
    $db->rollBack();
    echo json_encode('commit failure');
}

which outputs:

execute failure code gets here

And person 4's first name is updated to Adam.

Now I'm pretty sure that it's getting committed because the second query never actually failed, because it was never actually executed since the prepare was the point of failure.

It would be nice if a PDOException was thrown for the last catch since one was thrown in the "inner" try but I can work around that.

Now if I take out the "inner" try and have this code:

try{
        $db->beginTransaction();

        $update = "UPDATE persons SET first_name = 'Adam' WHERE person_id = 4";
        $stmt = $db->exec($update);


        $select = "SELECT person_id, column_that_doesnt_exist FROM persons";


            $stmt = $db->prepare($select);
            $stmt->execute();
            $rows = $stmt->fetchAll(PDO::FETCH_ASSOC);


        $db->commit();
    } catch(PDOException $e){
        $db->rollBack();
        echo json_encode('commit failure');
    }

the commit fails, db is rolled back and outputs commit failure as expected.

So in production, should I NOT wrap each individual statement in a try-catch, and instead throw all of my statements inside one big try block (transaction), then catch the commit Exception at the end? This doesn't seem right to me....and likewise wouldn't give you much info about which statement failed....

A.O.
  • 3,733
  • 6
  • 30
  • 49
  • you should wrap code only if you have other scenario how to catch and restore it.. if anythere it will rollback of all transaction, then no reason for this – vp_arth Apr 08 '14 at 20:15

2 Answers2

1

You catch the exception using the inner try..catch block, without rethrowing it. That means that that exception is handled, and the code continues as if nothing went wrong, committing the transaction.

Normally this is not the desired solution, since transactions are mainly used to make a combination of multiple data modifying statements atomic. By using a transaction, you can commit everything if everything went well, or rollback everything when something went wrong.

Also, since there is nothing to commit or rollback when starting the transaction itself would fail, you should pull that out of the exception handling. So the proper structure would be:

StartTransaction;
try {
  ModifyData;
  CommitTransaction;
}
catch {
  RollbackTransaction;
  // Log/mail/show/ignore error
}

If you want to continue inserting records even if inserting one of them fails, you don't need a transaction.

If you want to get specific information about which item failed, you can just rethrow the exception or throw a new one:

StartTransaction;
try {

  foreach ($persons as $person) {
    try {
      ModifyPerson($person);
    }
    catch {
      throw new Exception("Updating person {$person->name} failed");
    }
  }

  CommitTransaction;
}
catch {
  RollbackTransaction;
  // Log/mail/show/ignore error
}

By (re) throwing an exception from the inner exception handler, you jump directly to the outer exception handler, terminating the loop, and bypassing commit.

GolezTrol
  • 114,394
  • 18
  • 182
  • 210
  • This is exactly what I was looking for! I did not know you could throw the exception and skip to the outer handler, that is what I would like to do so I could give the user (and log) a little info on why exactly the request failed. Thanks for the info, this helped a lot! I should have read up a little more on Exception handling... – A.O. Apr 08 '14 at 20:35
0

Remove that inner try catch block.

Get the info of the error with echo $e->getMessage(); // take care not to show message in critical spots or functions (to the user), as this message could reveal db info and even your password.

You can't get more details in this specific case the way you doing. Why should one fail and not the others? But doing something like getting all rows first and then update 1 by 1(and here with an inner try catch or saving the current row) for getting the especific row that gives the error is much more nonsense, i think.

Cristo
  • 700
  • 1
  • 8
  • 20
  • you are inner wrapping an execution on all the records at once. the inner block doesn't do what you describe – Cristo Apr 08 '14 at 20:23