Project

General

Profile

Feature #8904

implement -catchStop and the integration/conversion of STOP conditions into exceptions (progress.lang.stop and subclasses)

Added by Greg Shah about 2 years ago. Updated about 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
-
Start date:
Due date:
% Done:

100%

billable:
No
vendor_id:
GCD
case_num:
version_reported:
version_resolved:
production:
No
env_name:
topics:

Related issues

Related to Base Language - Feature #4347: add runtime support for STOP-AFTER block option Closed
Related to Base Language - Feature #4373: finish core OO 4GL support New

History

#1 Updated by Greg Shah about 2 years ago

See #4347-19 and following for a discussion about how STOP conditions can be converted into Progress.Lang.Stop exceptions and the impact on control flow. This task is meant to implement the equivalent to the OE command line option -catchStop, the Progress.Lang.Stop and subclasses, and the proper control flow behavior (which will at least involve changes in our structured error handling, OO support and block manager. This is not just about STOP-AFTER but that is certainly included.

#2 Updated by Greg Shah about 2 years ago

  • Related to Feature #4347: add runtime support for STOP-AFTER block option added

#3 Updated by Greg Shah about 2 years ago

#5 Updated by Constantin Asofiei over 1 year ago

  • Assignee set to Eduard Soltan

#6 Updated by Eduard Soltan over 1 year ago

I have tried to write a simple program in OE, using Progress.Lang.Stop. But it seems that it was introduced in OE version 11.7.

But the OE I have available is only 11.6. The error Unable to locate class or interface Progress.Lang.Stop.

#7 Updated by Eduard Soltan over 1 year ago

Are there any tests cases on xfer, which concerns the Progress.Lang.StopAfter behaviour?

#8 Updated by Marian Edu over 1 year ago

Eduard Soltan wrote:

Are there any tests cases on xfer, which concerns the Progress.Lang.StopAfter behaviour?

There are some tests for stop handling in testcases/tests/stop_handling, I do not think catch was used there and we have no tests for Progress.Lang.StopError if this is what you are looking for. Normally in newer versions of OE you can catch stop conditions which wasn't possible before - on stop option was needed - the error thrown when such a stop condition occurs is of type P.L.StopError, I can quickly add a test for this under stop-handling.

#9 Updated by Eduard Soltan over 1 year ago

Marian Edu wrote:

Eduard Soltan wrote:

Are there any tests cases on xfer, which concerns the Progress.Lang.StopAfter behaviour?

There are some tests for stop handling in testcases/tests/stop_handling, I do not think catch was used there and we have no tests for Progress.Lang.StopError if this is what you are looking for. Normally in newer versions of OE you can catch stop conditions which wasn't possible before - on stop option was needed - the error thrown when such a stop condition occurs is of type P.L.StopError, I can quickly add a test for this under stop-handling.

That would be great.

#10 Updated by Constantin Asofiei over 1 year ago

Eduard, keep in mind that also the -catchStop option at the startup parameters need to be implemented - and the new STOP behavior will need to activated only if this option is set.

#11 Updated by Eduard Soltan over 1 year ago

Constantin Asofiei wrote:

Eduard, keep in mind that also the -catchStop option at the startup parameters need to be implemented - and the new STOP behavior will need to activated only if this option is set.

I have added -catchStop parameter in StartupParameters.java, and tried to add the parameter from client.xml. Are there any other places where the parameter should be added, perhaps AppServers, etc...?

#12 Updated by Constantin Asofiei over 1 year ago

Eduard Soltan wrote:

Constantin Asofiei wrote:

Eduard, keep in mind that also the -catchStop option at the startup parameters need to be implemented - and the new STOP behavior will need to activated only if this option is set.

I have added -catchStop parameter in StartupParameters.java, and tried to add the parameter from client.xml. Are there any other places where the parameter should be added, perhaps AppServers, etc...?

Make sure that the client.xml and clientConfig/cfgOverrides for i.e. web client work properly. If that works, then appservers will work, too.

#13 Updated by Marian Edu over 1 year ago

Eduard Soltan wrote:

Marian Edu wrote:

I can quickly add a test for this under stop-handling.

That would be great.

Added a test class in tests/stop_handling/TestCatchStop.cls, committed in testcases rev#1706.

Hope it explains the functionality but if there are questions do let me know, -catchStop was added in OE 11.7 but it needed to be enabled then (set it to 1), on later versions is enabled by default (one can turn it off by setting it to 0). There is a 'special error' - Progress.Lang.Stop that can be used in catch although it doesn't inherit from the base Progress.Lang.Error, there are comments in the test that tries to explain the behaviour.

#14 Updated by Eduard Soltan over 1 year ago

This are some answers the previous questions from #4347-19 which are regarding the Progress.Lang.Stop.

Greg wrote:

The 4GL docs say that when the timeout occurs, both a STOP condition is raised and a Progress.Lang.StopAfter exception will be thrown. This (and the references to enabling the old STOP behavior using -catchStop 0) suggests they have moved to an exception-first approach with STOP where they throw Progress.Lang.Stop or one of its subclasses like Progress.Lang.StopAfter, Progress.Lang.UserInterrupt for CTRL-C and Progress.Lang.LockConflict for record lock timeouts) and probably have an implicit catch block for any blocks that have ON STOP phrases. Is that how it works?

def var a as int.

doBlock1:
do stop-after 10 on stop undo, retry doBlock1:
   if retry then do:
      message "Retry message".
      leave doBlock1.
   end.

   repeat1:
   repeat:
     a = a + 1.
   end.

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

message "End do stop-after block".

Running this little program with -catchStop 1 startup parameter will give the following result:

Catch Stop Exception
End do stop-after block

This suggests that for a block that has ON STOP and Progress.Lang.Stop exception handling it will choose the exception handling path first. After handling the progress.Lang.Stop it will leave the doBlock1 block, completely ignoring ON STOP UNDO, RETRY phrase that we have on doBlock1.

As far as the implicit catch block for any blocks that have ON STOP phrase, I am not sure about that. I tried removing the catch block altogether and still run it with -catchStop 1:

def var a as int.

doBlock1:
do stop-after 10 on stop undo, retry doBlock1:
   if retry then do:
      message "Retry message".
      leave doBlock1.
   end.

   repeat1:
   repeat:
     a = a + 1.
   end.
end.

message "End do stop-after block".

In this case it followed the ON STOP UNDO, RETRY instructions, trying the doBlock1 block.

Does one need to use ROUTINE-LEVEL ON ERROR UNDO, THROW. or BLOCK-LEVEL ON ERROR UNDO, THROW. to get this exception behavior or is it really present in later releases? (extra credit: which OE release brought this?)

I have been testing without anything like that and I was getting the exception thrown, using OpenEdge 11.17.

Does STOP-AFTER add an implicit catch block for Progress.Lang.StopAfter to the block in which it is specified?

I really do not know about the implicit block, but my best guess is to say no. Please look at the following example:

def var a as int.

message "BEFORE DO BLOCK".
do stop-after 5:
   message "INSIDE DO BLOCK".
   repeat:
      a = a + 1.
   end.
end.

message "AFTER DO BLOCK".

As you can observe there is no explicit ON STOP or CATCH Progress.Lang.Stop statements in stop-after do block. In 4gl the message AFTER DO BLOCK will not be reached, so the Progress.Lang.Stop will be propagated up the stack till reaching AVM and closing the program. I think this shows that no implicit catch block is created.

Does using STOP-AFTER provide any behavior for Progress.Lang.Stop instances that are not Progress.Lang.StopAfter?

No, I don't think it has anything to do with adding STOP-AFTER. I think think it only strictly just with error or ON PHRASE handling.

Does it add an implicit ON STOP phrase in the case that exceptions are not being used (-catchStop 0)

I think that the implicit behaviour is ON STOP UNDO, THROW, although we can not define explicitly ON STOP UNDO, THROWN (just LEAVE, NEXT and RETURN).

If you can have catch blocks for Progress.Lang.Stop (or subclasses) in the same block as an ON STOP phrase or a STOP-AFTER phrase, how do they interact?

Yes absolutely, the catch blocks for Progress.Lang.Stop takes precedence over ON STOP phrase. After handling of the error, the 4gl block with a CATCH statement is leaved.

Does an ON STOP clause "catch" all Progress.Lang.Stop exceptions, no matter the subclass?

For sure it catches Progress.Lang.StopAfter and Progress.Lang.UserInterrupt exceptions, also the Progress.Lang.Stop, catches them both. I did not test for Progress.Lang.LockConflict though, but my intuition say it would be the same.

#15 Updated by Eduard Soltan over 1 year ago

Looking at the examples that Marian provided and a those that I made myself, there are a few observations

1) Progress.Lang.Stop extends the Progress.Lang.Object class. And catch ex as Progress.Lang.Error or catch ex as Progress.Lang.StopError will not catch neither progress.Lang.Stop nor STOP condition.

do on stop undo, leave:
   repeat:
      stop.

      // catch P.L.Error doesn't catch the stop statement
      catch oErr as Progress.Lang.Error:
         errObj = oErr.            
      end catch.

      // catch P.L.StopError doesn't catch the stop statement
      catch oErr as Progress.Lang.StopError:
         errObj = oErr.            
      end catch.
   end.
end.

2) at least for the OpenEdge version 11.17, Progress.Lang.Stop exceptions will be catched only with -catchStop 1

3) if ON STOP and CATCH ex as Progress.Lang.Stop are both used in the block, CATCH ex as Progress.Lang.Stop takes precedence.

repeat stop-after 1 on stop undo, leave:
   pause 2 no-message.

   // catch P.L.Stop does catch stop after conditions, even if 'on stop' used for the block
   catch oErr as Progress.Lang.Stop:
      errObj = oErr.            
      leave.
   end catch.
end.

4) for repetitive blocks like (repetitive, for each, etc...) after handling the error it will move to the next iteration of the block.

repeat stop-after 1 on error undo, leave:
   pause 2 no-message.

   // catch P.L.Stop does catch stop after conditions
   catch oErr as Progress.Lang.Stop:
      errObj = oErr.            

      // catching the stop will not leave the repeat loop even if leave is used on error
      numRepeats = numRepeats + 1.
      if numRepeats > 1 then
         leave.
      end catch.
end.

5) in case there is nested blocks with stop-after and catch ex as Progress.Lang.Stop defined. Lets take the following program:

def var a as int.

doBlock0:
do stop-after 15 on stop undo, leave doBlock0:
   doBlock1:
   do stop-after 5 on stop undo, retry doBlock1:
      if retry then do:
         message "Message Retry".
         message "Second message".
         leave doBlock1.
      end.

      doBlock2:
      do stop-after 10 on stop undo, retry doBlock2:
         repeat:
            a = a + 1.
         end.

         catch ex2 as Progress.Lang.Stop:
            message "doBlock2 Stop Block".
         end.
      end.

      message "EXECUTE NEXT".
      catch ex1 as Progress.Lang.Stop:
         message "doBlock1 Stop Block".
      end.
   end.

   message "After doBlock1".

   catch ex1 as Progress.Lang.Stop:
      message "doBlock0 Stop Block".
   end.
end.

message "END".

Stop-After timeout will be reached by doBlock1 first, however the execution is now blocked in doBlokc2 which also have catch Progress.Lang.Stop.

In this case the STOP condition will be catched by the CATCH of doBlock2 messaging doBlock2 Stop Block on the screen, and will propagate the error upwards where it will be catched by the CATCH block of doBlock1 and will message doBlock1 Stop Block on the screen.

From there is will just exit doBlock1 and the execution will continue normally.

#16 Updated by Eduard Soltan over 1 year ago

I tried to see what happens if I put Progress.Lang.Object in a catch block.

def var a as int.

doBlock1:
do stop-after 5 on stop undo, leave:
   repeat:
      a = a + 1.
   end.

   catch ex as Progress.Lang.Object:
      message "Object error".
   end.
end.

But get the following compilation error:

The CATCH and UNDO, THROW statements can only be used with a valid object and
it must either implement Progress.Lang.Error or be compatible with Progress.Lang.Stop. (18968)

#17 Updated by Greg Shah over 1 year ago

Stop-After timeout will be reached by doBlock1 first, however the execution is now blocked in doBlokc2 which also have catch Progress.Lang.Stop.

I don't understand. Are you saying that even though the Progress.Lang.Stop exception is raised inside doBlock2 AND there is a catch block in doBlock2, the Progress.Lang.Stop will be caught in doBlock1? That seems completely broken! It certainly is different than normal nested exception behavior.

#18 Updated by Constantin Asofiei over 1 year ago

Eduard, add FINALLY clause to each block and see how it gets executed. My assumption is that the STOP-AFTER first terminates current block (and exits it) and after that throws this progress.lang.stop. Also, please experiment without the ON STOP clause at doBlock2, or the catch.

#19 Updated by Eduard Soltan over 1 year ago

Greg Shah wrote:

Stop-After timeout will be reached by doBlock1 first, however the execution is now blocked in doBlokc2 which also have catch Progress.Lang.Stop.

I don't understand. Are you saying that even though the Progress.Lang.Stop exception is raised inside doBlock2 AND there is a catch block in doBlock2, the Progress.Lang.Stop will be caught in doBlock1? That seems completely broken! It certainly is different than normal nested exception behavior.

doBlock1:
do stop-after 5 on stop undo, leave:     // Progress.Lang.Stop WILL BE RAISED BY doBlock1 AFTER 5 SECONDS TIMEOUT

   doBlock2:
   do stop-after 30 on stop undo, leave:
      pause 40.                         // PROGRAM WILL PAUSE FOR 40 SECONDS, ONCE doBlock1 WILL RAISE THE Progress.Lang.Stop IT WILL BREAK FROM pause.

      catch ex2 as Progress.Lang.Stop:
         message "doBlock2 CATCH".     // Progress.Lang.Stop RAISED BY doBlock1 WILL BE CAUGHT HERE AND THE MESSAGE WILL BE DISPLAYED.
      end.  
   end.

   catch ex1 as Progress.Lang.Stop:
      message "doBlock1 CATCH".       // EVEN IF Progress.Lang.Stop WAS CAUGHT BY THE CATCH OF doBlock2 IT WILL BE CATCHED BY CATCH BLOCK OF doBlcok1 ALSO.
   end. 
end.

I suspect that Progress.Lang.StopAfter exception will be caught by all the catch blocks in the upward path to the block which raised the Progress.Lang.StopAfter with a stop-after timeout.

#20 Updated by Constantin Asofiei over 1 year ago

What happens if you add an outer doBlock0 which has STOP-AFTER 100 and also catch Progress.lang.stop?

#21 Updated by Eduard Soltan over 1 year ago

Constantin Asofiei wrote:

What happens if you add an outer doBlock0 which has STOP-AFTER 100 and also catch Progress.lang.stop?

doBlock0:
do stop-after 100 on stop undo, leave:

   doBlock1:
   do stop-after 5 on stop undo, leave:

      doBlock2:
      do stop-after 30 on stop undo, leave:
         pause 40.                        

         catch ex2 as Progress.Lang.Stop:
            message "doBlock2 CATCH".
         end.  
      end.

      catch ex1 as Progress.Lang.Stop:
         message "doBlock1 CATCH".
      end. 
   end.

   message "FINISH OF doBlock0".
   catch ex0 as Progress.Lang.Stop:
      message "doBlock0 CATCH".
   end.
end.

message "END OF PROGRAM".

In this case it will exit doBlock1 and will output:

doBlock2 CATCH
doBlock1 CATCH
FINISH OF doBlock0
END OF PROGRAM

CATCH of doBlock0 will not be hit.

#22 Updated by Eduard Soltan over 1 year ago

I have added the following classes Stop, StopAfter, UserInterrupt and LockConflict in com.goldencode.p2j.oo.lang.

However still getting Caused by: java.lang.RuntimeException: Cannot find class/interface Progress.Lang.Stop in PROPATH. at conversion of a program which is using Progress.Lang.Stop. Does someone knows how to modify the PROPATH to get added files?

#23 Updated by Constantin Asofiei over 1 year ago

Eduard Soltan wrote:

I have added the following classes Stop, StopAfter, UserInterrupt and LockConflict in com.goldencode.p2j.oo.lang.

However still getting Caused by: java.lang.RuntimeException: Cannot find class/interface Progress.Lang.Stop in PROPATH. at conversion of a program which is using Progress.Lang.Stop. Does someone knows how to modify the PROPATH to get added files?

You need the .cls skeletons.

Ioana - I know you looked at the new testcases project for these skeletons, and they are there - can you commit them to the skeleton project?

#24 Updated by Ioana-Cristina Prioteasa over 1 year ago

Added skeletons for Progress.Lang.Stop, Progress.Lang.StopAfter and Progress.Lang.UserInterrupt and updated Progress.Lang.LockConflict in the skeleton project, rev. 121.
Eduard, these need to be added in the skeleton of your project.

#25 Updated by Eduard Soltan over 1 year ago

doBlock1:
do on stop undo, leave:

   doBlock2:
   do on stop undo, retry:
      if retry then do:
         message "Retry doBlock1".
         leave doBlock2.
      end.

      message "doBlock2 message".
      stop.
   end.

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

message "Program end".

doBlock2 does have ON STOP condition, but do not have a CATCH ex as Progress.Lang.Stop.
doBlock1 have both a ON STOP and CATCH ex as Progress.Lang.Stop.

STOP will be raised inside doBlock2, and the STOP condition will be handled by the ON STOP phrase of doBlock2.

#26 Updated by Marian Edu over 1 year ago

Eduard Soltan wrote:

CATCH of doBlock0 will not be hit.

This is as expected, the stop condition was handled by doBlock1 and won't bubble up the outer block. It's actually pretty simple, when stop-after (time-out) is used in blocks the block that is timing-out will handle the stop condition - when doing so it will raise stop (actually StopAfter when catch is used) on all inner blocks - this will cascade and fire catch (if present) and the finally.

I will push an update for the test case with asserts on the order the catch/finally blocks get's executed in a bit.

#27 Updated by Eduard Soltan over 1 year ago

  • % Done changed from 0 to 30

Committed on 8904a, revision 15710 - 15711.

1) Added support for -catchStop startup parameter.
2) In case we encouter in 4GL code a CATCH block for Progress.Lang.Stop or one of its subclasses, a catchStop block will be emitted instead of catchError. Added runtime for that also.
3) Added some initial support for differing the StopConditionException in a LegacyStopExcpetion, and some initial handling of the error.
4) Also added the LegacyError's classes.

Note that I haven't performed anything on handling of subclasses of Progress.Lang.Stop (Progress.Lang.StopAfter, Progress.Lang.UserInterrupt and Progress.Lang.LockConflict).

#28 Updated by Eduard Soltan over 1 year ago

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.

#29 Updated by Eduard Soltan over 1 year ago

Committed on 8904a, revision 15712.

Changed handling of the StopConditionExection that has been raised by the remote side.

#30 Updated by Eduard Soltan over 1 year ago

Ioana, could you also add to skeleton the Progress.Lang.StopError class?

#31 Updated by Ioana-Cristina Prioteasa over 1 year ago

Committed to skeleton project, rev. 122: Added Progress.Lang.StopError skeleton.

#32 Updated by Eduard Soltan over 1 year ago

Marian, I have been trying to use the testcases from tests/error_handling and adopt then the Progress.Lang.Stop. Trying to run them in OE I get an error for the following statements support.test.AssertExt:NotErrorNotWarning()

With the following message Multiple markers at this line - ** C:\Users\Administrator\Progress\Developer Studio 4.3.1\workspace3\IndexSelectionUnitProject\tests\TestFunctionExtBlockBlock.p Could not understand line 36. (196) - Unknown database name support. (855)

I FWD it doesn't even pass the parsing phase.

#33 Updated by Eduard Soltan over 1 year ago

Eduard Soltan wrote:

Marian, I have been trying to use the testcases from tests/error_handling and adopt then the Progress.Lang.Stop. Trying to run them in OE I get an error for the following statements support.test.AssertExt:NotErrorNotWarning()

With the following message Multiple markers at this line - ** C:\Users\Administrator\Progress\Developer Studio 4.3.1\workspace3\IndexSelectionUnitProject\tests\TestFunctionExtBlockBlock.p Could not understand line 36. (196) - Unknown database name support. (855)

I FWD it doesn't even pass the parsing phase.

I am moved the necessary files in dataset project. The conversion of the tests files went successfully, but it fails at compilation phase.

Mainly at the following statement undo, return error vAppError., it seems that in FWD there isn't any runtime support for undoReturnErrorTopLevel(AppError err). There are only undoReturnErrorTopLevel() and undoReturnErrorTopLevel(String val)

#34 Updated by Constantin Asofiei over 1 year ago

For #8904-28 - the same happens in FWD for a normal exception. See this:

def var i as int.

repeat i = 1 to 10:
  undo, throw new progress.lang.apperror("a", 1).

  catch ex as progress.lang.error:
     message i ex.
  end.
end.

Eduard, please look into the history of TransactionManager and find if this ever worked. I'm interested if at some point it worked and we decided to move the catch processing in 'popScope'.

#35 Updated by Constantin Asofiei over 1 year ago

Eduard Soltan wrote:

Mainly at the following statement undo, return error vAppError., it seems that in FWD there isn't any runtime support for undoReturnErrorTopLevel(AppError err). There are only undoReturnErrorTopLevel() and undoReturnErrorTopLevel(String val)

Please add it. The support currently I think is only for UNDO, THROW in a top-level block.

#36 Updated by Eduard Soltan over 1 year ago

Constantin Asofiei wrote:

Eduard, please look into the history of TransactionManager and find if this ever worked. I'm interested if at some point it worked and we decided to move the catch processing in 'popScope'.

The part with execution of CATCH in TM.popScope was added to trunk in rev. 14629 (Refactored TM.masterFinish/TM.masterCommit to default to null, and also moved the management of this for CATCH and FINALLY in a common block (for TM.popScope).).

TM.executeBlockTermination only 2 appearances are in TM.popScope.
TM.processFinallyBlock is also executed in iterativeWorker but without the catch block execution.

#37 Updated by Eduard Soltan over 1 year ago

Worked on some tests cases for identifying the propagation behaviour of stop condition from different procedures, functions, methods and different files. Which I will add shortly to xfer testcases project.

A thing noticed Progress.Lang.Stop or any of its derivatives can't be instantiated. However it can be catched in a Progress.Lang.Stop CATCH block, and then rethrown.

However it will cause a compilation error, because Progress.Lang.Stop does not implement LegacyError required for signature undoThrowTopLevel(object < ? extends LegacyError > error).

I am thinking that we should have 2 separate methods for handling Error and Stop conditions: undoThrowTopLevelError(object < ? extends LegacyError > error) and undoThrowTopLevelStop(object < ? extends Stop > stop). But I struggle a bit to find the exact spot where undoThrowTopLevel is emitted during conversion time?

#38 Updated by Marian Edu over 1 year ago

Eduard Soltan wrote:

Marian, I have been trying to use the testcases from tests/error_handling and adopt then the Progress.Lang.Stop. Trying to run them in OE I get an error for the following statements support.test.AssertExt:NotErrorNotWarning()

With the following message Multiple markers at this line - ** C:\Users\Administrator\Progress\Developer Studio 4.3.1\workspace3\IndexSelectionUnitProject\tests\TestFunctionExtBlockBlock.p Could not understand line 36. (196) - Unknown database name support. (855)

I was out of office yesterday, did you solved the issue already? Your workspace doesn't look right, that file should be in tests/error_handling and 'support' isn't a database, it's just calling a static method from support.test.AssertExt class.

I FWD it doesn't even pass the parsing phase.

I would say either you have an incomplete project or it's something wrong with your workspace, other than that all used classes should be added to the file convert list.

#39 Updated by Eduard Soltan over 1 year ago

Marian Edu wrote:

I was out of office yesterday, did you solved the issue already? Your workspace doesn't look right, that file should be in tests/error_handling and 'support' isn't a database, it's just calling a static method from support.test.AssertExt class.

I would say either you have an incomplete project or it's something wrong with your workspace, other than that all used classes should be added to the file convert list.

I managed to pass this point. But thank you very much anyway.

#40 Updated by Eduard Soltan over 1 year ago

I committed on 8904a, rev. 15713.

Handling the stop condition in topLevelBlock and functions.

Added propagation of the StopCondition and Progress.Lang.Stop in topLevelBlock, functionBlock methods. Also added the transaction rollback in case of stop.

One think observed is that the stop will always be thrown to the caller method, no matter where the stop condition occurs. It can be in a procedure, function, a method call.
And it also does not matter whether we have routine-level on error undo, throw or block-level on error undo, throw set or not, the behaviour is still the same.

Honoring the Stop condition

Also changed the way we handle the Thread.interrupt, through TM.honorStopCondition. This the the main way we raise StopCondition's when stop-after timeout occurs or something unexpected happened.

The only places were it is called are in TransactionManager methods: (iterateWorker, popScope, pushScope and processRetry). And in this locations the stop condition that was raised in currently executing block can not be catched or processed by the block itself. And the stop condition will be propagated further. All the methods are called outside the processBody try-catch in coreLoop.

def var a as int.

doBlock1:
do stop-after 50 on stop undo, leave: 

   doBlock2:
   do stop-after 30 on stop undo, leave:
      repeat1:
      repeat stop-after 5 on stop undo, leave repeat1: // Progress.Lang.StopAfter WILL BE RAISED BY doBlock1 AFTER 5 SECONDS TIMEOUT. BECAUSE THE CONVERSATION THREAD WILL BE NOTIFIED OF  
         pause 40.                                     // THREAD INTERRUPT ONLY IN TM.honorStopCondition, FWD WILL COMPLETELY BYPASS THE STOP CONDITION EXCEPTION HANDLING OF THE repeat1 BLOCK.
      end.

      catch ex2 as Progress.Lang.Stop:                 // BECAUSE OF THIS BUG IT WILL ENTER IN THE CATCH BLOCK OF doBlock2 BLOCK, WHERE IN REALITY IT SHOULD HAVE JUST LEAVED THE repeat1 
         message "doBlock2 CATCH".                     // BLOCK.
      end.  
   end.

   catch ex1 as Progress.Lang.Stop:
      message "doBlock1 CATCH".
   end. 
end.

My proposed solution is to call TM.honorStopCondition after every execution of the block body in BM.processBody.

#41 Updated by Eduard Soltan over 1 year ago

Committed 8904a, rev. 15714.

Added conversion and runtime support, for UNDO, THROW ex statement. Where ex instanceof Progress.Lang.Stop.

One thing to observe is that Progress.Lang.Stop object can't be instantiated, but if we do have access to one of this object it can be thrown further.


catch ex as Progress.Lang.Stop:
    message "STOP CATCH BLOCK WHICH PROPAGATES THE ERROR FURTHER." 

    undo, throw ex.
end.

Or it could be saved into a variable, and that one could be thrown.

def var stopError as Progress.Lang.Stop no-undo.

do on stop undo, error:
   stop.

   catch ex as Progress.Lang.Stop:
      stopError = ex.
   end.
end.

undo, throw stopError.

#42 Updated by Eduard Soltan over 1 year ago

Rebased 8904a to trunk revision 15744.

#43 Updated by Eduard Soltan over 1 year ago

One interesting behavior observed, if we throw Progress.Lang.Stop as an error using the UNDO, THROW statement. Throwing the error all the way to the top, if we encouter an on stop undo statement before an catch block for Progress.Lang.Stop.

The Progress.Lang.Stop will follow on stop undo behaviour.

using Progress.Lang.*.

procedure throwStopError:
   do on stop undo, leave:
      stop.

      CATCH ex AS Progress.Lang.Stop:
         undo, throw ex.              // Progress.Lang.Stop OBJECT IS THROWN 
      END.
   end.
end.

test4Block:
do transaction on stop undo, retry:   // WHILE PROPAGATIONG THE Progress.Lang.Stop OBJECT ALL THE WAY TO THE AVM, IT WILL ENCOUNTER THE on stop PHRASE OF test4Block BLOCK AND IT WILL RETRY IT.
   if retry then do:
      message "RETRY test4Block".
      message error-status:error.
      leave test4Block.
   end.

   run throwStopError.
end.

#44 Updated by Eduard Soltan over 1 year ago

Committed on 8904a, rev. 15751.

Added support for propagation of Progress.Lang.StopAfter exception all the way to the block that raised the error after a stop-after timeout. And hitting all the catch ex as Progress.Lang.Stop blocks along the way.

Also make some initial testing on a unit tests of a large GUI application, and didn't not break the application. And in fact all tests pass, but I think it is not very helpful since I don't think it uses this behaviour.

#45 Updated by Eduard Soltan over 1 year ago

  • % Done changed from 30 to 50

#46 Updated by Eduard Soltan over 1 year ago

Hey Marian, I don't understand the following test tests/stop_handling/TestCatchStop.cls:

@Test.
    method public void TestStopConditionCatchError (  ):
        define variable errObj     as Progress.Lang.Error no-undo.
        define variable numRepeats as integer             no-undo.

        // catch stop condition in outer block as needed
        do on error undo, leave on stop undo, leave:

            repeat:
                run nonExistentProcedureHopefully.p.

                // catch P.L.Error does catch stop conditions if catchMode enabled
                catch oErr as Progress.Lang.Error :
                    errObj = oErr.    

                    // catching the error will leave the block
                    numRepeats = numRepeats + 1.
                    if numRepeats > 1 then
                        leave.        
                end catch.
            end.
        end.

        if isCatchStop() then 
        do:
            Assert:IsTrue(valid-object(errObj)).
            Assert:Equals('Progress.Lang.StopError', errObj:GetClass():TypeName).
            Assert:Equals(1, numRepeats).
        end.
        else 
            Assert:IsFalse(valid-object(errObj)).
    end method.

Running run nonExistentProcedureHopefully.p. if nonExistentProcedureHopefully procedure is non-existent should raise a StopConditionException?

If the answear is yes, then catch oErr as Progress.Lang.Error should not catch the STOP condition, right?

Then if I will run the program with -catchStop 1, isCatchStop() method will return true. And the assertions in if block will fail.

I mean does it work in OE, or it should be run with catchStop 0? But other tests in suite require it.

#47 Updated by Eduard Soltan over 1 year ago

Committed on 8904a, rev. 15752.

1) Added runtime support for UNDO, RETURN ERROR exe statement.
2) Cleaning up StopAfterTimer when a StopAfter condition reached the timeout block.

#48 Updated by Eduard Soltan over 1 year ago

I was doing some tests for catching the Progress.Lang.Stop behaviour with nested blocks with stop-after timeouts, on stop phrases and catch ex as Progress.Lang.Stop. And I noticed a very strange behaviour in OE

Example 1:

def var a as int.

doBlock0:
do stop-after 5 on stop undo, leave doBlock0: // STOP-AFTER TIMEOUT IS RAISED BY doBlock0 BLOCK, ALSO doBlock0 DOES NOT HAVE a CATCH FOR Progress.Lang.Stop.
   message "doBlock0 START".

   doBlock1:
   do stop-after 30 on stop undo, leave doBlock1: // doBlock1 HAS A CATCH FOR Progress.Lang.Stop
      message "doBlock1 START".

      doBlock2:
      do stop-after 20 on stop undo, leave doBlock2:  // doBlock2 HAS A CATCH FOR Progress.Lang.Stop
         message "doBlock2 START".

         repeat1:
         repeat:
            a = a + 1. // THE PROGRAM IS STUCK WITH EXECUTION IN THE REPEAT LOOP, WHEN STOP-AFTER INTERRUPT OF doBlock0 BLOCK OCCURS.
         end.

         message "END doBlock2".
         catch ex2 as Progress.Lang.Stop:
            message "doBlock2 CATCH STOP".
         end.
      end.

      message "END doBlock1".
      catch ex1 as Progress.Lang.Stop:
         message "doBlock1 CATCH STOP".
      end.
   end.
end.

message "END doBlock0".

Please observe that doBlock0 block that raised the the stop-after interrupt does not have a CATCH statement for Progress.Lang.Stop. And in the nested blocks down the path to the instruction that was interrupted by Progress.Lang.Stop (repeat block), there are some CATCH statements. The behaviour for this case is very strange from my POV in 4GL, it will infinity retry the execution of doBlock0.

The output looks like this:

doBlock0 START
doBlock1 START
doBlock2 START
doBlock2 CATCH STOP
doBlock1 CATCH STOP

...

doBlock0 START
doBlock1 START
doBlock2 START
doBlock2 CATCH STOP
doBlock1 CATCH STOP

...

And this way, in a infinite loop.
Please notice that doBlock0 has on stop undo, leave doBlock0 phrase, and the behaviour for STOP condition should leave the doBlock0 block. But the de facto behaviour is to retry the doBlock0 block.

However this retry is also very strange.
If I change the doBlock0 in the previous example to:

doBlock0: 
do on stop undo, retry doBlock0:
   message "doBlock0 START".

   if retry then do:
      message "RETRY doBlock0".
      leave doBlock0.
   end.

   message "doBlock0 START".

The behaviour would still be the same, and the if retry block will not be hit.

#49 Updated by Eduard Soltan over 1 year ago

Eduard Soltan wrote:

Please observe that doBlock0 block that raised the the stop-after interrupt does not have a CATCH statement for Progress.Lang.Stop. And in the nested blocks down the path to the instruction that was interrupted by Progress.Lang.Stop (repeat block), there are some CATCH statements. The behaviour for this case is very strange from my POV in 4GL, it will infinity retry the execution of doBlock0.

In fact even if I don't have any CATCH statements for Progress.Lang.Stop on doBlock1 and doBlock2 the behaviour would still be that same, would still be the same.

#50 Updated by Eduard Soltan over 1 year ago

Eduard Soltan wrote:

Please observe that doBlock0 block that raised the the stop-after interrupt does not have a CATCH statement for Progress.Lang.Stop. And in the nested blocks down the path to the instruction that was interrupted by Progress.Lang.Stop (repeat block), there are some CATCH statements. The behaviour for this case is very strange from my POV in 4GL, it will infinity retry the execution of doBlock0.

To make the situation even worse this is the behaviour in OE when run in CHUI mode.

Lets take as a example the following 4GL program:

def var a as int.

outerBlock1:
do stop-after 3 on stop undo, leave outerBlock1:  // STOP-AFTER INTERRUPT WILL BE TRIGGERED AFTER 3 SECONDS, AND THE EXPECTED BEHAVIOUR IS TO LEAVE THE outerBlock1 BLOCK.
   message "Outer Block1".

   innerBlock1:
   do stop-after 8 on stop undo, leave innerBlock1:
      message "Inner Block1".

      if retry then do:
        message "InnerBlock retry".
        leave innerBlock1.
      end.

      repeat1:
      repeat:
         a = a + 1. // EXECUTION WILL BE STUCK IN THE REPEAT BLOCK, WHEN THE STOP-AFTER INTERRUPT OF THE outerBlock1 WILL OCCUR.
      end.
   end.
end.

When run in CHUI mode the following program will output on the screen the following content:

Outer Block1
Inner Block1

...

Outer Block1
Inner Block1

...

When run in GUI mode the output will be the following:

Outer Block1
Inner Block1

#51 Updated by Marian Edu over 1 year ago

Eduard Soltan wrote:

Running run nonExistentProcedureHopefully.p. if nonExistentProcedureHopefully procedure is non-existent should raise a StopConditionException?

I don't know what StopConditionException is, in this case it will raise StopError:https://documentation.progress.com/output/ua/OpenEdge_latest/pdsoe/PLUGINS_ROOT/com.openedge.pdt.langref.help/rre1487108159944.html.

If the answear is yes, then catch oErr as Progress.Lang.Error should not catch the STOP condition, right?

The StopError is actually a P.L.Error not a P.L.Stop so the catch will do catch it :)

Then if I will run the program with -catchStop 1, isCatchStop() method will return true. And the assertions in if block will fail.

I mean does it work in OE, or it should be run with catchStop 0? But other tests in suite require it.

It sure works in OE, if you ran it with catchStop 0 the test methods won't do any assertions. There is for sure one place where we do different asserts to see if the catch works or not based on the catchStop settings but that is simple... it will raise a STOP condition and no catch will work, there are many more tests to see what happens if catchStop is enabled and catch is used.

#52 Updated by Eduard Soltan over 1 year ago

Marian Edu wrote:

I don't know what StopConditionException is, in this case it will raise StopError:https://documentation.progress.com/output/ua/OpenEdge_latest/pdsoe/PLUGINS_ROOT/com.openedge.pdt.langref.help/rre1487108159944.html.

This is helpful, do you know of other situations where the StopError should be raised? Because from documentation is not quite clear.

#53 Updated by Eduard Soltan over 1 year ago

Eduard Soltan wrote:

Marian Edu wrote:

I don't know what StopConditionException is, in this case it will raise StopError:https://documentation.progress.com/output/ua/OpenEdge_latest/pdsoe/PLUGINS_ROOT/com.openedge.pdt.langref.help/rre1487108159944.html.

This is helpful, do you know other situations where the StopError should be raised? Because from documentation is not quite clear.

#54 Updated by Eduard Soltan over 1 year ago

Committed on 8904a, rev. 15753. Added support for raising Progress.Lang.StopError legacy exception, when a procedure name is not found.

#55 Updated by Marian Edu over 1 year ago

Eduard Soltan wrote:

This is helpful, do you know other situations where the StopError should be raised? Because from documentation is not quite clear.

Apart from the run statement that treats external procedure not found case as a stop the only other case I think this could happen is if the database connection is lost - as if the client got disconnected from the server, there might be some other cases but the docs doesn't help much indeed - https://docs.progress.com/bundle/openedge-develop-abl-applications/page/STOP-condition.html

#56 Updated by Eduard Soltan over 1 year ago

Also the following behaviour tests is a also a bit weird in OE:

 @Test.
 method public void TestStopAfterCatchStop (  ):
    define variable errObj     as Progress.Lang.Stop no-undo.
    define variable stopAfter  as logical            no-undo.
    define variable numRepeats as integer            no-undo.
    define variable a          as int                no-undo.

    // catch stop condition in outer block as needed
    do on error undo, leave on stop undo, retry:
       if retry then 
       do:
          stopAfter = true.
          leave.
       end.

       repeat stop-after 1:
          pause 5 no-message.

          // catch P.L.Stop does catch stop after conditions
          catch oErr as Progress.Lang.Stop:
             errObj = oErr.            

             // catching the stop will not leave the repeat loop
             numRepeats = numRepeats + 1.
             if numRepeats > 1 then
                leave.
          end catch.
       end.

    end.

    Assert:IsFalse(stopAfter).
    Assert:IsTrue(valid-object(errObj)).
    Assert:Equals(2, numRepeats).
end method.

Progress.Lang.Stop is raised only after 5 seconds only when pause 5 no-message finishes. And indeed the repeat stop-after 1 is not leaved. However I would expect the condition to be raised after 1 second when the stop-after reaches timeout.

If instead a infinite loop would be used instead of pause 5 no-message, for example:

repeat:
   a = a + 1.
end.

In OE the Progress.Lang.Stop will be raised after 1 second. And after trapping of the error in CATCH block, the repeat stop-after 1 loop will be leaved.

#57 Updated by Constantin Asofiei over 1 year ago

Eduard, does this mean that STOP-AFTER does not affect running interactive UI code, like WAIT-FOR or UPDATE?

#58 Updated by Eduard Soltan over 1 year ago

Constantin Asofiei wrote:

Eduard, does this mean that STOP-AFTER does not affect running interactive UI code, like WAIT-FOR or UPDATE?

Yes, the WAIT-FOR and UPDATE have the same behaviour. I remember it pop-up during the implementation of stop-after, but in the end we moved inserting Ctrl+C on TypeHead queue to trigger interrupt on client side. Which will also cause an interrupt of interactive UI code.

#59 Updated by Eduard Soltan over 1 year ago

  • reviewer Constantin Asofiei added

Constantin, could you take a look at the changes in the branch 8904a, rev. 15755. The overall behaviour is implemented, but some edge cases are not fully handled. It is not ready for a full review, I just what to know if I in the right direction.

#60 Updated by Eduard Soltan over 1 year ago

Marian, do you know if we could actually set a timeout to database related statements like find first book. to specify the time it should wait for the record lock to get free?

#61 Updated by Eduard Soltan over 1 year ago

Eduard Soltan wrote:

Marian, do you know if we could actually set a timeout to database related statements like find first book. to specify the time it should wait for the record lock to get free?

Never mind, I found a startup parameter that does just that -lkwtmo.

#62 Updated by Constantin Asofiei over 1 year ago

Eduard Soltan wrote:

Eduard Soltan wrote:

Marian, do you know if we could actually set a timeout to database related statements like find first book. to specify the time it should wait for the record lock to get free?

Never mind, I found a startup parameter that does just that -lkwtmo.

This was added in OE 12.8 , so yes, is not implemented. Does this mean that LockConflict is raised only when this timeout finishes? Can you try EXCLUSIVE-LOCK NO-WAIT?

#63 Updated by Eduard Soltan over 1 year ago

Constantin Asofiei wrote:

Eduard Soltan wrote:

Eduard Soltan wrote:

Marian, do you know if we could actually set a timeout to database related statements like find first book. to specify the time it should wait for the record lock to get free?

Never mind, I found a startup parameter that does just that -lkwtmo.

This was added in OE 12.8 , so yes, is not implemented. Does this mean that LockConflict is raised only when this timeout finishes? Can you try EXCLUSIVE-LOCK NO-WAIT?

No, when a program waits for a record lock to get free table_name is used by username. Wait or press Ctrl + C to stop. message is shown on the screen. If Ctrl+C is pressed then LockConflict is raised.

#64 Updated by Eduard Soltan over 1 year ago

Constantin Asofiei wrote:

This was added in OE 12.8 , so yes, is not implemented. Does this mean that LockConflict is raised only when this timeout finishes? Can you try EXCLUSIVE-LOCK NO-WAIT?

no-wait just throws an error stating ** book record is locked. (445) and that is all.

#65 Updated by Constantin Asofiei over 1 year ago

Review for 8904a rev 15846:
  • control_flow.rules
    • please create a function for the .equals("Stop") etc condition, as now the check is repeated
  • Control.java
    • remove unneeded p2j.ui imports
  • Queue.java
    • this must not be aware of the exception's type. Why do you need to call ErrorManager.handleStopException there?
  • LockConflict.java, Stop.java, StopAfter.java, UserInterrupt.java
    • needs history entry
  • ConnectionManager.getTimeOut
    • this call is expensive; save this in the PersistenceContext instance, and calculate once.
    • needs javadoc
  • TypeAhead.java
    • line 513 was changed from if (honorServerEvents && EventManager.hasServerEvents()) to if (EventManager.hasServerEvents()) - this doesn't seem right
  • BlockDefinition.java
    • can we not save the stopAfterException here? This type is already kind of 'bloated';
  • BlockManager.java
    • import com.goldencode.p2j.util.BlockManager.Condition; is not needed
    • extra whitespace on line 554/555/9422
    • mustManageLegacyStop - save this SessionUtils._startupParameters().getCatchStop(); in the BlockManager$WorkArea, no need to resolve it every time.
    • processBody - can we call wa.tm.honorStopCondition(); only if -catchStop is enabled? same in processForBody
    • WorkArea.errHelper needs javadoc
    • WorkArea.stopAfterException is never used
  • ErrorManager.java
    • recordForLegacyThrow - add a comment why 293 is treated explicitly there
    • ServerErrorDataAccessor.mustThrowLegacyStop needs javadoc
  • StopConditionException.java
    • needs history entry
  • TransactionManager.java
    • iterateWorker - wa.honorStopCondition(); was removed. why?
  • DeferredLegacyStopException.java, LegacyStopException.java, LockConflictConditionException.java, StopAfterConditionException.java, UserInterruptConditionException.java
    • double-check the Module/Abstract/Copyright year, history entries, class javadoc - some are copy-pasted from an existing file
  • double-check copyright year on all files
  • please rebase
  • please run javadoc and fix any javadoc issues in the changed files

#66 Updated by Constantin Asofiei over 1 year ago

please look for all progress STOP and change to legacy STOP.

#67 Updated by Eduard Soltan over 1 year ago

Constantin Asofiei wrote:

Review for 8904a rev 15846:
  • Queue.java
    • this must not be aware of the exception's type. Why do you need to call ErrorManager.handleStopException there?

This is for action taken in the user (like Ctrl-C) which comes from client side as sub-classes of StopConditionException, and on server we should verify whether there is catch which could handle the error.

  • BlockDefinition.java
    • processBody - can we call wa.tm.honorStopCondition(); only if -catchStop is enabled? same in processForBody

-catchStop command-line parameter refers to trapping condition in catch exe as Progress.Lang.Stop block. But with wa.tm.honorStopCondition(); we verify if the thread as interrupted in the meantime. If -catchStop is set and there is a catch block to handle the condition then a LegacyStopCondition is raised, if not then a simple StopConditionException is raised.

  • TransactionManager.java
    • iterateWorker - wa.honorStopCondition(); was removed. why?

I moved it in processBody before and after the execution of the body. If a Thread interrupt is detected in iterateWorker, then it will bypass the error handling logic of the 4gl block that it is currently in execution.

#68 Updated by Eduard Soltan over 1 year ago

Fixed review issues and rebased to latest trunk.

#69 Updated by Constantin Asofiei over 1 year ago

Review for 8904a rev 15863:
  • control_flow.rules
    • executeLib should be execLib
  • Persistence.java
    • needs history entry
  • InMemoryLockManager.java
    • update copyright year
  • DeferredLegacyStopException
    • missing class javadoc
  • StopConditionException*
    • missing javadoc for logit
  • TransactionManager
    • missing javadoc for stopAfterException
  • UserInterruptConditionException
    • unneeded import
  • double-check copyright year on all files
  • run javadoc if you haven't already
  • re-run your standalone tests and make sure they are committed.
  • after that, do a round of ChUI testing please
  • conversion will need to be tested for all apps (I'll help here), same for runtime; we can coordinate via email

#70 Updated by Constantin Asofiei over 1 year ago

Also, please update the LegacyResourceSupport annotation for the LockConflict and other classes related to oo.lang.Stop.

#71 Updated by Eduard Soltan over 1 year ago

I have committed on the 8904a. Fix for review issues and a fix for a issues that encountered in the conversion of test cases. I am now converting the regression tests project and a large GUI app later tonight.

#72 Updated by Eduard Soltan over 1 year ago

Conversion and runtime testing for CHUI and CTRL-C tests went successfully. ✅
GUI application conversion and unit tests went successfully. ✅

#73 Updated by Constantin Asofiei over 1 year ago

Review for 8904a rev 15865.
  • dependent on the 9470a changes, so 9470a will go first after testing is finished
  • StopError.java - change the LegacyResourceSupport to FULL
  • StopConditionException.java - missing history entry (please double check all files)

#74 Updated by Constantin Asofiei about 1 year ago

Eduard, is it possible to have application-defined Stop sub-classes? I think the conversion rules need to check explicitly if super-class is Stop or not. Please change this function in control_flow.rules:
      <function name="check_stop_error">
         <parameter name="class_name"    type="java.lang.String" />
         <return    name="is_stop_class" type="java.lang.Boolean" />

         <rule>className.equals("Stop")          or 
               className.equals("StopAfter")     or 
               className.equals("UserInterrupt") or 
               className.equals("LockConflict") 
            <action>is_stop_class = true</action>
            <action on="false">is_stop_class = false</action>
         </rule>
      </function>


So that:
  • it receives the fully qualified class name, and not just the simple name
  • it calls a method in CommonAstSupport, isInstanceOf(String, String), where:
    • first argument is the fully qualified name to check
    • second argument is i.e. progress.lang.stop, the super-class (or interface) to check if is a parent
  • it loads the ClassDefinition for these two qualified names, and after that checks recursively the parents, if i.e. progress.lang.stop is found (note that the name match is case-insensitive)

Make isInstanceOf to work with both interfaces and classes (parent and the name to check).

#75 Updated by Eduard Soltan about 1 year ago

Constantin Asofiei wrote:

Eduard, is it possible to have application-defined Stop sub-classes? I think the conversion rules need to check explicitly if super-class is Stop or not. Please change this function in

According to the docs you can't inherit from this class.

You cannot inherit from this class.

#76 Updated by Constantin Asofiei about 1 year ago

Eduard Soltan wrote:

Constantin Asofiei wrote:

Eduard, is it possible to have application-defined Stop sub-classes? I think the conversion rules need to check explicitly if super-class is Stop or not. Please change this function in

According to the docs you can't inherit from this class.

You cannot inherit from this class.

Please confirm this with a test. And if confirmed, change check_stop_error to work with a fully qualified class name, not simple name.

#77 Updated by Constantin Asofiei about 1 year ago

Constantin Asofiei wrote:

You cannot inherit from this class.

Please confirm this with a test.

And also test if you can inherit LockConflict or other known sub-classes of progerss.lang.stop.

#78 Updated by Eduard Soltan about 1 year ago

No, I can't inherit from any of this classes.
It gives the following error, for all subclasses of Progress.Lang.Stop, or for the Stop itself:

Cannot inherit from class 'Progress.Lang.StopAfter' because it has a private constructor. (12920)
** MyStopAfter.cls Could not understand line 1. (196)

#79 Updated by Eduard Soltan about 1 year ago

Committed on 8904a, rev. 15888.

#80 Updated by Constantin Asofiei about 1 year ago

Eduard Soltan wrote:

Committed on 8904a, rev. 15888.

I assume there was a rebase - please push the branch.

#81 Updated by Eduard Soltan about 1 year ago

Constantin Asofiei wrote:

Eduard Soltan wrote:

Committed on 8904a, rev. 15888.

I assume there was a rebase - please push the branch.

Done.

#82 Updated by Constantin Asofiei about 1 year ago

Eduard, please double-check StopConditionException throw throughout the project (i.e. DbUtils.handleException)

After that, we can put this into testing.

Please also update the Status and %Done for this task.

#83 Updated by Eduard Soltan about 1 year ago

  • % Done changed from 50 to 100
  • Status changed from New to WIP

Committed the handling of StopConditionException through ErrorManager.

Conversion of large GUI app and another customer application went successfully.

Smoke tests of a large GUI app and smoke tests went successfully. ☑️
Smoke tests of a large app went successfully. ☑️

Unit test and fwd tests went successfully. ☑️
CHUI regression tests conversion and runtime. ☑️

#84 Updated by Eduard Soltan about 1 year ago

  • Status changed from WIP to Review

#85 Updated by Constantin Asofiei about 1 year ago

  • reviewer Ovidiu Maxiniuc added

The only concern is the usage of isIgnore() in ErrorManager.handleStopException - do we really need this?

Otherwise, please double-check the history entry for all files - DirtyShareContext is missing history.

Ovidiu: please also review the persistence parts.

#86 Updated by Eduard Soltan about 1 year ago

Constantin Asofiei wrote:

The only concern is the usage of isIgnore() in ErrorManager.handleStopException - do we really need this?

It was taken from LegacyError handling logic, I am inclined to say that it is not needed since Stop error is stronger (it stops the program unless explicitly handled). But I am unsure how to test it.

#87 Updated by Ovidiu Maxiniuc about 1 year ago

Review of 8904a/r15892.

Generally, I am OK with the changes. And this is a large change-set. I have just a few comments:

  • Queue.java: line 691: if this condition is true, the reply.getPayload() can be both processed by ErrorManager.handleStopException() and then thrown. If handleStopException() decided to ignore it, shouldn't we return something like unknown object?
  • ConnectionManager.java, new method getTimeOut():
    • a typo in parameter name: dbNmae;
    • the aforementioned parameter is missing from javadoc;
    • this method is not called very often, but we can optimise it by not using the stream API;
    • the Long.valueOf() does not seem necessary;
  • StopConditionException: there is a new static flag, logit. It can be used in tryLog() method instead of obtaining the value again;
  • TextOps.java: line 7040: the old line (currently commented) can be completely dropped;
  • TransactionManager.java:
    • line 11154: the new StopConditionException(errmsg) c'tor should be called on the else branch of the next conditional, avoiding the object to be created if not needed;
    • line 11180: the condition is useless. lex cannot be null since it is the same as stopAfterException which was already checked for null. In fact the local lex can be inlined.
  • the method tryLog(String message, Throwable cause) is implemented multiple time (with same content - code duplication) in the new classes. The similar method from StopConditionException can be called if changed to protected (in relation to above);
  • something strange happened with src/com/goldencode/p2j/oo/lang/LockConflict.java, src/com/goldencode/p2j/oo/lang/Stop.java, src/com/goldencode/p2j/oo/lang/StopAfter.java, src/com/goldencode/p2j/oo/lang/StopError.java, src/com/goldencode/p2j/oo/lang/UserInterrupt.java. They were removed and added back in the same revision. The problem is that the versioning history was lost in the process. I do not think we can recover that without flattening all the revisions from 8904a. Luckily, these are small files with shorter history. Please avoid such operations in the future. If a file needs to be moved, please use the bzr mv [--auto] statement.

#88 Updated by Eduard Soltan about 1 year ago

Ovidiu Maxiniuc wrote:

Review of 8904a/r15892.

Generally, I am OK with the changes. And this is a large change-set. I have just a few comments:

  • Queue.java: line 691: if this condition is true, the reply.getPayload() can be both processed by ErrorManager.handleStopException() and then thrown. If handleStopException() decided to ignore it, shouldn't we return something like unknown object?

In ErrorManager.handleStopException() it will either throw a DeferredLegacyStopException exception, or rethrow the StopConditionExcepetion. I don't there will be any case then the payload will be ignored.

  • ConnectionManager.java, new method getTimeOut():
    • the Long.valueOf() does not seem necessary;

In the majority of cases in the application timeout is used as a long, but ConnectOptionValue only has intValue method to get the int value of the parameter. That is why I had to wrapped as a Long.

  • something strange happened with src/com/goldencode/p2j/oo/lang/LockConflict.java, src/com/goldencode/p2j/oo/lang/Stop.java, src/com/goldencode/p2j/oo/lang/StopAfter.java, src/com/goldencode/p2j/oo/lang/StopError.java, src/com/goldencode/p2j/oo/lang/UserInterrupt.java. They were removed and added back in the same revision. The problem is that the versioning history was lost in the process. I do not think we can recover that without flattening all the revisions from 8904a. Luckily, these are small files with shorter history. Please avoid such operations in the future. If a file needs to be moved, please use the bzr mv [--auto] statement.

In the early stage of the branch I was testing with my own version of this files. But after a rebase I had to pick them up from trunk. I think this why this strange situation happened.

Committed review changes on 8904a, rev. 15893.

#89 Updated by Ovidiu Maxiniuc about 1 year ago

Review of 8904a/r15902

  • ConnectionManager.getTimeOut(): why moving the connect options from opts to options and then performing a map lookup? While opts is iterated we can stop and return the first entry with opt.getKey() == ConnectOption.LOCK_TIMEOUT, if any;
  • StopConditionException.java: can now LOG and logit be restricted to private? All access should be performed using the tryLog() method;
  • I did not notice previously: the new exceptions LockConflictConditionException, StopAfterConditionException, etc have a private static final long serialVersionUID = 1L;. I do not think this is useful (nor are they documented) so I am biased to remove them.

#90 Updated by Eduard Soltan about 1 year ago

Committed on 8904a, rev. 15903.

#91 Updated by Ovidiu Maxiniuc about 1 year ago

  • Status changed from Review to Internal Test

I have no observation related to this revision.

#92 Updated by Eduard Soltan about 1 year ago

Just got news that customer application harness and reports regression testing went succesful. ☑️

I also run Ctrl+C for several times and they were successful. ☑️

#93 Updated by Constantin Asofiei about 1 year ago

Please merge 8904a to trunk after 9877a.

#94 Updated by Eduard Soltan about 1 year ago

8904a was merged to trunk rev. 15887 and archived.

#95 Updated by Constantin Asofiei about 1 year ago

  • Status changed from Internal Test to Test

#96 Updated by Constantin Asofiei about 1 year ago

Eduard, please update Bootstrap_Configuration with for the catch-stop configuration.

#97 Updated by Eduard Soltan about 1 year ago

Constantin Asofiei wrote:

Eduard, please update Bootstrap_Configuration with for the catch-stop configuration.

Edited Bootstrap_Configuration wiki with catch-stop configuration.

#98 Updated by Greg Shah about 1 year ago

  • Status changed from Test to Closed

Also available in: Atom PDF