FWD » Core Development » Base Language

From #4384-121:

I have FWD fixes for cases of 'nested' calls (for ctors, methods, internal procedures, dynamic functions), which cover RETURN ERROR "foo", RETURN ERROR new AppError, UNDO, THROW ERROR and 'implicit' OE errors (like accessing an attribute in an unknown handle). These work, except the case for RETURN ERROR with nested constructors and default error management (no ROUTINE/BLOCK-LEVEL statements): if this is called from a super-constructor, then 4GL behaves differently - unlike RUN (or other types of calls), the ERROR flag propagates up the stack, until it reaches the NEW operator (and this makes sense, as the object needs to be 'invalidated', because the c'tor failed...); I haven't found a solution in FWD for this, yet.

The other parts which needs to be covered in 4GL tests are:

• direct function calls
• dynamic new
• dynamic object method calls
• external programs
• ON ERROR UNDO, THROW clauses.
• error propagation in nested blocks

From these, I think only the last two may need explicit fixing - as it looks like the ERROR can be 'eaten' by a parent block, even if it doesn't have a CATCH block to treat it.

My tests are in testcases/uast/error_handling (must be converted from this folder, with links to ../cfg/ and ../data/). Marian's tests are in xfer testcases project, error_handling folder.

Another case to cover: simple statements with NO-ERROR, like ch = h:name NO-ERROR. and input from os-dir("missingdir"). This checks for server-side and client-side raised ERROR conditions.

Marian, a weird question: in case of argument validation errors for appserver calls, does the caller's block-level or ROUTINE-LEVEL UNDO, THROW option get 'transferred' to the remote side?

And also, I think I need to check what happens with errors from argument validation for non-appserver RUN (and such) calls.

Constantin Asofiei wrote:

Marian, a weird question: in case of argument validation errors for appserver calls, does the caller's block-level or ROUTINE-LEVEL UNDO, THROW option get 'transferred' to the remote side?

And also, I think I need to check what happens with errors from argument validation for non-appserver RUN (and such) calls.

Constantin, the ROUTINE-LEVEL, BLOCK-LEVEL as well as ON ERROR options do not 'travel' to the other end... it only affects the block on the caller end, if you run something on the appsrv that would lead to an error but that one is shielded by the lack of an ROUTINE-LEVEL, BLOCK-LEVEL on the server side business logic there is no way to influence that from the client side.

Since the remote call is inherently a dynamic call the parameter validation occurs before the remote method is actually executed, in that case an error is thrown back to the client and what happens on the client it depends on error handling on that end.

I'm reproducing some discussion from #4349.

#4349-55:

Greg Shah wrote:

Consider this code:

[...]

LegacyErrorException extends ErrorConditionException which is the equivalent or the ERROR condition in the 4GL. But this code bypasses the ERROR-STATUS processing (recording error/warning state when NO-ERROR is present. The proper processing for ERROR-STATUS is done by the ErrorManager.

Marian wrote:

What we used, and seemed to work fine for our tests, is using undoThrow from BlockManager as in:

undoThrow(SysError.newInstance(String.format("Invalid value specified for %s:GetEnum",
                           ObjectOps.getLegacyName(enumClass)), 15246));

#4349-56:

Greg wrote:

Please note that the code above will result in tracked objects (e.g. in the session object chain) being created for the exceptions. If this is necessary, then it will need some attention as well in ErrorManager.

Marian wrote:

This is correct, the error pop up the stack till is being catch and if is not deleted in that catch block it will linger in the session objects chain - eventually will be garbage collected at some point. It was recommended to delete error objects inside catch block, before they changed the recommendation to not delete anything anymore and let the GC handle everything :)

I'd like to resolve the multiple issues highlighted here.

1. If I understand correctly, any direct usage of new ErrorConditionException(...) will not work properly in code that has structured error handling. But it is not obvious how to leverage ProErro or SysError instances. Without this, I think our catch processing is not going to match the 4GL. The highest priority is in ErrorManager (5 locations) which is used everywhere for proper NO-ERROR support. But I think this affects all of FWD as well. There are 61 other places in FWD that directly construct ErrorConditionException. I think we should create factory methods for construction of the proper errors and then rework the code to use those everywhere.

2. I guess by using SysError.newInstance() we can match the session object chain behavior, but this won't work without modifications.

3. Our session object chain is pinned in memory todat but in the 4GL the chaining is equivalent to using a WeakReference in Java. The 4GL chained references can be garbage collected. I do wonder what that means for the object chain (does the 4GL automatically clean up the chain when something is GC'd or can you get an invalid object reference back). I'm guessing invalid references are possible.

Constantin: please post your thoughts.

I'd like to resolve this ASAP since it will manifest in many bad ways at runtime. Also, I'd like to get a clean and common approach retrofitted everywhere so that in can be consistent in all future work.

Greg Shah wrote:

1. If I understand correctly, any direct usage of new ErrorConditionException(...) will not work properly in code that has structured error handling.

Yes, looks like these will need to be refactored.

But it is not obvious how to leverage ProErro or SysError instances.

I don't think the OE runtime ever throws ProError; I can't find any docs about this. The docs state that only SysError is thrown (and neither can be sub-classed or instantiated by application code).

Without this, I think our catch processing is not going to match the 4GL. The highest priority is in ErrorManager (5 locations) which is used everywhere for proper NO-ERROR support. But I think this affects all of FWD as well. There are 61 other places in FWD that directly construct ErrorConditionException. I think we should create factory methods for construction of the proper errors and then rework the code to use those everywhere.

LegacyErrorException was meant to only be used via undoThrow or undoThrowTopLevel APIs. And the usage in LegacyEnum.getEnum doesn't have a compatible top-level block. This will pose problems, as the current runtime requires all LegacyErrorException usage to be from 4GL-compatible blocks, even if you force a throw new LegacyErrorException.

Note that when implementing the p2j.oo classes, we may need to mark them as ROUTINE-LEVEL UNDO, THROW or BLOCK-LEVEL UNDO, THROW (Marian already did this for some on which he is working). Otherwise, all the management for the OO-style ERROR conditions (like SysError) will not be able to be processed correctly by parent blocks.

2. I guess by using SysError.newInstance() we can match the session object chain behavior, but this won't work without modifications.

I'm not sure I understand what is needed here - this creates a 4GL-compatible ObjectResource. We already do reference tracking and destroy objects once they are no longer referenced, as long as the instance is held in a ObjectVar. And automatically delete any instance which was never assigned in the first place (when the top-level block which created it finishes).

3. Our session object chain is pinned in memory todat but in the 4GL the chaining is equivalent to using a WeakReference in Java. The 4GL chained references can be garbage collected. I do wonder what that means for the object chain (does the 4GL automatically clean up the chain when something is GC'd or can you get an invalid object reference back). I'm guessing invalid references are possible.

We need tests for this. Something like 'create a new instance and save it in var A, check the chain, set var A to unknown, check the chain'. IIRC 4GL cleans after the reference pretty quickly.

But regardless, even if the chain would have invalid references, the application code can't do anything with them. At most it can do a valid-object test before accessing that instance.

I want to avoid using undoThrow() everywhere in our runtime. This was designed to be used from within a BlockManager block context, not from the runtime. I prefer that we continue using EM..recordOrThrowError() etc... and hide the setup of things inside there.

Greg Shah wrote:

I want to avoid using undoThrow() everywhere in our runtime. This was designed to be used from within a BlockManager block context, not from the runtime. I prefer that we continue using EM..recordOrThrowError() etc... and hide the setup of things inside there.

I think at least BlockManager.mustManageLegacyError should be checked before throwing a LegacyErrorException. But ErrorManager.recordOrThrowError is aware of this. So if you add another dedicated API, use mustManageLegacyError.

• pl.Contains - this is from PL/Java and must remain as throw new ErrorConditionException, or some different approach, as we can't check mustManageLegacyError in this 'headless' context.
• persist.TemporaryBuffer usage I think is OK to remain as is (same as DmoAsmWorker usage)
• p2j.ui usage I think can be switched safely
• p2j.util can be switched, except:
  • Agent which should throw new ErrorConditionException, and the AppServerHelper should decide what to do with this
  • SharedVariableManager.errorHelper has a comment there
  • other notes in CFO - I think at least some need to be changed to recordOrThrowError; currently the usage at least for parameter validation seems wrong, as if NO-ERROR clause is set, the error message is not recorded.

This work from #4352 needs to be worked here:

apperror vs syserror - these weren't tested comprehensively, the support is pretty good, but when combined with ROUTINE-LEVEL or BLOCK-LEVEL statements, we may have missing issues (as I recall, we need to convert ERROR condition to SysError class). AppError is used at least by RETURN ERROR statements.

We need to test and fix any deviations.

This will complete both ON THROW and UNDO THROW.

Some things to ensure are fully working:

• CATCH with inner UNDO, THROW
• FINALLY with DELETE OBJECT (without silent mode!)
• BLOCK/ROUTINE-LEVEL used in combination with ON ERROR UNDO, THROW.

Constantin Asofiei wrote:

Attached are automated generated tests covering FINALLY behavior. The fin1prowin.7z contains the logs of these tests when ran in 4GL.

There are also tests on the new testcases project, under tests/error_handling/. Please run these in FWD and check what it fails.

This is a plan to generate tests for CATCH block, in a similar way it was done for FINALLY (and maybe expand this set, also). You can start with what was noted in #4602-16, and expand from that. An exhaustive set would cover all combinations of:

• BLOCK/ROUTINE-LEVEL ON ERROR UNDO, THROW - optional
• types of blocks: REPEAT, DO, FOR, top-level (including OO methods)
• ways to throw an OO exception:
  • RETURN ERROR { "value" | exception }
  • UNDO, THROW exception
  • new apperror with and without a numeric value for the error
  • h:next-sibling (aka raising an ERROR condition)
• error raised from:
  • inner block
  • top-level block
  • catch block
  • and any combinations of the above
• ON ENDKEY/ERROR/QUIT/STOP at one of the (grand)parent block on the stack, with the NEXT/RETRY/LEAVE/RETURN/THROW options.
• zero or multiple CATCH blocks on the stack
• throwing exception from the CATCH block
• transaction behavior - this I think needs to be handled separately, as FINALLY should be included, too.

A lot of that is already included in tests/error_handling, also stop conditions tests are in tests/stop_handling. Of course this is probably not covering everything, finally block for instance was not tested and if I remember correctly the transaction mechanism was only tested against variables (undo vs. no-undo) - actual database transactions is to be covered under persistence. There is a lot of overlapping between various areas, we often end-up testing the same behaviour in many places and sometime is kind of hard to isolate a particular functionality in tests such as the tests won't fail because some other functionality used in those tests :(

Just to double check, is there anything that our team has to do with that "test suite" attached?

Marian Edu wrote:

Just to double check, is there anything that our team has to do with that "test suite" attached?

Not at this time.

I managed to setup the error handling test suite from tests/error_handling.

There were 66 tests failing out of 527.

Majority of problems came from non-existent support for undo, return error new Progress.Lang.AppError('test').

I added support for it, and committed on 4602a, rev. 15761.

With this implemented only 17 tests are failing.

Other problem that appears with tests from tests/error_handling seems to be a conversion issue:

In FWD the following statement throwMultipleAppError(?, input retValue) no-error. converts to silent(() -> FUNC_CALL_SITE_6.clone().execute(retValue));, with just one parameter.

Also there is the following example:

def temp-table tt1
    field f1 as char
    field f2 as char.

def var h as handle.

create tt1.
tt1.f1 = "a1".
tt1.f2 = "a2".
release tt1.

create tt1.
tt1.f1 = "b1".
tt1.f2 = "b2".
release tt1.

for each tt1:
   message tt1.f1.
   stop.

   catch ex as Progress.Lang.Stop:
      message "Progress Catch STOP".
   end.
end.

In 4GL the output will be the following:

a1
Progress Catch STOP
b1
Progress Catch STOP

Meaning that after catching the STOP Error, the block continues to the NEXT iteration of the iterative block.

In progress on the other hand after the error is caught, the iterative block will be left complety.

a1
Progress Catch STOP

I mean this is expected since the execution of the CATCH and FINALLY blocks will be performed at the TransactionManager.popScope() level, after the iterative block is exited.

There is a task with the changes concerning this issue on #9004-71, and some changes are in 9004b.

I made a bit more testscases, concerning the issue:

define variable h as handle.

for each book:
    delete book.
end.

create book.
book.book-id = 1.
book.book-title = "Title1".
release book.

create book.
book.book-id = 2.
book.book-title = "Title2".
release book.

create book.
book.book-id = 3.
book.book-title = "Title3".
release book.

// WHEN AN ERROR IS THROWN, IT IS CAUGHT BY THE CATCH BLOCK AND THE FOR EACH BLOCK IS CONTINUES TO THE NEXT ITERATION.
for each book:
   message book.book-id book.book-title.

   h = h:next-sibling.

   CATCH ex AS Progress.Lang.Error:
      message "ERROR BLOCK" book.book-id book.book-title.
   END CATCH.
end.

// SIMPLE REPEAT BLOCK, THAT SHOULD RUN INDEFINITELY.
// WHEN AN ERROR IS THROWN, IT IS CAUGHT BY THE CATCH BLOCK AND THE INFINITE REPEAT IS EXITED.
repeat:
    h = h:next-sibling.

    CATCH ex AS Progress.Lang.Error :
       message "SINGLE REPEAT ERROR".        
    END CATCH.
end.

def var i as int.

// WHEN AN ERROR IS THROWN, IT IS CAUGHT BY THE CATCH BLOCK AND THE NON-INFINITE REPEAT CONTINUES TO THE NEXT ITERATION.
repeat i = 0 to 10:
    h = h:next-sibling.

    CATCH ex AS Progress.Lang.Error :
       message "REPEAT ERROR " i.      
    END CATCH.
end.

One interesting thing here to note is the simple repeat case.

repeat:
    h = h:next-sibling.

    CATCH ex AS Progress.Lang.Error :
       message "SINGLE REPEAT ERROR".        
    END CATCH.
end.

h = h:next-sibling. will thrown the error and it will be catched by the CATCH ex AS Progress.Lang.Error and the block will be exited.

Error handling¶

Looking at 4gl documentation about CATCH blocks I have seen 2 things of interest.

The CATCH block is an undoable block with implicit ON ERROR UNDO, THROW error handling, and like all blocks, CATCH blocks implicitly throw all stop objects. You cannot explicitly override the implicit ON ERROR UNDO directive for a CATCH block. If a statement within the CATCH block raises ERROR and there is no nested CATCH block, the CATCH block will be undone, and the ERROR will be raised in the block that encloses the associated block of the CATCH block. Error is not raised in the associated block. Otherwise, infinite looping could occur. The same is true if the CATCH block raises STOP.

Thus, a statement that raises ERROR or STOP within a CATCH block causes the following to occur:

1) UNDO the CATCH block.
2) Leave the CATCH block and its associated block.
3) THROW the error or stop object to the block enclosing its associated block. If the CATCH block is at the routine level, the error or stop object gets thrown to the caller of the routine.

In the following 4gl tests case, the error raised in CATCH block will be handled by l1 block on error directive or a catch block inside l1 block:

l1: 
repeat transaction:
   if l1_ran then do: k = k + 1. leave. end.
   l1_ran = true.
   l2: 
   repeat transaction i = 1 to 100:
      k = k + 1. 
      h = h:next-sibling.
      catch err as Progress.Lang.Error:
         k = k + 1.
         undo, throw new Progress.Lang.AppError().
      end.
   end.
   k = k + 1.
   leave.
end.

Flow-of-control directives¶

However what bothers be a lot is the following documentation info:

The code in any CATCH block can contain an explicit flow-of-control directive, meaning LEAVE, NEXT, RETRY, RETURN, or THROW. (RETRY and THROW require UNDO.) Since CATCH is an undoable block, LEAVE, NEXT, and RETRY without a label will apply to the CATCH block itself and not the associated block.

If you want LEAVE, NEXT, or RETRY to apply to the associated block of a CATCH block, you must use the existing label syntax for these statements.

An explicit UNDO, THROW in a CATCH block causes the AVM to raise ERROR or STOP in the block that encloses the associated block of the CATCH block; not the associated block itself.

Here it states that if I don't explicitly specify the block label it will apply to catch block itself. Which i could understand for leave, but for next and retry it is pretty weird.

I tested some combinations of flow-of-control directives in a catch block, and get the following documented the following results.

Note that associated block means the block with the defined catch block which has the flow-of-control directive. And grandparent block I call the parent of the associated block.

More on control-of-flow directives¶

I looked at 4gl for leave, next and retry.

Leave:
The name of the block you want to leave. If you do not name a block, the AVM leaves the innermost iterating block that contains the LEAVE statement. If there is no such block, then the AVM leaves the procedure block.

NEXT:
The name of the block for which you want to start the next iteration. If you do not name a block, the AVM starts the next iteration of the innermost iterating block that contains the NEXT statement.

If I do not specify the label it will leave / next the most innermost iterating block. (could be even the associated block). So I guess there is no problem with it.

UNDO label1, [LEAVE [label2], NEXT [label2]] STATEMENT¶

Here things get a little more complicated. As the behaviour seems to differ in CATCH and in FINALLY. Lets consider the following example:

def var i as int.
def var j as int.
def var k as int.

l1:
repeat transaction i = 1 to 5:

   l2: 
   repeat j = 1 to 5 on error undo, leave:

      k = k + 1. 
      finally:
          k = k + 1.
          undo, leave. 
      end.
   end.
   k = k + 1.
   leave.
end.

message k.

There is undo, leave. in a FINALLY block, which cause the changes to k made in FINALLY to be undone, and the program continue with execution of the next iteration of the l2 labeled block. Same goes with the execution of undo, next and undo, retry.

With the CATCH block it is a different story:

l1:
repeat transaction i = 1 to 5:
   l2: 
   repeat j = 1 to 5 on error undo, leave:

      k = k + 1. 
      undo, throw new Progress.Lang.AppError("HEY").
      catch err as Progress.Lang.Error:
         k = k + 1.
         undo, leave.
      end.
   end.
   k = k + 1.
   leave.
end.

message k.

Either statements undo, leave, undo, next and undo, retry will undo the changes to k variable made in the catch, and leave the associated block. I am thinking that undo in a catch block will cause a leave in the associated block.

I have generated a test suite with errors or control-of-flow condition being thrown in different blocks (do, repeat, for-each, function, procedures and methods).
For all the respecitve blocks, there are tests created that throws errors or uses control-of-flow from the catch blocks. Tests uses alternatively block-level on error undo, throw, routine-level on error undo, throw or none.
Statements used in the tests are next, leave, retry, undo, next, stop, quit, undo, leave, undo, retry, undo, throw new Progress.Lang.Error, return error, return new Progress.Lang.Error and h = h:next-sibiling.

Problems noticed:
1) undo, retry statment in repetive blocks like repeat_to, repeat_to_while, do_to, do_to_while, for_loop_to, for_loop_to_while will move the iteration to the next iteration of the block. With the current version of trunk it will leave the block.

2) there were a couple of issues with the errors and control-of-flow statments in catch blocks.

The code in any CATCH block can contain an explicit flow-of-control directive, meaning LEAVE, NEXT, RETRY, RETURN, or THROW. (RETRY and THROW require UNDO.) Since CATCH is an undoable block, LEAVE, NEXT, and RETRY without a label will apply to the CATCH block itself and not the associated block.

After different experiment I concluded the following things about control-of-flow stamtents in catch blocks:

leave with no label will leave the associated block.

next with no label will go to next iteration.

undo, leave, undo, next and undo, retry with no label at all will have the same effect: undo the chages made in catch block and leave the associated block. Note that it is totally different from these statments in catch blocks. In finally blocks it will go to the next iteration blocks.

undo, leave someLabel, undo, next someLabel and undo, retry someLabel with no label for undo: will undo the chages made in catch block and the subsequent action of the respective label.

undo, leave this statment in catch block will be converted to undoLeave("associated_block_label"). I think that it is not right, it should be converted to undoLeave("") in this way it will get the catch block from the stack and perform the corect action.

3) error throwing in catch blocks mostly works right with some exception.

Failing example found:

Throwing an error in a topLevel block when we have neither block-level un error undo, throw nor routine-level un error undo, throw set, will not propagate the error up to the caller. But when the smae error is thrown inside the catch block of the topLevel block, in this case the error will acually be propagate to the caller.

But I think this relates to a more deep problem with how we handle the error thrown in catch block. In processLegacyError we get the block from the top of the stack (which will be a catch block), and iterate till we get a block with a catch block which could handle the error type thrown. But there are cases when we could match the catch block that we are already in at that moment.
I think that when we detect that we are in a catch block, we should verify the stack starting with 2 levels down.

The problem that I am facing right now. I have some tests with control-of-flow statement and error throwings directly from file procedures. I execute the those procedures with 2 input-output parameters which change the value depending of different scenarios. When the errror is being thrown and the control returns to the caller procedure, those variables does not update their values in FWD but in OE it doesn't. I don;t know whter this is a error handling issue, but this doesn't allow me to test automatically the correctness of the tests.

Eduard Soltan wrote:

The problem that I am facing right now. I have some tests with control-of-flow statement and error throwings directly from file procedures. I execute the those procedures with 2 input-output parameters which change the value depending of different scenarios. When the errror is being thrown and the control returns to the caller procedure, those variables does not update their values in FWD but in OE it doesn't. I don;t know whter this is a error handling issue, but this doesn't allow me to test automatically the correctness of the tests.

Eduard, not sure what is the difference here - I assume in FWD the variables are updated (although you say it `does not`), in OE input-output/output parameters are updated only if there wasn't any error - well unless those parameters are passed by reference (memptr pointer, temp-table, dataset, any handle), otherwise when passed `by value` the value is only update if there wasn't any error.

Marian Edu wrote:

Eduard, not sure what is the difference here - I assume in FWD the variables are updated (although you say it `does not`), in OE input-output/output parameters are updated only if there wasn't any error - well unless those parameters are passed by reference (memptr pointer, temp-table, dataset, any handle), otherwise when passed `by value` the value is only update if there wasn't any error.

Sorry I was more referring that I have problems with my tests. Indeed for errors being thrown in topLevel blocks (function, procedure, method) indeed the value of input-output parameter is not updated in the caller. However for statements like (QUIT, STOP, LEAVE, etc), these values in OE does update in the caller. But in FWD they does not.

Managed to find and solve (hopefully) the problem input-output parameter updates in the caller with different flow-of-control directives.

1) LEAVE, NEXT and RETRY has no effect on the parameters, it will get updated in the caller. (No matter it comes from the main block or from catch/finally block).

2) Different error conditions, will never update the input-output parameter in caller, even if the error itself it will not get propagated to the the caller in case of default error handling behaviour.

3) QUIT will always update the parameters values in the caller. Weird part comes from STOP, if encountered in the main block it will not-update the parameters in the caller, however when encountered in the catch or finally it will.

Committed on 4602a the changes to match this behaviour.

Eduard, for now please commit the tests (the ones I sent and what you wrote) to old testcases project, testcases/uast/error_handling2 (a new folder). Will see how we can migrate these as unit tests.

Committed the testcases to old testcases project under uast/error_handling2.

1) undo, retry statment in repetive blocks like repeat_to, repeat_to_while, do_to, do_to_while, for_loop_to, for_loop_to_while will move the iteration to the next iteration of the block. With the current version of trunk it will leave the block.

Please see rules/annotations/block_properties.rules where we have this note:

      <!-- ON phrases and the UNDO statement can be specified with secondary
           actions (RETRY, LEAVE, NEXT) which have no explicit target (label),
           in such cases the target is always the same as the target of the
           UNDO AND depending on the target block properties, the action may
           be statically converted as in the following:

           1. If the action is LEAVE or NEXT and the target block is a
              top-level block, then the action will be converted to a RETURN.
           2. If the action is NEXT and the target block is a non-iterating
              inner block (it is not a top-level block), then the NEXT will
              be converted to a LEAVE.

           RETURN is an action that never references a label so it is
           ignored in this processing.

           RETRY will automatically be targetted to the same block as a
           pending UNDO by the runtime, so it can be ignored here. This means
           that we avoid emitting an extra string in the converted source
           code, which is better.
           -->

After different experiment I concluded the following things about control-of-flow stamtents in catch blocks:

leave with no label will leave the associated block.

next with no label will go to next iteration.

undo, leave, undo, next and undo, retry with no label at all will have the same effect: undo the chages made in catch block and leave the associated block. Note that it is totally different from these statments in catch blocks. In finally blocks it will go to the next iteration blocks.

undo, leave someLabel, undo, next someLabel and undo, retry someLabel with no label for undo: will undo the chages made in catch block and the subsequent action of the respective label.

undo, leave this statment in catch block will be converted to undoLeave("associated_block_label"). I think that it is not right, it should be converted to undoLeave("") in this way it will get the catch block from the stack and perform the corect action.

Please review rules/annotations/block_properties.rules very carefully. There is a lot of logic in there to calculate the target block when a label is not provided. There is also logic to implicitly convert LEAVE to NEXT, and LEAVE/NEXT to RETURN in certain cases. I want to ensure anything we do here is consistent with our previous findings.

Eduard Soltan wrote:

Committed the testcases to old testcases project under uast/error_handling2.

Please also commit the generator .java programs.

Greg Shah wrote:

1) undo, retry statment in repetive blocks like repeat_to, repeat_to_while, do_to, do_to_while, for_loop_to, for_loop_to_while will move the iteration to the next iteration of the block. With the current version of trunk it will leave the block.

Please see rules/annotations/block_properties.rules where we have this note:

[...]

After different experiment I concluded the following things about control-of-flow stamtents in catch blocks:

leave with no label will leave the associated block.

next with no label will go to next iteration.

undo, leave, undo, next and undo, retry with no label at all will have the same effect: undo the chages made in catch block and leave the associated block. Note that it is totally different from these statments in catch blocks. In finally blocks it will go to the next iteration blocks.

undo, leave someLabel, undo, next someLabel and undo, retry someLabel with no label for undo: will undo the chages made in catch block and the subsequent action of the respective label.

undo, leave this statment in catch block will be converted to undoLeave("associated_block_label"). I think that it is not right, it should be converted to undoLeave("") in this way it will get the catch block from the stack and perform the corect action.

Please review rules/annotations/block_properties.rules very carefully. There is a lot of logic in there to calculate the target block when a label is not provided. There is also logic to implicitly convert LEAVE to NEXT, and LEAVE/NEXT to RETURN in certain cases. I want to ensure anything we do here is consistent with our previous findings.

Yes, I understand that.

Looking at progress documentation regarding catch blocks, there is a interesting part about flow-of-control statements.

The code in any CATCH block can contain an explicit flow-of-control directive, meaning LEAVE, NEXT, RETRY, RETURN, or THROW. (RETRY and THROW require UNDO.) Since CATCH is an undoable block, LEAVE, NEXT, and RETRY without a label will apply to the CATCH block itself and not the associated block.

If you want LEAVE, NEXT, or RETRY to apply to the associated block of a CATCH block, you must use the existing label syntax for these statements.

An explicit UNDO, THROW in a CATCH block causes the AVM to raise ERROR or STOP in the block that encloses the associated block of the CATCH block; not the associated block itself.

And by many experiments I performed with this statements, I can conclude some things:

1) simple statements like leave, next, retry in the body of the catch block have a similar effect on the associated block. leave will leave the associated block (iterative or non-iterative), NEXT it jumps to the next iteration of the iterative block.

2) operation like UNDO, LEAVE, UNDO, NEXT and UNDO, RETRY inside the catch block inside a (repetitive or non-repetitive) will do the same thing. Undo the changes done inside the catch block in case of a transaction, and leave the associated block.

Lets take the UNDO, NEXT as a example:
If this statement will be converted in the same way for both cases whether in the main body of the block or the catch block as undoNext("associated_block_label")..

For the first case, with the statement inside the main block this will match the 4GL behaviour, meaning that it will undo the changes in associated_block and go to the next iteration.

However for the second case with UNDO statement inside a CATCH, this will not match the behaviour of 4gl, since it should undo the changes in CATCH and LEAVE associated block.

If the UNDO statement will be present in the CATCH block of a topLevelBlock it is fine to be converted into undoReturnNormal("") in this way it will rollback the CATCH and return.

The logic from block_properties.rules is take into consideration, it just this case inside a CATCH block that should be treated differently. Passing empty string as a parameter to undoNext("") as a pramter will ensure that operations are performed on the block from the top of stack.

Committed on UAST testcases project under uast/error_handling2/4GLTestsGenerator/ the Java sources used to generate 4gl tests cases.

A little break down of the things performed on 4602a:

1) Added support for catch blocks in iteration blocks. Basically when the error is thrown in the block body and also catched and handled by the CATCH block of the repetive block, it should move to the next iteration of the block.

This is done by calling the executeBlockTermination(wa, blk, blk.possibleCatch, false); with blk.possibleCatch lamda as a parameter in TM.iterateWorker. TM.iterateWorker is called from TM.blockSetup, that is called before every execution of the processBody in BlockManager.

2) Support for catch blocks in iteration blocks introduces some overhead, since there are errors or flow-of-control statemnts (LEAVE, NEXT, RETRY) that could be thrown from CATCH block and they should be handled differently then the error handling of the processBody.

Examples:
- errors thrown from CATCH should be thrown directly to the next block and not handled.
- NEXT of the current block from the CATCH will skip a iteration, if the error is handled by the error handling logic of processBody.

That is why some error handling logic was introduce for the call of TM.blockSetup and TM.popScope.

3) Also there is a need for handling flow-of-control errors from popScope (when we have non-iterative blocks or the last iteration of an iterative block). In order to handle that we should have the access to the current block in TM.blocks. Which is not the case after execution of TM.popScope. That is why I move execution of FINALLY and CATCH blocks in a separate method.

4) to determine if ErrorConditionException in catch blocks should be handled as a LegacyError, we should start the lookup from the parent of the associated block. Otherwise we could match the same catch block we are at the moment when the error was thrown.

5) there is some input-output assignment logic modification in case of flow-of-control statements. See #4602-32

6) also a conversion change for UNDO, LEAVE, UNDO, NEXT and UNDO, RETRY inside catch blocks , basically it will convert these statements in undoLeave(""), undoNext("") to target the current block from the stack. Although this should suffer some modification once 9470a will reach trunk. And some runtime changes to address that in doTerminationBlockWorker.

Committed on 4602a, rev. 15763. Added history entries and java docs.

Also there is the following test case which is not handled in fwd:

define variable h as handle.
DEFINE VARIABLE myCustObj AS CLASS DataModelObject NO-UNDO.

def temp-table tt1
    field f1 as int
    field f2 as int
    field f3 as int.

def var i as int.
def var j as int.

doLabel1:
do transaction:
   repeat j = 1 to 4 on error undo, throw:
      h = h:next-sibling.
   end.

   CATCH ex AS Progress.Lang.Error :
      message "ERROR".
   END CATCH.
end.

message "END".

Catch block of the doLabel1 should be handled.

Eduard, please rebase 4602a.

Rebased 4602a branch.

Eduard, please rebase again.

Constantin Asofiei wrote:

Eduard, please rebase again.

I rebased to trunk. 16335.

I have noticed a bug:

start.p

output to start1.p.

run start1.p.

output close.

start1.p

output to start1.log.

def temp-table tt1 field f1 as int.
def var k as int.
def var h as handle.

create tt1.
tt1.f1 = 4.
release tt1.

for each tt1 :
      k = k + 1. 
      h = h:next-sibling.

      catch err as Progress.Lang.Error:
         k = k + 1.
         h = h:next-sibling.
      end.
end.

output close.

When h = h:next-sibling error is thrown and propagated, start1.log is FileStream is closed. But the problem is that start.log stream is not restored. Solved the problem in rev. 16346.

I will rerun the generated tests, but for now I am facing a different issue (NPE when logging an error in a test file). Tried to create a small test case, but the problem appears only in generated tests (could be a config problem).

I have run the generated tests and the testcases/tests/error_handling test, and they still pass with the rebased version of 4602a.

Eduard, please rebase the 4602a branch and add history entries, fix copyright years and javadocs as needed.

Rebased 4602a to trunk rev. 16401, and added javadocs and history entries.

Rebased 4602a to trunk rev. 16471.

Eduard, please rebase again and lets do full testing (conversion/runtime) on it.

• TransactionManager.iterateWorker is no longer used
• ToClause.decrement()
  • are you saying if an exception is thrown in a catch or finally block, then the iteration is retried with the same counter? Or is it when the block is exit via an exception from catch/finally, it ends with the previous counter value?
  • please make it package-private
• javadoc missing
  • BlockManager.doTerminationBlockWorker for parentLabel
  • BlockManager.hasLegacyError for startLevel; check also the line length in the for block
  • UnnamedStreams$WorkArea.temporaryEvictedOut field

Thanks.

Constantin Asofiei wrote:

• TransactionManager.iterateWorker
• ToClause.decrement()
  • are you saying if an exception is thrown in a catch or finally block, then the iteration is retried with the same counter? Or is it when the block is exit via an exception from catch/finally, it ends with the previous counter value?
  • please make it package-private

It is a leftover from on of the previous implementation of 4602a. In trunk the catch or finally block are executed from TM.blockSetup, which is executed after needsIterate where iterate variable is increased.

Since the catch or finally block execution was moved in TM.blockTermination, executed after the processBody method, it is no longer required.

I tested 4602a branch on regression test project and ETF tests. I managed to get all the tests passing, but had to make a few adjustment.

- commit/rollback of the changes made in the main block has to be executed during after each iteration. (4602a changes has all this logic moved from TM.iterateWorker and TM.popScope to TM.blockTermination). TM.blockTermination has the contains the commit/rollback logic and the termination block logic. So this method has to be executed even with blocks that don't have a termination block at all (GenericFrame.updateWorker, BlockManager.editingBlock).

- processFinalizables and some block flags changes moved from TM.blockTermination back to the next iteration setup.

- about the change in UnnamedStream.

The real problem in the following tests:

start.p

output to start1.p.

run start1.p.

output close.

start1.p

output to start1.log.

def temp-table tt1 field f1 as int.
def var k as int.
def var h as handle.

create tt1.
tt1.f1 = 4.
release tt1.

for each tt1 :
      k = k + 1. 
      h = h:next-sibling.

      catch err as Progress.Lang.Error:
         k = k + 1.
         h = h:next-sibling.
      end.
end.

output close.

Now all the behaviour that I will describe is about trunk:

I start1.p I open up an output stream, and will register its closer on top level procedure return wa.pm.executeOnReturn. If the UnnamedStream is closed before the procedure finishes (UnnamedStream.closeOut() in converted code), it's closer is deregistered from executeOnReturn. Also the code UnnamedStream is registered as Scopeable.

The problem is that Runnable code registered for executeOnReturn is executed before the code scopeFinished of UnnamedStream. Which will definitely leads to invalid state of UnnamedStream.

I fixed this problem by bypassing the registration of closers to executeOnReturn, and execute the close logic from scopeFinished.

I rebased the branch have pushed all the changes to 4602a, rev. 16521. Could you please take a look, especially at UnnamedStream?

+ Constantin

Rebase 4602a to trunk rev. 16588.

Rebased 4602a to trunk rev. 16642. Please review.