Propagate DataflowError in middle of block instead of ignoring it

CHANGELOG.md

@@ -1,3 +1,11 @@
+# Next Next Release
+
+#### Enso Language & Runtime
+
+- [Propagate Error ASAP instead of ignoring it][11777].
+
+[11777]: https://github.com/enso-org/enso/pull/11777
+
 # Next Release
 
 #### Enso IDE

distribution/lib/Standard/Test/0.0.0-dev/src/Extensions.enso

@@ -4,6 +4,7 @@ import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
 
 import project.Spec_Result.Spec_Result
 import project.Test.Test
+from project.Helpers import should_equal_error_check
 
 ## Expect a function to fail with the provided dataflow error.
 
@@ -70,23 +71,24 @@ Error.should_fail_with self matcher frames_to_skip=0 unwrap_errors=True =
 
          example_should_equal = Examples.add_1_to 1 . should_equal 2
 Any.should_equal : Any -> Integer -> Spec_Result
-Any.should_equal self that frames_to_skip=0 = case self == that of
-    True -> Spec_Result.Success
-    False ->
-        loc = Meta.get_source_location 2+frames_to_skip
-        additional_comment = case self of
-            _ : Vector -> case that of
-                _ : Vector ->
-                    case self.length == that.length of
-                        True ->
-                            diff = self.zip that . index_of p->
-                                p.first != p.second
-                            "; first difference at index " + diff.to_text + " "
-                        False -> "; lengths differ (" + self.length.to_text + " != " + that.length.to_text + ") "
+Any.should_equal self that frames_to_skip=0 = should_equal_error_check that <|
+    case self == that of
+        True -> Spec_Result.Success
+        False ->
+            loc = Meta.get_source_location 2+frames_to_skip
+            additional_comment = case self of
+                _ : Vector -> case that of
+                    _ : Vector ->
+                        case self.length == that.length of
+                            True ->
+                                diff = self.zip that . index_of p->
+                                    p.first != p.second
+                                "; first difference at index " + diff.to_text + " "
+                            False -> "; lengths differ (" + self.length.to_text + " != " + that.length.to_text + ") "
+                    _ -> ""
                 _ -> ""
-            _ -> ""
-        msg = self.pretty + " did not equal " + that.pretty + additional_comment + " (at " + loc + ")."
-        Test.fail msg
+            msg = self.pretty + " did not equal " + that.pretty + additional_comment + " (at " + loc + ")."
+            Test.fail msg
 
 ## Asserts that `self` value is equal to the expected type value.
 
@@ -266,8 +268,7 @@ Error.should_end_with self that frames_to_skip=0 =
 
          example_should_equal = Examples.add_1_to 1 . should_equal 2
 Error.should_equal : Any -> Integer -> Spec_Result
-Error.should_equal self that frames_to_skip=0 =
-    _ = [that]
+Error.should_equal self that frames_to_skip=0 = should_equal_error_check that <|
     Test.fail_match_on_unexpected_error self 1+frames_to_skip
 
 ## Asserts that `self` is within `epsilon` from `that`.
@@ -293,7 +294,7 @@ Error.should_equal self that frames_to_skip=0 =
          example_should_equal =
             1.00000001 . should_equal 1.00000002 epsilon=0.0001
 Number.should_equal : Float -> Float -> Integer -> Spec_Result
-Number.should_equal self that epsilon=0 frames_to_skip=0 =
+Number.should_equal self that epsilon=0 frames_to_skip=0 = should_equal_error_check that <|
     matches = case that of
         _ : Number -> self.equals that epsilon
         _ -> self==that
@@ -312,7 +313,7 @@ Number.should_equal self that epsilon=0 frames_to_skip=0 =
    - frames_to_skip (optional, advanced): used to alter the location which is
      displayed as the source of this error.
 Decimal.should_equal : Number -> Float-> Float -> Integer -> Spec_Result
-Decimal.should_equal self that epsilon=0 frames_to_skip=0 =
+Decimal.should_equal self that epsilon=0 frames_to_skip=0 = should_equal_error_check that <|
     self.to_float . should_equal that.to_float epsilon frames_to_skip+1
 
 ## Asserts that `self` value is not an error.

distribution/lib/Standard/Test/0.0.0-dev/src/Helpers.enso

@@ -3,6 +3,7 @@ private
 from Standard.Base import all
 from Standard.Base.Runtime import assert
 from Standard.Base.Runtime import State
+import Standard.Base.Errors.Illegal_Argument.Illegal_Argument
 
 import project.Clue.Clue
 import project.Group.Group
@@ -82,3 +83,12 @@ type Finished_With
        - stack_trace_text: A textual representation of the stack trace for the
          error.
     Error err stack_trace_text
+
+
+## PRIVATE
+should_equal_error_check that ~action =
+    case that.is_error of
+        False -> action
+        True ->
+            msg = "Dataflow error provided as expected value for `should_equal`. Use `should_fail_with` instead."+ ' Error stack trace was:\n'+that.get_stack_trace_text
+            Panic.throw (Illegal_Argument.Error msg)

engine/runtime-integration-tests/src/test/java/org/enso/interpreter/test/semantic/DataflowErrorPropagationTest.java

@@ -0,0 +1,86 @@
+package org.enso.interpreter.test.semantic;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.enso.common.MethodNames;
+import org.enso.test.utils.ContextUtils;
+import org.graalvm.polyglot.Context;
+import org.graalvm.polyglot.PolyglotException;
+import org.graalvm.polyglot.Value;
+import org.junit.AfterClass;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+public class DataflowErrorPropagationTest {
+  private static Context ctx;
+  private static Value suppressError;
+  private static Value suppressErrorWithAssign;
+
+  @BeforeClass
+  public static void prepareCtx() {
+    ctx = ContextUtils.createDefaultContext();
+    var code =
+        """
+    from Standard.Base import all
+
+    private yield_error yes:Boolean -> Text =
+        if yes then Error.throw "Yielding an error" else
+            "OK"
+
+    suppress_error yes:Boolean value =
+        yield_error yes
+        value
+
+    suppress_error_with_assign yes:Boolean value =
+        _ = yield_error yes
+        value
+    """;
+    suppressError =
+        ctx.eval("enso", code).invokeMember(MethodNames.Module.EVAL_EXPRESSION, "suppress_error");
+    suppressErrorWithAssign =
+        ctx.eval("enso", code)
+            .invokeMember(MethodNames.Module.EVAL_EXPRESSION, "suppress_error_with_assign");
+  }
+
+  @AfterClass
+  public static void disposeCtx() {
+    ctx.close();
+    ctx = null;
+  }
+
+  @Test
+  public void noErrorReturnValue() {
+    var value = suppressError.execute(false, 42);
+    assertTrue("It is a number", value.isNumber());
+    assertEquals(42, value.asInt());
+  }
+
+  @Test
+  public void propagateErrorImmediatelly() {
+    var value = suppressError.execute(true, 42);
+    assertFalse("It is not a number", value.isNumber());
+    assertTrue("It is an error", value.isException());
+    try {
+      throw value.throwException();
+    } catch (PolyglotException ex) {
+      assertEquals("Yielding an error", ex.getMessage());
+    }
+  }
+
+  @Test
+  public void noErrorReturnValueWithAssign() {
+    var value = suppressErrorWithAssign.execute(false, 42);
+    assertTrue("It is a number", value.isNumber());
+    assertEquals(42, value.asInt());
+  }
+
+  @Test
+  public void errorIsAssignedAndThatIsEnoughReturnValue() {
+    var value = suppressErrorWithAssign.execute(true, 42);
+    assertTrue("It is a number", value.isNumber());
+    assertFalse("Not an error", value.isException());
+    assertEquals(42, value.asInt());
+  }
+}

engine/runtime/src/main/java/org/enso/interpreter/node/callable/function/BlockNode.java

@@ -9,6 +9,8 @@
 import com.oracle.truffle.api.source.SourceSection;
 import java.util.Set;
 import org.enso.interpreter.node.ExpressionNode;
+import org.enso.interpreter.runtime.EnsoContext;
+import org.enso.interpreter.runtime.error.DataflowError;
 
 /**
  * This node defines the body of a function for execution, as well as the protocol for executing the
@@ -55,8 +57,15 @@ public static BlockNode buildSilent(ExpressionNode[] expressions, ExpressionNode
   @Override
   @ExplodeLoop
   public Object executeGeneric(VirtualFrame frame) {
+    var ctx = EnsoContext.get(this);
+    var nothing = ctx.getBuiltins().nothing();
     for (ExpressionNode statement : statements) {
-      statement.executeGeneric(frame);
+      var result = statement.executeGeneric(frame);
+      if (result != nothing) {
+        if (result instanceof DataflowError err) {
+          return err;
+        }
+      }
     }
     return returnExpr.executeGeneric(frame);
   }

test/Table_Tests/src/Util.enso

@@ -5,29 +5,30 @@ import Standard.Database.DB_Column.DB_Column
 from Standard.Table import Column, Table
 
 from Standard.Test import all
+from Standard.Test.Helpers import should_equal_error_check
 
 polyglot java import org.enso.base_test_helpers.FileSystemHelper
 
 Table.should_equal : Any -> Integer -> Any
-Table.should_equal self expected frames_to_skip=0 =
+Table.should_equal self expected frames_to_skip=0 = should_equal_error_check that <|
     loc = Meta.get_source_location 1+frames_to_skip
     Panic.catch Test_Failure_Error (table_should_equal_impl self expected loc) error->
         Test.fail error.payload.message 
 
 Column.should_equal : Any -> Integer -> Any
-Column.should_equal self expected frames_to_skip=0 =
+Column.should_equal self expected frames_to_skip=0 = should_equal_error_check that <|
     loc = Meta.get_source_location 1+frames_to_skip
     Panic.catch Test_Failure_Error (column_should_equal_impl self expected loc) error->
         Test.fail error.payload.message
 
 DB_Table.should_equal : DB_Table -> Integer -> Any
-DB_Table.should_equal self expected frames_to_skip=0 =
+DB_Table.should_equal self expected frames_to_skip=0 = should_equal_error_check that <|
     t0 = self.read
     t1 = expected.read
     t0 . should_equal t1 frames_to_skip+1
 
 DB_Column.should_equal : DB_Column -> Integer -> Any
-DB_Column.should_equal self expected frames_to_skip=0 =
+DB_Column.should_equal self expected frames_to_skip=0 = should_equal_error_check that <|
     t0 = self.read
     t1 = expected.read
     t0 . should_equal t1 frames_to_skip+1