CATCH Brain-freeze

The second error is thrown because the bad ttRow record still exists when the procedure (and its temp-table) goes out of scope. You must delete any bad records before they go out of scope to avoid this.  

Why is the UNDO not UNDO'ing the temp-table create? The temp-table is /not/ defined with NO-UNDO so I would have expected it to be UNDOne?

Because you don't have a transaction.

Change your NO-LOCK to EXCLUSIVE-LOCK and you have a transaction and temp-table is being UN-DONE

/Torben

hmm

it does have a 'create' there, sure it must be a transaction block only not sure what Stefan expected to be rolled-back... running the code I only end-up with one record in temp-table and that is for me the expected result (for each transaction in on every block iteration, only second record created gets roll-back)

why the error seems to still propagate even if was catch already escapes me, maybe a nasty bug? :)

Create need to be on a 'real' database object not a temp-table to initiate a transaction!

Interestingly, if I change it as below, I just get the catch error.

DEFINE TEMP-TABLE ttRow 
  FIELD RowIdent AS CHARACTER
  INDEX ttRow1 IS UNIQUE RowIdent.
 
DEFINE VARIABLE i AS INTEGER NO-UNDO.
 
do transaction ON ERROR UNDO, THROW:
  FOR EACH customer NO-LOCK ON ERROR UNDO, THROW:
    CREATE ttRow.
    ASSIGN ttRow.Rowident = "1".
  END.
end. 
 
CATCH e AS PROGRESS.Lang.SysError:
  MESSAGE "I got it! " SKIP(1) e:GetMessage(1) VIEW-AS ALERT-BOX.
END CATCH.

Edit: I should add I'm on 11.2.1

Which was explained by Torben - I did not have a transaction to be undone. If you put the do transaction around the create and assign of the temp-table you also have enough transaction.

The amount of additional undoing and throwing always throws (pun intended) me off structured error handling.

DEFINE TEMP-TABLE ttRow
  FIELD RowIdent AS CHARACTER
  INDEX ttRow1 IS UNIQUE RowIdent.
 
DEFINE VARIABLE i AS INTEGER NO-UNDO.
 
FOR EACH sports2000.customer NO-LOCK ON ERROR UNDO, THROW:
  DO transaction ON ERROR UNDO, THROW:
    CREATE ttRow.
    ASSIGN ttRow.Rowident = "1".
  END.
END.
 
CATCH e AS PROGRESS.Lang.SysError:
  MESSAGE "I got it! " SKIP(1) e:GetMessage(1) VIEW-AS ALERT-BOX.
END CATCH.

Note that one can use the TRANSACTION keyword directly on the FOR EACH block.  

Also note that this issue is independent of the structured error handling. If you remove the catch block you will get more instances of the message. If you remove the ON ERROR , THROW entirely the infinite loop protection seems to get confused (you can get out with a couple of breaks).  

Thanks everyone. Adding the TRANSACTION to the FOR..EACH scope does indeed resolve. I won't pretend to understand exactly why, but it does. :)

The behavior of this code would be the same whether you were using structured error handling or not.  The multiple errors come from the fact that the record create and update were not undone - as you all have said.  It is not related to the fact that the error is being thrown or re-thrown.  For example, if you changed ON ERROR UNDO, THROW to ON ERROR UNDO, LEAVE and you removed the CATCH block, you will see the error 3 times - once on the ASSIGN line where the error is first raised, once when you leave the FOR EACH block and once when you leave the PROCEDURE block (which is the transaction block unless you explicitly code otherwise).  

So I'd be interested to hear more details on your statement that the additional undoing and throwing has discouraged you from using structured error handling.

I agree that there is just as much undoing and leaving as there is undoing and throwing. In the first posts there were multiple (unnecessary) undo, throws which tripped me a bit (along with a missing transaction block)

When CATCH was initially introduced I had hoped that the CATCH would be a more ultimate CATCH (for example at the entry point of an AppServer call), but each block needs to be catching and throwing. Maybe this is unrealistic, but it could have made CATCH a very powerful thing.

Currently I'm only CATCHing OUTPUT TO statements which CATCH can deal with whereas NO-ERROR cannot.

Hi.

"When CATCH was initially introduced I had hoped that the CATCH would be a more ultimate CATCH (for example at the entry point of an AppServer call), but each block needs to be catching and throwing."

That has not been my experience when using the ROUTINE-LEVEL ON ERROR UNDO, THROW. For one of our products, we have a single appserver entry point that catches all App and System errors.

There are some exceptions where we want to catch a specific error to handle it right away before it is thrown back up the stack but the ROUTINE-LEVEL definition seems to help with all the rest. I'm not sure if that is a best-practice, but it has worked for us.

Thank you!!!

ROUTINE-LEVEL ON ERROR UNDO, THROW seems to be the one line I was missing! (or the session startup parameters for the compiler - will need to check availability on 10.2B / 11.2.1)

But... in my top level CATCH - how do I cleanup any mess left behind? In the temp-table example, how do I know I need to undo a temp-table record?  

What happens if I the tt error occurs in a persistent super procedure on the AppServer?

You put a general purpose CATCH in the top level procedure that launches everything.  It you use ROUTINE-LEVEL or BLOCK-LEVEL, that keeps throwing the error up the call chain until caught by that CATCH.

Yes, that part is clear. But what about cleaning up any mess?

The top level procedure has no, and wants no, idea of the work that has been done at lower levels. How does it know what it needs to clean up? In the case of dynamic objects these could be dumped into (named) widget pools or deleted by walking the dynamic object handles.

But what about a persistent super that has errored out with a duplicate (no-undo) static temp-table record? Any subsequent hit to that super will persistently keep erroring since it has nowhere to put the duplicate.

Maybe persistent supers need more care and need to catch and cleanup their own messes?

Everyone should clean up his own mess, `finally` can (and should) be used even with no catch block :)

I for one don't see much use of that one catch-all block, using the new error handling still mean we have to handle those errors somehow and it's most of the time at a lower level than the main start-up routine.

[quote user="medu"]

Everyone should clean up his own mess, `finally` can (and should) be used even with no catch block :)

[/quote]

Fully agree. FINALLY was the best new feature of 10.1C (who needed the dynamic TTY browse widget at all?)

[quote user="medu"]

I for one don't see much use of that one catch-all block, using the new error handling still mean we have to handle those errors somehow and it's most of the time at a lower level than the main start-up routine.

[/quote]

I would say it depends - if an error can be handled by performing an alternative action (e.g. retuning an alternative value from a method, searching a different record, etc.).

However, when all you can do is inform the user that something went wrong, I'm also propagating that there should be a CATCH-all block (if you want to call it that way) at the end of the hand full of entry points to the AppServer and at the end of every (GUI) event handler.

Yes, everyone /should/ clean up their own mess. And yes, all exceptions /should/ be handled. But they are not - so I would /like/ to be able to catch it and clean it.

In the old days of fat client / server a client restart was sufficient to clean up all. An AppServer however is not - just throwing something out there - if an unhandled error is caught on the AppServer then QUIT? But this will probably not get the actual error thrown back to the client?