diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/pattern/EnsureExceptionHandling.java b/logback-classic/src/main/java/ch/qos/logback/classic/pattern/EnsureExceptionHandling.java index e3f23022a4..521ced9b36 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/pattern/EnsureExceptionHandling.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/pattern/EnsureExceptionHandling.java @@ -16,61 +16,85 @@ import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.spi.ILoggingEvent; import ch.qos.logback.core.Context; +import ch.qos.logback.core.pattern.CompositeConverter; import ch.qos.logback.core.pattern.Converter; import ch.qos.logback.core.pattern.ConverterUtil; import ch.qos.logback.core.pattern.PostCompileProcessor; public class EnsureExceptionHandling implements PostCompileProcessor { - /** - * This implementation checks if any of the converters in the chain handles - * exceptions. If not, then this method adds a - * {@link ExtendedThrowableProxyConverter} instance to the end of the chain. - *

- * This allows appenders using this layout to output exception information - * event if the user forgets to add %ex to the pattern. Note that the - * appenders defined in the Core package are not aware of exceptions nor - * LoggingEvents. - *

- * If for some reason the user wishes to NOT print exceptions, then she can - * add %nopex to the pattern. - * - * - */ - public void process(Context context, Converter head) { - if (head == null) { - // this should never happen - throw new IllegalArgumentException("cannot process empty chain"); - } - if (!chainHandlesThrowable(head)) { - Converter tail = ConverterUtil.findTail(head); - Converter exConverter = null; - LoggerContext loggerContext = (LoggerContext) context; - if (loggerContext.isPackagingDataEnabled()) { - exConverter = new ExtendedThrowableProxyConverter(); - } else { - exConverter = new ThrowableProxyConverter(); - } - tail.setNext(exConverter); - } - } + /** + * This implementation checks if any of the converters in the chain handles + * exceptions. If not, then this method adds a + * {@link ExtendedThrowableProxyConverter} instance to the end of the chain. + *

+ * This allows appenders using this layout to output exception information event + * if the user forgets to add %ex to the pattern. Note that the appenders + * defined in the Core package are not aware of exceptions nor LoggingEvents. + *

+ * If for some reason the user wishes to NOT print exceptions, then she can add + * %nopex to the pattern. + * + * + */ + public void process(Context context, Converter head) { + if (head == null) { + // this should never happen + throw new IllegalArgumentException("cannot process empty chain"); + } + if (!chainHandlesThrowable(head)) { + Converter tail = ConverterUtil.findTail(head); + Converter exConverter = null; + LoggerContext loggerContext = (LoggerContext) context; + if (loggerContext.isPackagingDataEnabled()) { + exConverter = new ExtendedThrowableProxyConverter(); + } else { + exConverter = new ThrowableProxyConverter(); + } + tail.setNext(exConverter); + } + } - /** - * This method computes whether a chain of converters handles exceptions or - * not. - * - * @param head - * The first element of the chain - * @return true if can handle throwables contained in logging events - */ - public boolean chainHandlesThrowable(Converter head) { - Converter c = head; - while (c != null) { - if (c instanceof ThrowableHandlingConverter) { - return true; - } - c = c.getNext(); - } - return false; - } + /** + * This method computes whether a chain of converters handles exceptions or not. + * + * @param head The first element of the chain + * @return true if can handle throwables contained in logging events + */ + public boolean chainHandlesThrowable(Converter head) { + Converter c = head; + while (c != null) { + if (c instanceof ThrowableHandlingConverter) { + return true; + } else if (c instanceof CompositeConverter) { + if (compositeHandlesThrowable((CompositeConverter) c)) { + return true; + } + } + c = c.getNext(); + } + return false; + } + + /** + * This method computes whether a composite converter handles exceptions or not. + * + * @param converter The composite converter + * @return true if can handle throwables contained in logging events + */ + public boolean compositeHandlesThrowable(CompositeConverter compositeConverter) { + Converter childConverter = compositeConverter.getChildConverter(); + + for (Converter c = childConverter; c != null; c = c.getNext()) { + if (c instanceof ThrowableHandlingConverter) { + return true; + } else if (c instanceof CompositeConverter) { + boolean r = compositeHandlesThrowable((CompositeConverter) c); + if (r) + return true; + } + + } + return false; + } } diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/pattern/EnsureExceptionHandlingTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/pattern/EnsureExceptionHandlingTest.java new file mode 100644 index 0000000000..074ca7b26b --- /dev/null +++ b/logback-classic/src/test/java/ch/qos/logback/classic/pattern/EnsureExceptionHandlingTest.java @@ -0,0 +1,50 @@ +package ch.qos.logback.classic.pattern; + +import org.junit.Before; +import org.junit.Test; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.LoggerContext; +import ch.qos.logback.classic.PatternLayout; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.classic.spi.LoggingEvent; + +public class EnsureExceptionHandlingTest { + + private PatternLayout pl = new PatternLayout(); + private LoggerContext lc = new LoggerContext(); + Logger logger = lc.getLogger(this.getClass()); + + static final String XTH = "xth"; + static final String XCC = "xcc"; + + @Before + public void setUp() { + pl.setContext(lc); + pl.getInstanceConverterMap().put(XTH, XThrowableHandlingConverter.class.getName()); + pl.getInstanceConverterMap().put(XCC, XCompositeConverter.class.getName()); + } + + ILoggingEvent makeLoggingEvent(String msg, Exception ex) { + return new LoggingEvent(EnsureExceptionHandlingTest.class.getName(), logger, + Level.INFO, msg, ex, null); + } + + @Test + public void smoke() { + pl.setPattern("%m %"+XTH+")"); + pl.start(); + ILoggingEvent le = makeLoggingEvent("assert", null); + pl.doLayout(le); + } + + @Test + public void withinComposite() { + pl.setPattern("%m %"+XCC+"(%"+XTH+")"); + pl.start(); + ILoggingEvent le = makeLoggingEvent("assert", null); + pl.doLayout(le); + } + +} diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/pattern/XCompositeConverter.java b/logback-classic/src/test/java/ch/qos/logback/classic/pattern/XCompositeConverter.java new file mode 100644 index 0000000000..1ac92f1e11 --- /dev/null +++ b/logback-classic/src/test/java/ch/qos/logback/classic/pattern/XCompositeConverter.java @@ -0,0 +1,21 @@ +package ch.qos.logback.classic.pattern; + +import static org.junit.Assert.assertNull; + +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.pattern.CompositeConverter; + +public class XCompositeConverter extends CompositeConverter{ + + void assertNoNext() { + assertNull("converter instance has next element", this.getNext()); + } + + @Override + protected String transform(ILoggingEvent event, String in) { + if (event.getMessage().contains("assert")) + assertNoNext(); + return ""; + } + +} diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/pattern/XThrowableHandlingConverter.java b/logback-classic/src/test/java/ch/qos/logback/classic/pattern/XThrowableHandlingConverter.java new file mode 100644 index 0000000000..f927068203 --- /dev/null +++ b/logback-classic/src/test/java/ch/qos/logback/classic/pattern/XThrowableHandlingConverter.java @@ -0,0 +1,20 @@ +package ch.qos.logback.classic.pattern; + +import static org.junit.Assert.assertNull; + +import ch.qos.logback.classic.spi.ILoggingEvent; + +public class XThrowableHandlingConverter extends ThrowableHandlingConverter { + + void assertNoNext() { + assertNull("has next", this.getNext()); + } + + @Override + public String convert(ILoggingEvent event) { + if (event.getMessage().contains("assert")) + assertNoNext(); + return ""; + } + +}