Feature #8904
implement -catchStop and the integration/conversion of STOP conditions into exceptions (progress.lang.stop and subclasses)
100%
Related issues
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
- Related to Feature #4373: finish core OO 4GL support added
#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.StopAfterbehaviour?
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.StopAfterbehaviour?There are some tests for stop handling in
testcases/tests/stop_handling, I do not thinkcatchwas used there and we have no tests forProgress.Lang.StopErrorif this is what you are looking for. Normally in newer versions of OE you cancatchstop conditions which wasn't possible before -on stopoption was needed - the error thrown when such astopcondition occurs is of typeP.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
-catchStopoption 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
-catchStopoption 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
-catchStopparameter inStartupParameters.java, and tried to add the parameter fromclient.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
doBlock1first, however the execution is now blocked indoBlokc2which also have catchProgress.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
doBlock1first, however the execution is now blocked indoBlokc2which also have catchProgress.Lang.Stop.I don't understand. Are you saying that even though the
Progress.Lang.Stopexception is raised insidedoBlock2AND there is a catch block indoBlock2, theProgress.Lang.Stopwill be caught indoBlock1? 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
doBlock0which hasSTOP-AFTER 100and alsocatch 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,UserInterruptandLockConflictincom.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 usingProgress.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_handlingand adopt then theProgress.Lang.Stop. Trying to run them inOEI get an error for the following statementssupport.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 forundoReturnErrorTopLevel(AppError err). There are onlyundoReturnErrorTopLevel()andundoReturnErrorTopLevel(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_handlingand adopt then theProgress.Lang.Stop. Trying to run them inOEI get an error for the following statementssupport.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_handlingand 'support' isn't a database, it's just calling a static method fromsupport.test.AssertExtclass.
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
doBlock0block that raised the the stop-after interrupt does not have aCATCHstatement forProgress.Lang.Stop. And in the nested blocks down the path to the instruction that was interrupted byProgress.Lang.Stop(repeatblock), there are some CATCH statements. The behaviour for this case is very strange from my POV in 4GL, it will infinity retry the execution ofdoBlock0.
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
doBlock0block that raised the the stop-after interrupt does not have aCATCHstatement forProgress.Lang.Stop. And in the nested blocks down the path to the instruction that was interrupted byProgress.Lang.Stop(repeatblock), there are some CATCH statements. The behaviour for this case is very strange from my POV in 4GL, it will infinity retry the execution ofdoBlock0.
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.ifnonExistentProcedureHopefullyprocedure is non-existent should raise aStopConditionException?
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.Errorshould 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 inifblock 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
StopConditionExceptionis, 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
StopConditionExceptionis, 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
StopErrorshould 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
StopErrorshould 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
LockConflictis raised only when this timeout finishes? Can you tryEXCLUSIVE-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
LockConflictis raised only when this timeout finishes? Can you tryEXCLUSIVE-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
control_flow.rules- please create a function for the
.equals("Stop") etccondition, as now the check is repeated
- please create a function for the
Control.java- remove unneeded
p2j.uiimports
- remove unneeded
Queue.java- this must not be aware of the exception's type. Why do you need to call
ErrorManager.handleStopExceptionthere?
- this must not be aware of the exception's type. Why do you need to call
LockConflict.java,Stop.java,StopAfter.java,UserInterrupt.java- needs history entry
ConnectionManager.getTimeOut- this call is expensive; save this in the
PersistenceContextinstance, and calculate once. - needs javadoc
- this call is expensive; save this in the
TypeAhead.java- line 513 was changed from
if (honorServerEvents && EventManager.hasServerEvents())toif (EventManager.hasServerEvents())- this doesn't seem right
- line 513 was changed from
BlockDefinition.java- can we not save the
stopAfterExceptionhere? This type is already kind of 'bloated';
- can we not save the
BlockManager.javaimport com.goldencode.p2j.util.BlockManager.Condition;is not needed- extra whitespace on line 554/555/9422
mustManageLegacyStop- save thisSessionUtils._startupParameters().getCatchStop();in theBlockManager$WorkArea, no need to resolve it every time.processBody- can we callwa.tm.honorStopCondition();only if-catchStopis enabled? same inprocessForBodyWorkArea.errHelperneeds javadocWorkArea.stopAfterExceptionis never used
ErrorManager.javarecordForLegacyThrow- add a comment why293is treated explicitly thereServerErrorDataAccessor.mustThrowLegacyStopneeds javadoc
StopConditionException.java- needs history entry
TransactionManager.javaiterateWorker-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.handleStopExceptionthere?
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 callwa.tm.honorStopCondition();only if-catchStopis enabled? same inprocessForBody
-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
control_flow.rulesexecuteLibshould beexecLib
Persistence.java- needs history entry
InMemoryLockManager.java- update copyright year
DeferredLegacyStopException- missing class javadoc
StopConditionException*- missing javadoc for
logit
- missing javadoc for
TransactionManager- missing javadoc for
stopAfterException
- missing javadoc for
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
- dependent on the 9470a changes, so 9470a will go first after testing is finished
StopError.java- change theLegacyResourceSupportto FULLStopConditionException.java- missing history entry (please double check all files)
#74 Updated by Constantin Asofiei about 1 year ago
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
ClassDefinitionfor these two qualified names, and after that checks recursively the parents, if i.e.progress.lang.stopis 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
Stopsub-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
Stopsub-classes? I think the conversion rules need to check explicitly if super-class is Stop or not. Please change this function inAccording 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()inErrorManager.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, thereply.getPayload()can be both processed byErrorManager.handleStopException()and then thrown. IfhandleStopException()decided to ignore it, shouldn't we return something like unknown object?ConnectionManager.java, new methodgetTimeOut():- 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;
- a typo in parameter name:
StopConditionException: there is a new static flag,logit. It can be used intryLog()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 theelsebranch of the next conditional, avoiding the object to be created if not needed; - line 11180: the condition is useless.
lexcannot benullsince it is the same asstopAfterExceptionwhich was already checked fornull. In fact the locallexcan be inlined.
- line 11154: the
- the method
tryLog(String message, Throwable cause)is implemented multiple time (with same content - code duplication) in the new classes. The similar method fromStopConditionExceptioncan be called if changed toprotected(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 thebzr 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, thereply.getPayload()can be both processed byErrorManager.handleStopException()and then thrown. IfhandleStopException()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 methodgetTimeOut():
- 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 thebzr 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 fromoptstooptionsand then performing a map lookup? Whileoptsis iterated we can stop and return the first entry withopt.getKey() == ConnectOption.LOCK_TIMEOUT, if any;StopConditionException.java: can nowLOGandlogitbe restricted toprivate? All access should be performed using thetryLog()method;- I did not notice previously: the new exceptions
LockConflictConditionException,StopAfterConditionException, etc have aprivate 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-stopconfiguration.
Edited Bootstrap_Configuration wiki with catch-stop configuration.
#98 Updated by Greg Shah about 1 year ago
- Status changed from Test to Closed