LOG4J2-2659: AbstractAction handles unchecked RuntimeException and Error (#296)

Author: carterkozak

log4j-core/src/main/java/org/apache/logging/log4j/core/appender/rolling/action/AbstractAction.java

@@ -64,8 +64,12 @@ public synchronized void run() {
         if (!interrupted) {
             try {
                 execute();
-            } catch (final IOException ex) {
+            } catch (final RuntimeException | IOException ex) {
                 reportException(ex);
+            } catch (final Error e) {
+                // reportException takes Exception, widening to Throwable would break custom implementations
+                // so we wrap Errors in RuntimeException for handling.
+                reportException(new RuntimeException(e));
             }
 
             complete = true;

log4j-core/src/test/java/org/apache/logging/log4j/core/appender/rolling/action/AbstractActionTest.java

@@ -35,11 +35,46 @@ public void testExceptionsAreLoggedToStatusLogger() {
                 "$TestAction.execute(AbstractActionTest.java:"));
     }
 
+    @Test
+    public void testRuntimeExceptionsAreLoggedToStatusLogger() {
+        StatusLogger statusLogger = StatusLogger.getLogger();
+        statusLogger.clear();
+        new AbstractAction() {
+            @Override
+            public boolean execute() {
+                throw new IllegalStateException();
+            }
+        }.run();
+        List<StatusData> statusDataList = statusLogger.getStatusData();
+        assertEquals(1, statusDataList.size());
+        StatusData statusData = statusDataList.get(0);
+        assertEquals(Level.WARN, statusData.getLevel());
+        String formattedMessage = statusData.getFormattedStatus();
+        assertTrue(formattedMessage.contains("Exception reported by action"));
+    }
+
+    @Test
+    public void testErrorsAreLoggedToStatusLogger() {
+        StatusLogger statusLogger = StatusLogger.getLogger();
+        statusLogger.clear();
+        new AbstractAction() {
+            @Override
+            public boolean execute() {
+                throw new AssertionError();
+            }
+        }.run();
+        List<StatusData> statusDataList = statusLogger.getStatusData();
+        assertEquals(1, statusDataList.size());
+        StatusData statusData = statusDataList.get(0);
+        assertEquals(Level.WARN, statusData.getLevel());
+        String formattedMessage = statusData.getFormattedStatus();
+        assertTrue(formattedMessage.contains("Exception reported by action"));
+    }
+
     private static final class TestAction extends AbstractAction {
         @Override
-        public boolean execute() {
-            this.reportException(new IOException("failed"));
-            return false;
+        public boolean execute() throws IOException {
+            throw new IOException("failed");
         }
     }
-}
+}

src/changes/changes.xml

@@ -158,6 +158,9 @@
         AbstractAction.reportException records a warning to the status logger, providing more information when file
         based appenders fail to compress rolled data asynchronously.
       </action>
+      <action issue="LOG4J2-2659" dev="ckozak" type="fix">
+        AbstractAction handles and records unchecked RuntimeException and Error in addition to IOException.
+      </action>
     </release>
     <release version="2.12.0" date="2019-06-23" description="GA Release 2.12.0">
       <action issue="LOG4J2-2547" dev="rgoers" type="fix">