improve exception logging and catching during closures

Author: brachy84

src/main/java/com/cleanroommc/groovyscript/api/GroovyLog.java

@@ -231,6 +231,16 @@ default void errorMC(Object o) {
      */
     void exception(Throwable throwable);
 
+    /**
+     * Formats and logs an exception to this log AND Minecraft's log with a message.<br>
+     * The log will be printed without formatting to Minecraft's log.
+     * Unnecessary lines that clutter the log will get removed before logging to this log.<br>
+     * <b>The exception will NOT be thrown!</b>
+     *
+     * @param throwable exception to log
+     */
+    void exception(String msg, Throwable throwable);
+
     /**
      * Formats a {@link String} and arguments according to the defined rules.
      *

src/main/java/com/cleanroommc/groovyscript/sandbox/ClosureHelper.java

@@ -100,7 +100,7 @@ public static Closure<?> getUnderlyingClosure(Object functionalInterface) {
                 return (Closure<?>) convertedClosure.getDelegate();
             }
         } catch (IllegalArgumentException | IllegalAccessException e) {
-            GroovyLog.get().exception(e);
+            GroovyLog.get().exception("A reflection error occurred while trying to obtain a closure from a lambda.", e);
         }
         return null;
     }

src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyLogImpl.java

@@ -250,18 +250,24 @@ public void errorMC(String msg, Object... args) {
         logger.error(msg, args);
     }
 
+    @Override
+    public void exception(Throwable throwable) {
+        exception("An exception occurred while running scripts.", throwable);
+    }
+
     /**
      * Logs an exception to the groovy log AND Minecraft's log. It does NOT throw the exception! The stacktrace for the groovy log will be
      * stripped for better readability.
      *
      * @param throwable exception
      */
     @Override
-    public void exception(Throwable throwable) {
-        String msg = throwable.toString();
-        this.errors.add(msg);
-        writeLogLine(formatLine("ERROR", "An exception occurred while running scripts. Look at latest.log for a full stacktrace:"));
-        writeLogLine("\t" + msg);
+    public void exception(String msg, Throwable throwable) {
+        String throwableMsg = throwable.toString();
+        this.errors.add(throwableMsg);
+        msg += " Look at latest.log for a full stacktrace:";
+        writeLogLine(formatLine("ERROR", msg));
+        writeLogLine("\t" + throwableMsg);
         Pattern pattern = Pattern.compile("(\\w*).run\\(\\1(\\.\\w*):(\\d*)\\)");
         for (String line : prepareStackTrace(throwable.getStackTrace())) {
             Matcher matcher = pattern.matcher(line);
@@ -271,6 +277,7 @@ public void exception(Throwable throwable) {
                 writeLogLine("\t\tat " + line);
             }
         }
+        GroovyScript.LOGGER.error(msg);
         GroovyScript.LOGGER.throwing(throwable);
     }
 

src/main/java/com/cleanroommc/groovyscript/sandbox/GroovySandbox.java

@@ -259,8 +259,7 @@ protected Class<?> loadScriptClass(GroovyScriptEngine engine, File file) {
 
             // if the file is still not found something went wrong
         } catch (Exception e) {
-            GroovyLog.get().fatalMC("An error occurred while trying to load script class {}", file.toString());
-            GroovyLog.get().exception(e);
+            GroovyLog.get().exception("An error occurred while trying to load script class " + file.toString(), e);
         }
         return scriptClass;
     }

src/main/java/com/cleanroommc/groovyscript/sandbox/GroovyScriptSandbox.java

@@ -161,8 +161,7 @@ public void run(LoadStage currentLoadStage) {
         try {
             super.load();
         } catch (IOException | ScriptException | ResourceException e) {
-            GroovyLog.get().errorMC("An exception occurred while trying to run groovy code! This is might be a internal groovy issue.");
-            GroovyLog.get().exception(e);
+            GroovyLog.get().exception("An exception occurred while trying to run groovy code! This is might be a internal groovy issue.", e);
         } catch (Throwable t) {
             GroovyLog.get().exception(t);
         } finally {
@@ -194,26 +193,30 @@ public <T> T runClosure(Closure<T> closure, Object... args) {
         try {
             result = runClosureInternal(closure, args);
         } catch (Throwable t) {
-            this.storedExceptions.computeIfAbsent(Arrays.asList(t.getStackTrace()), k -> {
-                GroovyLog.get().error("An exception occurred while running a closure!");
-                GroovyLog.get().exception(t);
-                return new AtomicInteger();
-            }).addAndGet(1);
+            List<StackTraceElement> stackTrace = Arrays.asList(t.getStackTrace());
+            AtomicInteger counter = this.storedExceptions.get(stackTrace);
+            if (counter == null) {
+                GroovyLog.get().exception("An exception occurred while running a closure at least once!", t);
+                this.storedExceptions.put(stackTrace, new AtomicInteger(1));
+                UncheckedThrow.rethrow(t);
+                return null; // unreachable statement
+            } else {
+                counter.getAndIncrement();
+            }
         } finally {
             if (!wasRunning) stopRunning();
         }
         return result;
     }
 
     @GroovyBlacklist
-    private static <T> T runClosureInternal(Closure<T> closure, Object[] args) {
+    private static <T> T runClosureInternal(Closure<T> closure, Object[] args) throws Throwable {
         // original Closure.call(Object... arguments) code
         try {
             //noinspection unchecked
             return (T) closure.getMetaClass().invokeMethod(closure, "doCall", args);
         } catch (InvokerInvocationException e) {
-            UncheckedThrow.rethrow(e.getCause());
-            return null; // unreachable statement
+            throw e.getCause();
         } catch (Exception e) {
             if (e instanceof RuntimeException) {
                 throw e;