fix: throw MBeanException when setLayout cannot instantiate

Returning a diagnostic string from a void-declared JMX operation is
easy for clients to treat as success. Throw MBeanException with an
IllegalArgumentException target instead so failure is unambiguous,
while still logging the error.

Update the regression test to assert MBeanException.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

Author: SebTardif

log4j-1.2-api/src/main/java/org/apache/log4j/jmx/AppenderDynamicMBean.java

@@ -188,9 +188,12 @@ public Object invoke(final String operationName, final Object params[], final St
             final Layout layout =
                     (Layout) OptionConverter.instantiateByClassName((String) params[0], Layout.class, null);
             if (layout == null) {
-                cat.error("Could not instantiate layout class [" + params[0] + "] for appender ["
-                        + getAppenderName(appender) + "].");
-                return "Could not instantiate layout class.";
+                final String message = "Could not instantiate layout class [" + params[0] + "] for appender ["
+                        + getAppenderName(appender) + "].";
+                cat.error(message);
+                // Fail via MBeanException so JMX clients can distinguish failure from
+                // success (setLayout is declared void; a return string is not reliable).
+                throw new MBeanException(new IllegalArgumentException(message), message);
             }
             appender.setLayout(layout);
             registerLayoutMBean(layout);

log4j-1.2-api/src/test/java/org/apache/log4j/jmx/AppenderDynamicMBeanTest.java

@@ -16,11 +16,13 @@
  */
 package org.apache.log4j.jmx;
 
-import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
+import javax.management.MBeanException;
 import org.apache.log4j.ConsoleAppender;
 import org.apache.log4j.PatternLayout;
 import org.junit.jupiter.api.Test;
@@ -36,11 +38,13 @@ void setLayoutDoesNotNpeWhenClassCannotBeInstantiated() throws Exception {
         appender.setName("jmx-layout-test");
         final AppenderDynamicMBean mbean = new AppenderDynamicMBean(appender);
 
-        final Object result = assertDoesNotThrow(
+        final MBeanException thrown = assertThrows(
+                MBeanException.class,
                 () -> mbean.invoke("setLayout", new Object[] {"this.class.does.not.exist.MissingLayout"}, new String[] {
                     String.class.getName()
                 }));
-        assertTrue(result == null || result.toString().contains("Could not instantiate"));
+        assertTrue(thrown.getMessage().contains("Could not instantiate layout class"));
+        assertInstanceOf(IllegalArgumentException.class, thrown.getTargetException());
         assertNull(appender.getLayout());
     }