try-with-resources: follow the Java rules on propagating an exception up

the callstack 1. exception from body block 2. exception from catch block 3. exception from finally block 4. exception from resource auto-close
jlangch · Oct 4, 2023 · b18f454 · b18f454
1 parent 0cf86b8
commit b18f454
Show file tree

Hide file tree

Showing 4 changed files with 125 additions and 25 deletions.
diff --git a/ChangeLog.md b/ChangeLog.md
@@ -11,6 +11,14 @@ All notable changes to this project will be documented in this file.
 
 - functions to handle locks based on a semphore
 
+### Fixed 
+
+- Venice to follow the Java rules when propagating exceptions from try-with-resources
+    1. exception from body block
+    2. exception from catch block
+    3. exception from finally block
+    4. exception from resource auto-close
+
 ### Updated 
 
 - :pdf module dependencies to flyingSaucer 9.3.1 and openpdf 1.3.30

diff --git a/...main/java/com/github/jlangch/venice/impl/specialforms/SpecialForms_TryCatchFunctions.java b/...main/java/com/github/jlangch/venice/impl/specialforms/SpecialForms_TryCatchFunctions.java
@@ -41,6 +41,7 @@
 import com.github.jlangch.venice.impl.specialforms.util.FinallyBlock;
 import com.github.jlangch.venice.impl.specialforms.util.SpecialFormsContext;
 import com.github.jlangch.venice.impl.thread.ThreadContext;
+import com.github.jlangch.venice.impl.types.Constants;
 import com.github.jlangch.venice.impl.types.VncBoolean;
 import com.github.jlangch.venice.impl.types.VncFunction;
 import com.github.jlangch.venice.impl.types.VncJavaObject;
@@ -242,16 +243,30 @@ public VncVal apply(
                     }
                 }
 
-                try {
-                    return handleTryCatchFinally(
+
+                // Follow the Java rules on propagating an exception up the callstack
+                //    1. exception from body block
+                //    2. exception from catch block
+                //    3. exception from finally block
+                //    4. exception from resource auto-close
+                VncVal retVal = Constants.Nil;
+                RuntimeException primaryEx = null;
+                RuntimeException autoCloseEx = null;
+
+             	try {
+             		retVal = handleTryCatchFinally(
                                 "try-with",
                                 args.rest(),
                                 ctx,
                                 localEnv,
                                 specialFormMeta,
                                 boundResources);
-                }
-                finally {
+            	}
+            	catch(RuntimeException ex) {
+            		primaryEx = ex;
+            	}
+
+                try {
                     final List<VncException> exceptions = new ArrayList<>();
 
                     // close resources in reverse order, do best effort and close all resources
@@ -279,10 +294,24 @@ public VncVal apply(
                         }
                     });
 
-                    // throw the first exception
+                    // throw the first auto-close exception, we came across
                     if (!exceptions.isEmpty()) {
                         throw exceptions.get(0);
                     }
+            	}
+            	catch(RuntimeException ex) {
+            		autoCloseEx = ex;
+            	}
+
+
+                if (primaryEx != null) {
+                	throw primaryEx;
+                }
+                else if (autoCloseEx != null) {
+                	throw autoCloseEx;
+                }
+                else {
+                	return retVal;
                 }
             }
 

diff --git a/src/test/java/com/github/jlangch/venice/impl/specialforms/SpecialFormsTest_TryWith.java b/src/test/java/com/github/jlangch/venice/impl/specialforms/SpecialFormsTest_TryWith.java
@@ -22,11 +22,13 @@
 package com.github.jlangch.venice.impl.specialforms;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.fail;
 
 import org.junit.jupiter.api.Test;
 
 import com.github.jlangch.venice.Parameters;
 import com.github.jlangch.venice.Venice;
+import com.github.jlangch.venice.VncException;
 import com.github.jlangch.venice.util.CapturingPrintStream;
 
 
@@ -105,4 +107,82 @@ public void test_try_with_3() {
         assertEquals(-1L, venice.eval(lisp, Parameters.of("*out*", ps)));
         assertEquals("100101200201300301", ps.getOutput());
     }
+
+    @Test
+    public void test_try_with_ex_autocloseable_1() {
+        final Venice venice = new Venice();
+
+        // if the 'try-with' body throws an exception it has precedence of an exception
+        // thrown on auto-close. the latter one will be suppressed.
+        final String script =
+                "(do                                                                   \n" +
+                "   (import :com.github.jlangch.venice.support.AutoCloseableString)    \n" +
+                "   (try-with [r1 (. :AutoCloseableString :new \"hello-1\" false)      \n" +
+                "              r2 (. :AutoCloseableString :new \"hello-2\" false)]     \n" +
+                "     (throw (ex :VncException \"EX-BODY\"))))                         ";
+
+        try {
+        	venice.eval(script);
+        	fail("Expected a VncException");
+        }
+        catch(VncException ex) {
+        	final String msg = ex.getMessage();
+        	assertEquals("EX-BODY", msg);
+        }
+        catch(Exception ex) {
+        	fail("Expected a VncException");
+        }
+    }
+
+    @Test
+    public void test_try_with_ex_autocloseable_2() {
+        final Venice venice = new Venice();
+
+        // if the 'try-with' body throws an exception it has precedence of an exception
+        // thrown on auto-close. the latter one will be suppressed.
+        final String script =
+                "(do                                                                   \n" +
+                "   (import :com.github.jlangch.venice.support.AutoCloseableString)    \n" +
+                "   (try-with [r1 (. :AutoCloseableString :new \"hello-1\" true)       \n" +
+                "              r2 (. :AutoCloseableString :new \"hello-2\" true)]      \n" +
+                "     (throw (ex :VncException \"EX-BODY\"))))                         ";
+
+        try {
+        	venice.eval(script);
+        	fail("Expected a VncException");
+        }
+        catch(VncException ex) {
+        	final String msg = ex.getMessage();
+        	assertEquals("EX-BODY", msg);
+        }
+        catch(Exception ex) {
+        	fail("Expected a VncException");
+        }
+    }
+
+    @Test
+    public void test_try_with_ex_autocloseable_3() {
+        final Venice venice = new Venice();
+
+        // if the 'try-with' body throws an exception it has precedence of an exception
+        // thrown on auto-close. the latter one will be suppressed.
+        final String script =
+                "(do                                                                   \n" +
+                "   (import :com.github.jlangch.venice.support.AutoCloseableString)    \n" +
+                "   (try-with [r1 (. :AutoCloseableString :new \"hello-1\" true)       \n" +
+                "              r2 (. :AutoCloseableString :new \"hello-2\" true)]      \n" +
+                "     nil))                                                            ";
+
+        try {
+        	venice.eval(script);
+        	fail("Expected a VncException");
+        }
+        catch(VncException ex) {
+        	final String msg = ex.getMessage();
+        	assertEquals("'try-with' failed to close resource r2.", msg);
+        }
+        catch(Exception ex) {
+        	fail("Expected a VncException");
+        }
+    }
 }
diff --git a/src/test/java/com/github/jlangch/venice/support/AutoCloseableString.java b/src/test/java/com/github/jlangch/venice/support/AutoCloseableString.java
@@ -23,36 +23,19 @@
 
 import com.github.jlangch.venice.VncException;
 import com.github.jlangch.venice.impl.types.VncString;
-import com.github.jlangch.venice.impl.types.VncVal;
-import com.github.jlangch.venice.impl.types.custom.VncWrappingTypeDef;
 
 
 public class AutoCloseableString extends VncString implements AutoCloseable {
 
-    public AutoCloseableString(final String v) {
+    public AutoCloseableString(final String v, final boolean throwExOnClose) {
         super(v);
-    }
-
-    public AutoCloseableString(final String v, final VncVal meta) {
-        super(v, meta);
-    }
-
-    public AutoCloseableString(
-            final String v,
-            final VncWrappingTypeDef wrappingTypeDef,
-            final VncVal meta
-    ) {
-        super(v, wrappingTypeDef, meta);
-    }
-
-    public void throwExOnClose() {
-        this.throwExOnClose = true;
+        this.throwExOnClose = throwExOnClose;
     }
 
     @Override
     public void close() {
         if (throwExOnClose) {
-            throw new VncException("AutoCloseableString");
+            throw new VncException(super.getValue());
         }
     }