FWD » Core Development » Base Language

Created task branch 3406a. The changes for this task are going to be in a sensitive area and will affect many files.

I will post more details of the problem and the proposed solution shortly. But for now, I have a question.

The persistence layer has the following usage of EM.isPendingError():

AbstractQuery

   protected static void validateSubstitutionArguments(Object[] args)
   {
      if (args != null)
      {
         int len = args.length;
         for (int i = 0; i < len; i++)
         {
            Object next = args[i];

            // If there is an error pending, an argument may be null.  Allow
            // processing to continue.
            if (next == null && ErrorManager.isPendingError())
            {
               continue;
            }

            // Argument must be one of these types.
            if (!((next instanceof BaseDataType) ||
                  (next instanceof Resolvable)   ||
                  (next instanceof P2JQuery.Parameter)))
            {
               throw new IllegalArgumentException(
                  "Invalid where expression substitution argument:  " + next + " [" + i + "]");
            }
         }
      }
   }

...

   protected boolean prepareFetch()
   {
      if (ErrorManager.isPendingError())
      {
         try
         {
            releaseBuffers();
         }
         catch (QueryOffEndException exc)
         {
            // intentionally ignored
         }

         if (LOG.isLoggable(Level.FINE))
         {
            String msg = "Error pending in query";
            LOG.log(Level.FINE, msg, new ErrorConditionException(msg));
         }

         return false;
      }

      return true;
   }

RandomAccessQuery

   private void resolveArgs()
   {
      // Resolve regular query substitution arguments.
      if (args != null)
      {
         int len = args.length;
         currentArgs = new Object[len];
         for (int i = 0; i < len; i++)
         {
            Object next = args[i];
            if (next == null && ErrorManager.isPendingError())
            {
               currentArgs = null;

               return;
            }
            if (next instanceof FieldReference)
            {
               currentArgs[i] = next;
            }
            else if (next instanceof Resolvable)
            {
               currentArgs[i] = ((Resolvable) next).resolve();
            }
            else
            {
               currentArgs[i] = ((BaseDataType) next).duplicate();
            }
         }
      }
   }

RecordBuffer.invoke()

            if (!isGetter)
            {
               getter = getPojoGetterMap().get(property);

               // if this is a read-only mode buffer do not allow this to happen:
               if (readonly)
               {
                  int[] errCodes = {2665, 142};
                  String[] errMessages =
                  {
                     "Attempt to update a Read-Only buffer for table " + table,
                     "Unable to update " + property + " Field" 
                  };

                  // TODO: this seems like an always fatal error, cannot be put on hold with
                  // no-error
                  ErrorManager.recordOrThrowError(errCodes, errMessages, true);
               }

               // if a template record is loaded we are not allowed to alter it that easy
               if (currentRecord.getId() < 0)
               {
                  final String msg = "Cannot update a template record without a schema lock";
                  ErrorManager.recordOrThrowError(12376, msg, false, false);
                  return null;
               }

               // We don't want to upgrade the lock, validate, or assign if an error is pending.
               if (ErrorManager.isPendingError())
               {
                  return null;
               }

Eric: Of these 4 usages, if EM.recordOrThrowError() now raises an ErrorConditionException such that control flow is aborted even in silent mode, then can these calls to isPendingError() be removed?

I need to investigate more thoroughly, but my initial thoughts:

RecordBuffer$Handler.invoke: should be safe to remove the check.

RandomAccessQuery.resolveArgs, AbstractQuery.validateSubstitutionArguments: I think it should be safe to remove the check. My only reservation is that I can't recall where the assumption comes from that an argument may be null if an error is pending. If it is not possible for the argument to be null otherwise, removing the entire if (next == null && ErrorManager.isPendingError()) block should be safe in both cases.

AbstractQuery.prepareFetch: This one may be problematic. It is called by every navigation method in RandomAccessQuery and releases the buffers if an error is pending. I'm not sure how we would replicate this functionality otherwise.

ETF testing failed immediately because the DMO ASM code used the old BDT assign signatures. Once I fixed this (committed to 3406a rev 11216), ETF testing proceeded. However, there are regressions. Looking into these...

There were no instances of the temporary logging from RandomAccessQuery and AbstractQuery in the server log; however, I only have run the search ETF tests so far.

The errors I've looked at so far seem to be cases of P2JQuery.Parameter lambda expressions being passed too far along unresolved as query substitution parameters. The errors occur when downstream query processing code is expecting data types of the resolved parameters, not the lambdas themselves.

The root cause of the first failing test is that the new code in AbstractQuery.preprocessSubstitutionArguments doesn't behave the same way as the code it replaced.

The old code would attempt to resolve a P2JQuery.Parameter lambda in silent error mode, and store the resolved argument in the args array, replacing the lambda originally at that index position in the array with the resolved value. If there was a failure during this resolution which resulted in a call to ErrorManager.recordOrThrowError, generally this would cause the lambda expression to resolve to unknown value (because an ErrorConditionException would not be thrown, due to silent mode, and the expression processing would complete). So, we would end up with a BDT subclass instance set to unknown value in the query substitution parameter array where the P2JQuery.Parameter object had been.

The new code aborts the query substitution parameter immediately upon an error, so the expression never resolves to unknown value. Instead, the P2JQuery.Parameter lambda object remains in the args array. This blows up downstream processing, which is prepared to deal with BDT types, not lambda expressions.

I haven't looked at the other 37 failures, but I suspect they all stem from this same regression.

Investigation so far shows that the 4GL's where clause processing and error handling is a bit different than ours. An error caused by a field reference to an empty buffer will be handled silently and will abort processing at least some of the where clause expression, but it is not clear exactly how much.

I haven't been able to determine hard and fast rules about what is aborted; it may depend on the order of the internal evaluation of the where clause as to exactly what is processed and what is aborted. Processing of a where clause with a compound set of sub-expressions ANDed together seems to abort at a sub-expression which is the child of an AND node if an error is raised evaluating a component of that sub-expression, but this does not abort processing of its sibling sub-expression. I explored this using a user-defined function with side-effects, by embedding references to the field of an empty buffer in the same sub-expression as the UDF. One example:

define temp-table tt1
   field f1 as int.

define temp-table tt2
   field f1 as int.

define var i as int init 1.

function func1 int:
   message "inside func1; i =" string(i) "; tt2 available:" (avail tt2).
   i = i + 1.
   return 1.
end.

function func2 int:
   message "inside func2; i =" string(i) "; tt2 available:" (avail tt2).
   i = i + 1.
   return 1.
end.

function func3 int:
   message "inside func3; i =" string(i) "; tt2 available:" (avail tt2).
   i = i + 1.
   return 1.
end.

function func4 int:
   message "inside func4; i =" string(i) "; tt2 available:" (avail tt2).
   i = i + 1.
   return 1.
end.

create tt1.
tt1.f1 = 1.
create tt1.
tt1.f1 = 2.

create tt2.
release tt2.

message "FIND...".

find first tt1
     where func1() = 1
       and tt1.f1 = tt2.f1
       and func2() + tt2.f1 + func3() = ?
       and tt1.f1 = func4()
       no-error
       .

if avail tt1 then display tt1.

/*
Message output:

FIND...
inside func1; i = 1; tt2 available: no.
inside func2; i = 2; tt2 available: no.
inside func4; i = 3; tt2 available: no.
*/

The fact that func4 is invoked suggests the expression processing aborts at the sub-expression level, but processing of other WHERE clause sub-expressions is allowed to continue. This simple example suggests the sub-expressions are demarcated by the logical AND operators, but a more complicated WHERE clause might indicate further details of this demarcation.

The FIND statement does not return a record.

The 4GL does not necessarily refactor out substitution parameters per se, the way we do, and it probably evaluates these inline within the WHERE clause expression. As such, we are losing some information as to the precedence of these operations, since we are just seeing an array of parameters to be resolved at runtime, and we no longer have the context of the expression they are in for purposes of operation precedence. In general, this should be ok, except when it comes to the side effects of evaluating certain WHERE clause expression components, notably user-defined functions. Since we have lost the context of the overall expression, we will be evaluating these too aggressively some cases where an error has occurred. This is a limitation of the current implementation.

Greg, 3406a/11222 has passed ETF search and dev tests (2 clean runs of the latter). I am testing batch next, but please go ahead and run standard regression testing against this revision when you can.

ETF batch tests passed. If it passes regular regression testing, I think this revision can be merged to trunk.

Unfortunately, the ChUI app regression testing fails with the same core regressions that have been happening earlier in the week. 117 tests failed in the last run.

3406a/11223 passed ETF developer testing, but failed regular regression testing twice with 4 of the same tests failing identically both times. So, it seems there is still a real regression there.

The issue for at least two of the tests (a concurrent pair) is that the harness is expecting a validation failure (unique constraint violation) message to be printed on screen, but it is not, so the test pair times out. The flow is:

1. create a new record
2. modify a field which does not fully update any index of the table (validation is thus deferred)
3. execute a FIND ... NO-ERROR on the buffer in which the new record was created and modified

The FIND performs a flush of the current buffer contents, triggering validation of the record currently in the buffer. It fails the uniqueness test and throws ValidationException (a child class of NumberedException). The exception is caught and passed to EM.recordOrThrowError(NumberedException). However, since we're operating within the context of the FIND ... NO-ERROR statement (i.e., the EM.silent worker), the message is not written to the screen because of the changes in rev. 11223.

The other cases for which we want to suppress the screen message do not represent errors resulting from deferred processing, so the 11223 fix works for those. This is an edge case, however, which should not be handled silently. The state machine for record validation is quite complex. So far, I don't know how to detect the fact that the validation we are performing has been deferred from a non-NO-ERROR context, knowledge we somehow would need to pass on to the error manager in order not to suppress the message.

Revision 3406a/11224 almost resolves the deferred validation issue, except instead of the pause and "Press space bar to continue." message expected by the test case, I get no pause and "Enter data or press F4 to end.". The focus is on a fill-in and it is in edit mode, when it should be blocked on the pause. Not sure what is needed, since the display code in EM hasn't changed from when this was working.

Per our offline discussion, revision 11225 enables the re-throw of the error condition inside the silent worker's catch block when the exception is raised by EM.throwError(NumberedException, suppress) when suppress is false.

I changed the other direct users of throwError() to be consistent.

I've also added TODO and ECF_TODO comments in other locations that need review.

Eric: Please restart testing and also look at the ECF-TODO locations.

Constantin: Please do a code review. In particular, look at the changes (and TODOs) in GuiWebSocket, ControlFlowOps etc...

Stanislav: Please look at Browse and BrowseWidget for the TODOs I've added.

I will write up the overall approach next so that everyone can understand it.

Stanislav: Please look at Browse and BrowseWidget for the TODOs I've added.

These exceptions serve the purpose of passing 4GL error code to server side, or caught immediately in BrowseWidget. After they caught, they are re-raised using ErrorManager.recordOrShowError.
If that it not ideologically correct, I can replace it with any exception which holds a number.
Also I found one place where it is not caught properly.

Our original approach to silent error mode was to bracket each NO-ERROR statement (or assignment or standalone expression) with calls to enable/disable silent error mode in the ErrorManager.

Language Statements

/* find stmt */
find first bogus no-error.

/* run stmt */
run something (input bogus.num) no-error.

This converted to:

         ErrorManager.silentErrorEnable();
         /* find stmt */
         new FindQuery(bogus, (String) null, null, "bogus.id asc").first();
         ErrorManager.silentErrorDisable();

         ErrorManager.silentErrorEnable();
         /* run stmt */
         ControlFlowOps.invokeWithMode("something", "I", bogus.getNum());
         ErrorManager.silentErrorDisable();

Standalone Expression (whatevs() is a user defined function)

/* standalone expression */
whatevs(bogus.num) no-error.

This converted to:

         ErrorManager.silentErrorEnable();
         whatevs(bogus.getNum());
         ErrorManager.silentErrorDisable();

Assignment

/* assignment */
i = whatevs(bogus.num) no-error.

This converted to:

         ErrorManager.silentErrorEnable();
         /* assignment */
         i.assign(whatevs(bogus.getNum()));
         ErrorManager.silentErrorDisable();

Inside the ErrorManager we stored a silent mode flag that was toggled by silentErrorEnable() and silentErrorDisable().

In any location where we would normally raise the error condition (e.g. throw new ErrorConditionException(...) in Java), we would instead use ErrorManager.recordOrThrowError() which in silent mode would store the state of the error in ERROR-STATUS and then return without actually throwing the error. This is the first half of the solution, since in the 4GL NO-ERROR (usually) avoids control flow changes when an error is encountered during a statement/assignment/expression executed with NO-ERROR. By suppressing the throw of the exception, FWD avoided the flow of control change.

Any code that calls ErrorManager.recordOrThrowError() would also need to return gracefully in case of silent mode. A common example would be to return unknown value from a getter that encounters an error in silent mode.

But this introduced a a problem in FWD that did not exist in the 4GL. In the 4GL, when an error occurs in NO-ERROR mode, the execution of the statement/assignment/expression aborts processing immediately but the error condition is not raised in the containing business logic. In FWD, we did NOT abort processing immediately. In the assignment and standalone expression examples above, if there is no record in the buffer, the whatevs() function DOES NOT execute in the 4GL but in FWD it DOES. In the 4GL, if an error occurs during the evaluation of the argument to this function call, the function call is bypassed. In FWD it is not bypassed.

To minimize the impact of this, we implemented safety code all over FWD.

By handling things gracefully

More importantly, we then modified the processing all over FWD to bypass processing when in silent mode AND an error had occurred during the current statement's execution. This was most commonly detected by calling isPendingError():

   public void assign(NumberType value, boolean force)
   {
      if (force || !ErrorManager.isPendingError())
      {
         // actual assignment logic here
      }
   }

There were some cases where we still had to do the processing even if isPendingError() was true, so often this logic had to have a force flag. Although these efforts were successful in getting large applications running properly, we had never resolved the underlying issue: properly and immediately aborting processing.

We could make FWD code safe for this isPendingError() processing. The problem here is that there is converted code that can be called in FWD when it would not be called in the 4GL. The whatevs() case above is a perfect example. If that code has side-effects of any invocation, then the FWD results can be different from the 4GL. An example of a side effect would be to increment a counter every time the function is called. In FWD, it may be called more than the 4GL so the counter would be different. Data corruption and other problems are also possible with such side-effect processing.

This task was meant to fix this deviation. Since we had intrusively modified code throughout FWD to bypass when isPendingError() is true, the changes for branch 3406a are also quite intrusive. The solution in our case is to implement any converted NO-ERROR statement/assignment/expression as a lambda and pass that code in to an ErrorManager worker to process. That worker can then catch any raised error condition and return without letting it propagate into the surrounding business logic. This allows all the locations that use recordOrThrowError() to actually throw the exception, thus aborting processing immediately, which will match the 4GL behavior exactly.

Where the old ErrorManager code had this:

   public static void silentErrorEnable()
   {
      // enable silent error processing
      da.setSilent(true);
   }

...

   public static void silentErrorDisable()
   {
      // disable further silent error processing
      da.setSilent(false);

      // copy the pending error data into the ERROR-STATUS handle and clear the pending data
      da.forwardPending();
   }

The new ErrorManager.silent() (worker routine) is this:

   public static boolean silent(Runnable code)
   {
      ServerDataAccess srv = (ServerDataAccess) da;
      WorkArea         wa  = work.obtain();

      // enable silent error processing
      srv.setSilent(wa, true);

      try
      {
         code.run();
      }

      catch (ErrorConditionException err)
      {
         // normally we just eat the exception because it is being used to abort processing
         // exactly where the error occurred, which will restart processing after the given
         // code block; however, there is an exception for some deferred error processing use
         // cases where we must bypass silent error mode (throwError(NumberedException, boolean))
         if (err.isForce())
         {
            // don't honor silent mode, no suppression of errors here
            throw err;
         }
      }

      finally
      {
         // disable further silent error processing
         srv.setSilent(wa, false);

         srv.forwardPending();
      }

      return wa.errorStatus.error;
   }

This is intentionally a server-side worker. It also deliberately avoids multiple context-local lookups to improve performance.

The converted code can now look like this:

         /* find stmt */
         silent(() -> new FindQuery(bogus, (String) null, null, "bogus.id asc").first());

...

         /* run stmt */
         silent(() -> ControlFlowOps.invokeWithMode("something", "I", bogus.getNum()));

         silent(() -> whatevs(bogus.getNum()));

         /* assignment */
         silent(() -> i.assign(whatevs(bogus.getNum())));

In the case of any multi-line output, the Java code will use a block:

silent(() ->
{
   ...
});

By using a static import of ErrorManager and minimizing the use of the multi-line block, the result is a real improvement in the converted source code for silent error mode.

I also removed nearly all usage of isPendingError() because it is now almost always unnecessary AND it involved two context-local lookups (which are relatively expensive when done in loops/millions of times). This simplifies the FWD code base and it also provides a performance improvement (we are hoping for a few percentage points).

As can be seen from the dialog above, this first approach did show regressions in both the large ChUI app and in ETF. These regressions were mostly in the persistence layer, which was abusing silent error mode to duplicate some behavior in the 4GL for WHERE clause processing which ignored errors but did not record state in ERROR-STATUS. There were also some subtle persistence layer dependencies on the side effects of continuing to execute the statement. For example, in a Find, a WHERE clause failure would now cause abort of the processing before the first(), last() ... method can be called. But the old code implemented the proper buffer release and state cleanup at that point which had to be handled differently in the new approach.

Another issue was found with code that needs to raise an error regardless of silent error mode. The most obvious cases are in the persistence layer, but basically there are places where the 4GL does not honor NO-ERROR. We had to find and fix those locations.

The version as it exists right now is close to passing ChUI regression testing and ETF. There may still be a ChUI regression, testing is ongoing right now.

Please let me know if there are questions.

Stanislav Lomany wrote:

Stanislav: Please look at Browse and BrowseWidget for the TODOs I've added.

These exceptions serve the purpose of passing 4GL error code to server side, or caught immediately in BrowseWidget. After they caught, they are re-raised using ErrorManager.recordOrShowError.
If that it not ideologically correct, I can replace it with any exception which holds a number.
Also I found one place where it is not caught properly.

ErrorManager.recordOrShowError or ErrorManager.recordOrThrowError?

Generally, it is OK to do this to transfer the flow of control to the server for processing so long as you are handling it properly there. In these cases, please change the TODO comment to mention something like "this is being raised to transfer control to the server where the proper ErrorManager processing will occur".

On the server side, if these do not raise real errors in the 4GL business logic, then an approach with ErrorManager.recordOrShowError is OK. If the error must be raised as visible to the business logic (changing flow of control), then ErrorManager.recordOrThrowError should be used.

Greg Shah wrote:

I've also added TODO and ECF_TODO comments in other locations that need review.

Eric: Please restart testing and also look at the ECF-TODO locations.

I've checked in my changes as revisioin 11226, and I am ready to go through the next round of regression testing. However, I will have to re-run it once everyone else has committed their changes. Please let me know how close you are, in case it makes sense to wait.

ErrorManager.recordOrShowError or ErrorManager.recordOrThrowError?

ErrorManager.recordOrShowError

1. SharedVariableManager.errorHelper - this is used when there is a problem resolving a shared resource. We explicitly throw here because we assume this will be caught by ControlFlowOps$ExternalProgramResolver.resolve - this catches all errors and dispatches them via ErrorManager.recordOrThrowError. So no matter how, this needs to be throw.
2. ControlFlowOps.invokeImpl:4424 - I'm not sure about this one. The InvalidParameterConditionException can be raised I think only when there is some temp-table parameter error - see TemporaryBuffer.handleTablesDoNotMatch. The location for the throw suggests that the ERROR condition is raised regardless of NO-ERROR mode. But this was not my code so I can't explain further...
3. Agent.invokeFailure - this one throws a ControlFlowOps.invokeFailure incoming error. This will allow current command to abort (which is a run for an external program) and will just 'eat' the exception and copy it to the RemoteInvocationResult instance. I would leave this as is, as we are outside of 4GL normal behavior; anyway, the idea is also 'this needs to be thrown'.
4. ControlFlowOps$MapArgumentResolver.resolve - this has to do with the MSG_P2J_INVOKE incoming from the JS side, in embedded mode. This is under our control (new FWD feature), so we can change it even to recordOrThrowError, as long as the remote JS side is properly informed of the error. This was originally adapted from ControlFlowOps.validArguments.
5. GuiWebSocket.processBinaryMessage - for MSG_P2J_INVOKE, the ControlFlowOps.invoke(InvocationRequestPayload) will eat any conditions; the reason is because we want to send the response to the JS side BEFORE re-raising the conditions to allow the business logic to intercept/treat them. So, for the ERROR (without NO-ERROR mode) case, we need to throw it - no matter how.

Otherwise, I'm OK with the changes. But please check the embedded Hotel GUI, too.

So, are we expecting any more code changes at this point, or is revision 11226 what we're testing? Actually, I already started the harness, but I can restart it if necessary.

I wanted to fix TODOs and error in processing in one place, but can do that in another branch.

Stanislav Lomany wrote:

I wanted to fix TODOs and error in processing in one place, but can do that in another branch.

It seems it would be best to do them in this branch, since Greg flagged them as potentially risky areas for this set of changes.

How involved are the changes? I'm trying to get a sense of timing on this testing.

No, its not risky, just minor fix which IIRC doesn't affect regression testing.

OK, please put it in and I'll restart testing.

OK, please put it in and I'll restart testing.

That may require some time because is not yet ready.

OK, then please hold off on updating this branch, unless you believe these changes are necessary to making the new error-handling idiom described in #3406-18 work (please let me know if this is the case, but it doesn't seem so from your comments above).

I'll continue with regression testing. If all passes, I will merge to trunk and we can follow up with these changes in another branch.

It's not the case.

3406a/11226 has passed dev, search, and batch ETF testing. Two runs of main runtime regression testing show a handful of overlapping failed tests. Once we have the fixups to hand-written code for the regression environment I will try to re-run the failing tests manually to be sure they work correctly. Ctrl-C testing is running now.

Task branch 3406a has been rebased to p2j trunk revision 11212.

I have manually tested Hotel GUI (Swing, Virtual Desktop and Embedded Modes) in both 3406a (11232) and trunk (11212). There is no difference in behavior, no regressions. This includes the PUBLISH/SUBSCRIBE support in embedded mode (where there was the highest chances of problems).

I will say that the 3406a version seems much more responsive than trunk. Perhaps that is wishful thinking, but hopefully my initial perception is accurate and there really is a noticeable performance improvement. :)

Is this a known Hotel GUI issue? I hit this while trying to check in a guest on the "Available" screen first thing after logging in. I tested in embedded mode, 3406a/11232, hotel_gui rev. 156, freshly converted, fresh database import (PostgreSQL). I did not test with trunk, so I don't know if it's specific to 3406a. Seems to be a client-side issue. The effect on the UI is that it creates an overlay with no window reachable, so you're effectively dead in the water.

Caused by: java.lang.IllegalStateException: Can not change window to 10 within the same thread, if a window is already selected!
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.selectWindow(AbstractGuiDriver.java:2052)
        at com.goldencode.p2j.ui.client.gui.driver.GuiDriver.withSelectedWindow(GuiDriver.java:774)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.computeLineWidths(EditorGuiImpl.java:3317)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.maxLineWidthNative(EditorGuiImpl.java:3081)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.access$2500(EditorGuiImpl.java:131)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl$EditorScrollContainer.getScrollDimension(EditorGuiImpl.java:3858)
        at com.goldencode.p2j.ui.client.gui.ScrollPaneGuiImpl.adjustScrollLayoutImpl(ScrollPaneGuiImpl.java:728)
        at com.goldencode.p2j.ui.chui.ThinClient.eventBracket(ThinClient.java:15448)
        at com.goldencode.p2j.ui.client.gui.ScrollPaneGuiImpl.adjustScrollLayout(ScrollPaneGuiImpl.java:226)
        at com.goldencode.p2j.ui.client.gui.ScrollPaneGuiImpl.draw(ScrollPaneGuiImpl.java:244)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.lambda$draw$2(EditorGuiImpl.java:888)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.EditorGuiImpl.draw(EditorGuiImpl.java:881)
        at com.goldencode.p2j.ui.client.Editor.draw(Editor.java:839)
        at com.goldencode.p2j.ui.client.widget.AbstractContainer.draw(AbstractContainer.java:513)
        at com.goldencode.p2j.ui.client.gui.FrameGuiImpl$GuiScrollContainer.lambda$draw$0(FrameGuiImpl.java:1896)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.FrameGuiImpl$GuiScrollContainer.draw(FrameGuiImpl.java:1896)
        at com.goldencode.p2j.ui.client.widget.Viewport.draw(Viewport.java:112)
        at com.goldencode.p2j.ui.client.widget.AbstractContainer.draw(AbstractContainer.java:513)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.access$001(BorderedPanelGuiImpl.java:94)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.lambda$null$0(BorderedPanelGuiImpl.java:309)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.lambda$draw$1(BorderedPanelGuiImpl.java:295)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.draw(BorderedPanelGuiImpl.java:245)
        at com.goldencode.p2j.ui.client.gui.ScrollPaneGuiImpl$1.run(ScrollPaneGuiImpl.java:261)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.ScrollPaneGuiImpl.draw(ScrollPaneGuiImpl.java:252)
        at com.goldencode.p2j.ui.client.gui.FrameGuiImpl.lambda$draw$1(FrameGuiImpl.java:777)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.FrameGuiImpl.draw(FrameGuiImpl.java:765)
        at com.goldencode.p2j.ui.client.Frame.draw(Frame.java:2355)
        at com.goldencode.p2j.ui.client.widget.AbstractContainer.draw(AbstractContainer.java:513)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.access$001(BorderedPanelGuiImpl.java:94)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.lambda$null$0(BorderedPanelGuiImpl.java:309)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.lambda$draw$1(BorderedPanelGuiImpl.java:295)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.draw(BorderedPanelGuiImpl.java:245)
        at com.goldencode.p2j.ui.client.widget.AbstractContainer.draw(AbstractContainer.java:513)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.access$001(BorderedPanelGuiImpl.java:94)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.lambda$null$0(BorderedPanelGuiImpl.java:309)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.lambda$draw$1(BorderedPanelGuiImpl.java:295)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2411)
        at com.goldencode.p2j.ui.client.gui.driver.AbstractGuiDriver.draw(AbstractGuiDriver.java:2454)
        at com.goldencode.p2j.ui.client.gui.BorderedPanelGuiImpl.draw(BorderedPanelGuiImpl.java:245)
        at com.goldencode.p2j.ui.client.widget.AbstractContainer.draw(AbstractContainer.java:513)
        at com.goldencode.p2j.ui.client.widget.OuterFrame.drawInt(OuterFrame.java:146)
        at com.goldencode.p2j.ui.client.widget.OuterFrame.draw(OuterFrame.java:138)
        at com.goldencode.p2j.ui.client.gui.ModalWindow.draw(ModalWindow.java:342)
        at com.goldencode.p2j.ui.client.OutputManager.setInvalidate(OutputManager.java:1339)
        at com.goldencode.p2j.ui.client.gui.driver.GuiOutputManager.setInvalidate(GuiOutputManager.java:204)
        at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:15398)
        at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:15295)
        at com.goldencode.p2j.ui.chui.ThinClient.viewWorker(ThinClient.java:10837)
        at com.goldencode.p2j.ui.chui.ThinClient.view_(ThinClient.java:9906)
        at com.goldencode.p2j.ui.chui.ThinClient.view(ThinClient.java:9670)
        at sun.reflect.GeneratedMethodAccessor6.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:124)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:757)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:412)
        at com.goldencode.p2j.net.Conversation.waitMessage(Conversation.java:348)
        at com.goldencode.p2j.net.Queue.transactImpl(Queue.java:1170)
        at com.goldencode.p2j.net.Queue.transact(Queue.java:641)
        at com.goldencode.p2j.net.BaseSession.transact(BaseSession.java:271)
        at com.goldencode.p2j.net.HighLevelObject.transact(HighLevelObject.java:211)
        at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1473)
        at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:145)
        at com.sun.proxy.$Proxy10.trigger(Unknown Source)
        at com.goldencode.p2j.ui.chui.ThinClient.trigger(ThinClient.java:12871)
        at com.goldencode.p2j.ui.chui.ThinClient.invokeTriggers(ThinClient.java:18355)
        at com.goldencode.p2j.ui.chui.ThinClient.invokeTriggers(ThinClient.java:18121)
        at com.goldencode.p2j.ui.chui.ThinClient.processProgressEvent(ThinClient.java:17412)
        at com.goldencode.p2j.ui.chui.ThinClient.processEventsWorker(ThinClient.java:16476)
        at com.goldencode.p2j.ui.chui.ThinClient.pop(ThinClient.java:15485)
        at com.goldencode.p2j.ui.chui.ThinClient.eventBracket(ThinClient.java:15468)
        at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:15392)
        at com.goldencode.p2j.ui.chui.ThinClient.applyWorker(ThinClient.java:15153)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:12218)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:11702)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:11645)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:11599)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:124)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:757)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:412)
        at com.goldencode.p2j.net.Conversation.waitMessage(Conversation.java:348)
        at com.goldencode.p2j.net.Queue.transactImpl(Queue.java:1170)
        at com.goldencode.p2j.net.Queue.transact(Queue.java:641)
        at com.goldencode.p2j.net.BaseSession.transact(BaseSession.java:271)
        at com.goldencode.p2j.net.HighLevelObject.transact(HighLevelObject.java:211)
        at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1473)
        at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:145)
        at com.sun.proxy.$Proxy10.trigger(Unknown Source)
        at com.goldencode.p2j.ui.chui.ThinClient.trigger(ThinClient.java:12871)
        at com.goldencode.p2j.ui.chui.ThinClient.invokeTriggers(ThinClient.java:18355)
        at com.goldencode.p2j.ui.chui.ThinClient.invokeTriggers(ThinClient.java:18121)
        at com.goldencode.p2j.ui.chui.ThinClient.processProgressEvent(ThinClient.java:17412)
        at com.goldencode.p2j.ui.chui.ThinClient.processEventsWorker(ThinClient.java:16476)
        at com.goldencode.p2j.ui.chui.ThinClient.pop(ThinClient.java:15485)
        at com.goldencode.p2j.ui.chui.ThinClient.eventBracket(ThinClient.java:15468)
        at com.goldencode.p2j.ui.chui.ThinClient.eventDrawingBracket(ThinClient.java:15392)
        at com.goldencode.p2j.ui.chui.ThinClient.applyWorker(ThinClient.java:15153)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:12218)
        at com.goldencode.p2j.ui.chui.ThinClient.waitForWorker(ThinClient.java:11702)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:11645)
        at com.goldencode.p2j.ui.chui.ThinClient.waitFor(ThinClient.java:11599)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at com.goldencode.p2j.util.MethodInvoker.invoke(MethodInvoker.java:124)
        at com.goldencode.p2j.net.Dispatcher.processInbound(Dispatcher.java:757)
        at com.goldencode.p2j.net.Conversation.block(Conversation.java:412)
        at com.goldencode.p2j.net.Conversation.waitMessage(Conversation.java:348)
        at com.goldencode.p2j.net.Queue.transactImpl(Queue.java:1170)
        at com.goldencode.p2j.net.Queue.transact(Queue.java:641)
        at com.goldencode.p2j.net.BaseSession.transact(BaseSession.java:271)
        at com.goldencode.p2j.net.HighLevelObject.transact(HighLevelObject.java:211)
        at com.goldencode.p2j.net.RemoteObject$RemoteAccess.invokeCore(RemoteObject.java:1473)
        at com.goldencode.p2j.net.InvocationStub.invoke(InvocationStub.java:145)
        at com.sun.proxy.$Proxy12.standardEntry(Unknown Source)
        at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:361)
        at com.goldencode.p2j.main.ClientCore.start(ClientCore.java:158)
        at com.goldencode.p2j.main.ClientDriver.start(ClientDriver.java:250)
        at com.goldencode.p2j.main.CommonDriver.process(CommonDriver.java:444)
        at com.goldencode.p2j.main.ClientDriver.process(ClientDriver.java:144)
        at com.goldencode.p2j.main.ClientDriver.main(ClientDriver.java:313)

Follow-up: I tested with trunk; same error.

However, it's fixed in 3394a (tested with 11299), so... never mind.

Greg, in ControlFlowOps - is the H061 still needed?

### Eclipse Workspace Patch 1.0
#P p2j
Index: src/com/goldencode/p2j/util/ControlFlowOps.java
===================================================================
--- src/com/goldencode/p2j/util/ControlFlowOps.java    (revision 1387)
+++ src/com/goldencode/p2j/util/ControlFlowOps.java    (working copy)
@@ -130,6 +130,9 @@
 ** 059 CA  20170328          Fixed a problem with parameter validation when invoked by a remote 
 **                           non-P2J side (i.e. API invoked by an embedded client).
 ** 060 GES 20171206          Switch to ErrorManager.silent(). Added TODOs.
+** 061 CA  20171202          If at the time of a function/procedure invocation there is a pending
+**                           error (and NO-ERROR mode is on), then do not continue with the 
+**                           invocation.
 */

 /*
@@ -4136,6 +4139,11 @@
                                           ArgumentResolver argResolver)
    {
       BaseDataType result = new unknown();
+      
+      if (ErrorManager.isPendingError())
+      {
+         return result;
+      }

       String name = cname.toStringMessage();

@@ -4578,6 +4586,11 @@
                                             String           modes,
                                             Object...        args) 
    {
+      if (ErrorManager.isPendingError())
+      {
+         return;
+      }
+      
       // this will always be an external procedure call
       if (name.isUnknown() || name.toStringMessage().length() == 0)
       {
@@ -7138,4 +7151,4 @@
                                               args);
       }
    }   
-}
\ No newline at end of file
+}

in ControlFlowOps - is the H061 still needed?

No, not after trunk rev 11213.

Greg Shah wrote:

in ControlFlowOps - is the H061 still needed?

No, not after trunk rev 11213.

Backed out in 3394a rev 11306.

Created task branch 3406b from trunk rev 11213.

Greg, I've left changes for browse TODOs in 3406b rev 11214.

Branch 3406a was archived.

Task branch 3406b has been rebased from trunk rev 11214. The latest revision is 11215.