Look Ma, No Errors!



  • I wrote this PL/SQL code, and it made it through our VAT (verification and testing dept) to production , after all no errors were ever reported!

    FOR loopindex IN temptranstable.FIRST .. temptranstable.LAST LOOP
          card_id := get_card_id(temptranstable(loopindex));
          IF (NOT (card_id IS NULL)) THEN
            errormessage := put_in_order_trans(card_id,
                                               temptranstable(loopindex));
            IF errormessage = 'SUCCESS' THEN
              errormessage := put_in_ordertransdesc(temptranstable(loopindex));
              IF errormessage = 'SUCCESS' THEN
                errormessage := put_in_chargecodetrans(temptranstable(loopindex));

                IF errormessage = 'SUCCESS' THEN
                  put_in_log(temptranstable(loopindex), errormessage);
                  COMMIT;
                  reccount := reccount + 1;
                ELSE
                  put_in_log(temptranstable(loopindex), errormessage);
                  ROLLBACK;
                END IF;
              ELSE
                put_in_log(temptranstable(loopindex), errormessage);
                ROLLBACK;
              END IF;
            ELSE
              put_in_log(temptranstable(loopindex), errormessage);
              ROLLBACK;
            END IF;
          ELSE
            put_in_log(temptranstable(loopindex), 'No matching card id found');
            ROLLBACK;
          END IF;

    OPS :) Good thing it was only dealing with money. :)



  •  "put_in_log()" puts stuff into a table in the database?

    Been there, done that, took a while to figure out...

     



  • @D-Coder said:

     "put_in_log()" puts stuff into a table in the database?

    Been there, done that, took a while to figure out...

     

    Yep. :)



  • @D-Coder said:

     "put_in_log()" puts stuff into a table in the database?

    Been there, done that, took a while to figure out...

    Yep, I've done that too...Luckily a quick fix is to use "EXEC" for the log calls.



  • Looks like a nice place to use the for-case paradigm:

    FOR loopindex IN temptranstable.FIRST .. temptranstable.LAST LOOP
      FOR loopindex2 IN 0 .. 4 LOOP
        CASE loopindex2
        WHEN 0 THEN
          card_id := get_card_id(temptranstable(loopindex));
          IF (NOT (card_id IS NULL)) THEN
            errormessage := 'SUCCESS';
          ELSE
            errormessage :=  'No matching card id found';
          END IF;
        WHEN 1 THEN errormessage := put_in_order_trans(card_id, temptranstable(loopindex));
        WHEN 2 THEN errormessage := put_in_ordertransdesc(temptranstable(loopindex));
        WHEN 3 THEN errormessage := put_in_chargecodetrans(temptranstable(loopindex));
        ELSE
          put_in_log(temptranstable(loopindex), errormessage);
          COMMIT;
          reccount := reccount + 1;
        END IF;
        IF errormessage != 'SUCCESS' THEN
          ROLLBACK;
          put_in_log(temptranstable(loopindex), errormessage);
          EXIT;
        END IF;
      END LOOP;
    END LOOP;


  •  But also ... those commits and rollbacks shouldn't really be inside the loop, should they? If one fails and you are exiting, don't you want all of them to be rolled back so that you end up in the same condition as when you started (apart from the error log messages of course).


     



  • @trashingdays said:

     But also ... those commits and rollbacks shouldn't really be inside the loop, should they? If one fails and you are exiting, don't you want all of them to be rolled back so that you end up in the same condition as when you started (apart from the error log messages of course).

     

     

    Actually, that is exactly what you would want on an error. Their are multiple things that happen when a transaction is being recorded, if any step fails, you want to roll back to before you did anything, other wise you end up with the tables in an erroneous state. For example if we were a store (we are not, but this is easier to show) unable to charge your card, we would need to revert your bill back to the state it was before you paid, ie un paid.

    The hot-fix we applied was to roll back, then write to the logs, then commit. 



  • @VydorScope said:

    @trashingdays said:

     But also ... those commits and rollbacks shouldn't really be inside
    the loop, should they? If one fails and you are exiting, don't you want
    all of them to be rolled back so that you end up in the same condition
    as when you started (apart from the error log messages of course).

     

     

    Actually, that is exactly what you would want on an error. Their are multiple things that happen when a transaction is being recorded, if any step fails, you want to roll back to before you did anything, other wise you end up with the tables in an erroneous state. For example if we were a store (we are not, but this is easier to show) unable to charge your card, we would need to revert your bill back to the state it was before you paid, ie un paid.

    The hot-fix we applied was to roll back, then write to the logs, then commit. 

    But, when you roll back, do you perchance undo any logging of any step which completed before the error occurred?  While this may not be considered a WTF, because the roll back also undoes the action about which the logging was reporting, it could cause some increased difficulty in tracking down the problem.

    A paraphrase of my coworker, the day I was introduced to the concept of logging to a database (many eons ago):

    The code's gone crazy!  It's logging that it's having an error in step 5, apparently because it completely skipped steps 1 through 4.  We need to find out why it's getting to step 5 without steps 1 through 4 happening first!  I need some help tracking this down; I don't see any way this can be happening.



  •  Yes but ... the original is committing on any successful entry. So the subsequent rollback will only rollback the failed one ...

     

    Hence why I said - you should not be committing/rolling back inside the loop ...



  • @tgape said:

    But, when you roll back, do you perchance undo any logging of any step which completed before the error occurred?  While this may not be considered a WTF, because the roll back also undoes the action about which the logging was reporting, it could cause some increased difficulty in tracking down the problem.

    A paraphrase of my coworker, the day I was introduced to the concept of logging to a database (many eons ago):

    The code's gone crazy!  It's logging that it's having an error in step 5, apparently because it completely skipped steps 1 through 4.  We need to find out why it's getting to step 5 without steps 1 through 4 happening first!  I need some help tracking this down; I don't see any way this can be happening.

     

     

    Not exactly. What is logged is something like "Trasaction 15 failed with the follow error..." Then, rolls back 15, and it moves on to 16. Since each success is commited, the only thing that would be roll back is the one tranasction that failed.   It not like 1-14 all succeed, then 15 failes, and we roll back all 15 of them. Just the one that fails. So you know that 15 failed, and you know why. You then fix 15, and resubmit it.



  • @trashingdays said:

     Yes but ... the original is committing on any successful entry. So the subsequent rollback will only rollback the failed one ...

     

     

    Correct. That is the intended / desired behavior. Each trip through the loop represents a complete transaction. So we only want to roll back the failures and let the success commit.


  • "Whoosh!"

    I nearly missed the WTF and I think a few other posters have not understood the utter brilliance of this anti-pattern.  The error is logged into the database and then the transaction is rolled back - removing the error log from the database.  It is so beautiful and clean - there are no errors reported.  It covers its tracks perfectly.

    A close relative is the attempt to log errors to the database such as "cannot connect to database."  Your error reporting mechanism always needs to be independent of the failure.  For a file based system, the error might be "disk full - cannot create log file" so you want some other method of notifying the failure to someone - even if it's an "unexpected error" presented to the end-user.  In this case you either send out the message via another channel (log file, email etc) and then roll back or, as suggested, roll back and then log the error in the database.

    I hope this gets picked up by one of those people that collects lists of anti-patterns.  It looks like an important new example to me.



  • @Qwerty said:

    I nearly missed the WTF and I think a few other posters have not understood the utter brilliance of this anti-pattern.  The error is logged into the database and then the transaction is rolled back - removing the error log from the database.  It is so beautiful and clean - there are no errors reported.

    Yeah, I didn't spot that on my first read-through either. I was thinking the WTF was something along the lines of failing to pick up errors in the logging process - the second anti-pattern you mentioned. Then I was looking at it again and I realised that he was carefully logging the error and then rolling back the log entry as well as the failed transaction.

    That being the case, I think the real WTF is that it passed verification testing. That ought to have checked that when invalid data was presented, the transaction was not processed and the appropriate log entry was made. I suspect they just fired some good data at it and since that got through decided it was OK. It's also possible that they sent some bad transactions and only made sure that they didn't get processed without actually checking the logs, but that seems less likely to me.


Log in to reply