Improved JS error detection during DFS traversal

Author: michbarsinai

README.md

@@ -54,11 +54,15 @@ If you use BPjs in an academic work, please consider citing it as:
 
 
 ### 2022-02 (Including the 1st BP Day Hackathon)
+* :part_alternation_mark: Calling `bp.sync` from global scope during runtime does not cause an ugly exception anymore ([#174](https://github.com/bThink-BGU/BPjs/issues/174)). The error is reported to the listeners.
+    * Also when making the illegal call from an interrupt handler.
+* :arrow_up: Improved JavaScript code error detection during analysis - 
 * :arrow_up: Improvements to handling `null`s in event set arrays ([#178](https://github.com/bThink-BGU/BPjs/issues/178)).
 * :arrow_up: Improvements to handling Rhino context in BPjs class ([#176](https://github.com/bThink-BGU/BPjs/issues/176)).
 * :arrow_up: Improvements to the `MapProxy` classes, allowing client code pre-process changes.
 * :part_alternation_mark: `BEvent::getDataField` data accessors wrap the lower-level `getData()` ([#161](https://github.com/bThink-BGU/BPjs/issues/161)).
 * :arrow_up: `JsEventSet` now honors the event set name during equality checks.
+* :put_litter_in_its_place: Consolidated tests for `BProgramJsProxy`.
 
 ### 2022-01
 * :arrow_up: Improvements to the `BEvent` class, to make it better support client Java code.

src/main/java/il/ac/bgu/cs/bp/bpjs/analysis/DfsBProgramVerifier.java

@@ -170,7 +170,10 @@ public VerificationResult verify(BProgram aBp) throws Exception {
         ExecutorService execSvc = BPjs.getExecutorServiceMaker().makeWithName("DfsBProgramRunner-" + INSTANCE_COUNTER.incrementAndGet());
         long start = System.currentTimeMillis();
         listener.started(this);
-        Violation vio = dfsUsingStack(new DfsTraversalNode(currentBProgram, currentBProgram.setup().start(execSvc, currentBProgram.getStorageModificationStrategy()), null), execSvc);
+        Violation vio = dfsUsingStack(new DfsTraversalNode(currentBProgram, 
+            currentBProgram.setup().start(execSvc, currentBProgram.getStorageModificationStrategy()), null), 
+            execSvc
+        );
         long end = System.currentTimeMillis();
         execSvc.shutdown();
         listener.done(this);

src/main/java/il/ac/bgu/cs/bp/bpjs/analysis/DfsTraversalNode.java

@@ -6,6 +6,7 @@
 import java.util.*;
 
 import il.ac.bgu.cs.bp.bpjs.execution.listeners.BProgramRunnerListener;
+import il.ac.bgu.cs.bp.bpjs.execution.listeners.BProgramRunnerListenerAdapter;
 import il.ac.bgu.cs.bp.bpjs.model.BProgram;
 import il.ac.bgu.cs.bp.bpjs.model.BProgramSyncSnapshot;
 import il.ac.bgu.cs.bp.bpjs.model.BEvent;
@@ -89,8 +90,26 @@ public String toString() {
      * 
      */
     public DfsTraversalNode getNextNode(BEvent e, ExecutorService exSvc) throws BPjsRuntimeException {
+        final Exception[] caughtException = new Exception[1];
+        final BProgramRunnerListener l = new BProgramRunnerListenerAdapter(){
+            @Override
+            public void error(BProgram bp, Exception ex) {
+                caughtException[0] = ex;
+            }
+            
+        };
         try {
-            return new DfsTraversalNode(bp, BProgramSyncSnapshotCloner.clone(systemState).triggerEvent(e, exSvc, Collections.emptyList(), bp.getStorageModificationStrategy()), e);
+            DfsTraversalNode next = new DfsTraversalNode(bp, 
+                BProgramSyncSnapshotCloner.clone(systemState).triggerEvent(e, exSvc, Collections.singletonList(l),bp.getStorageModificationStrategy()), e);
+            if ( caughtException[0] != null ) {
+                if ( caughtException[0] instanceof BPjsRuntimeException ){
+                    throw (BPjsRuntimeException)(caughtException[0]);
+                } else {
+                  throw new BPjsRuntimeException("Exception while reaching for a new node", caughtException[0]);  
+                }
+            }
+            return next;
+            
         } catch ( InterruptedException ie ) {
             throw new BPjsRuntimeException("Thread interrupted during event invocaiton", ie);
         }

src/main/java/il/ac/bgu/cs/bp/bpjs/execution/BProgramRunner.java

@@ -68,7 +68,11 @@ public BProgramRunner(BProgram aBProgram) {
             bprog.setAddBThreadCallback( (bp,bt)->listeners.forEach(l->l.bthreadAdded(bp, bt)));
         }
     }
-    
+    /**
+     * Kicks off the execution of the {@link BProgram}. This method does not 
+     * {@code return} or {@code throw} anything, as it is part or the {@link Runnable}
+     * interface. Errors and results are reported to the listeners.
+     */
     @Override
     public void run() {
         try {

src/main/java/il/ac/bgu/cs/bp/bpjs/execution/jsproxy/BProgramJsProxy.java

@@ -234,6 +234,11 @@ public SyncStatementBuilder hot(boolean isHot) {
      */
     @Override
     void synchronizationPoint(NativeObject jsRWB, Boolean hot, Object data) {
+        
+        if ( CURRENT_BTHREAD.get() == null ) {
+            throw new BPjsRuntimeException("Calling bp.sync outside of a b-thread is forbidden");
+        }
+        
         Map<String, Object> jRWB = (Map) Context.jsToJava(jsRWB, Map.class);
 
         SyncStatement stmt = SyncStatement.make();
@@ -252,7 +257,9 @@ void synchronizationPoint(NativeObject jsRWB, Boolean hot, Object data) {
                     stmt = stmt.request((BEvent) req);
                 }
             } catch (ClassCastException cce) {
-                throw new BPjsRuntimeException("A non-event object requested in a sync statement. Offending object:'" + ScriptableUtils.stringify(req) + "'");
+                throw new BPjsRuntimeException("A non-event object requested in a sync statement. "
+                    + "Offending object:'" + ScriptableUtils.stringify(req) + "' in BThread " + 
+                    CURRENT_BTHREAD.get().snapshot.getName());
             }
         }
 

src/main/java/il/ac/bgu/cs/bp/bpjs/model/BProgram.java

@@ -226,12 +226,7 @@ protected Object evaluate(String script, String scriptName, Context curCtx) {
             } catch (BPjsException be ) {
                 throw be;
             } catch ( IllegalStateException ise ) {
-                String msg = ise.getMessage();
-                if ( msg.contains("Cannot capture continuation") && msg.contains("executeScriptWithContinuations or callFunctionWithContinuations") ){
-                    throw new BPjsCodeEvaluationException("bp.sync called outside of a b-thread");
-                } else {
-                    throw ise;
-                }
+                throw ise;
             } catch ( Throwable generalException ) {
                 throw new BPjsRuntimeException("(Wrapped) Exception evaluating BProgram code: " + generalException.getMessage(), generalException);
             }

src/main/java/il/ac/bgu/cs/bp/bpjs/model/BProgramSyncSnapshot.java

@@ -248,8 +248,13 @@ private void handleInterrupts(BEvent anEvent, Iterable<BProgramRunnerListener> l
                             final Scriptable scope = bt.getScope();
                             try {
                                 ctxt.callFunctionWithContinuations(func, scope, new Object[]{anEvent});
-                            } catch ( ContinuationPending ise ) {
-                                throw new BPjsRuntimeException("Cannot call bp.sync or fork from a break-upon handler. Please consider pushing an external event.");
+                            } catch ( BPjsRuntimeException e) {
+                                listeners.forEach( l -> l.error(bprog, e));
+                            } catch ( WrappedException wpe ) {
+                                if ( wpe.getWrappedException() instanceof BPjsRuntimeException ) {
+                                    BPjsRuntimeException e = (BPjsRuntimeException) wpe.getWrappedException();
+                                    listeners.forEach( l -> l.error(bprog, e));
+                                }
                             }
                         });
             });

src/test/java/il/ac/bgu/cs/bp/bpjs/execution/BpProxyFunctionalityTest.java

@@ -2,7 +2,9 @@
 
 import il.ac.bgu.cs.bp.bpjs.model.BProgram;
 import il.ac.bgu.cs.bp.bpjs.exceptions.BPjsCodeEvaluationException;
+import il.ac.bgu.cs.bp.bpjs.execution.listeners.BProgramRunnerListenerAdapter;
 import il.ac.bgu.cs.bp.bpjs.model.StringBProgram;
+import java.util.concurrent.atomic.AtomicBoolean;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
@@ -22,14 +24,17 @@ public void testInvalidBSyncCall() throws InterruptedException {
             "var i=0;\n var j=42;\n var k=5; bp.sync({request:bp.Event(\"A\")});"
         );
 
-        try { 
-            new BProgramRunner(sut).run();
-            fail("System should have thrown an error due to bp.sync called outside of a BThread.");
-        } catch (BPjsCodeEvaluationException exp) {
-            assertTrue( exp.getMessage().contains("bp.sync"));
-            System.out.println("Error message details: ");
-            System.out.println(exp.getDetails());
-        }
+        final AtomicBoolean errorCalled = new AtomicBoolean(false);
+        final BProgramRunner bProgramRunner = new BProgramRunner(sut);
+        bProgramRunner.addListener( new BProgramRunnerListenerAdapter(){
+            @Override
+            public void error(BProgram bp, Exception ex) {
+                errorCalled.set(true);
+            }
+            
+        });
+        bProgramRunner.run();
+        assertTrue("The runner listener should have had its 'error' method called ", errorCalled.get() );
     }
 
     @Test