From 33d1dcb6d66fb4253b6492a3a7c44f9bd84fdb07 Mon Sep 17 00:00:00 2001 From: Ceki Gulcu Date: Tue, 11 Jul 2023 15:20:49 +0200 Subject: [PATCH] test for circular throwables for JsonAppender Signed-off-by: Ceki Gulcu --- .../logback/classic/encoder/JsonEncoder.java | 16 +++++++- .../logback/classic/spi/ThrowableProxy.java | 11 ++--- .../classic/encoder/JsonEncoderTest.java | 41 ++++++++----------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/encoder/JsonEncoder.java b/logback-classic/src/main/java/ch/qos/logback/classic/encoder/JsonEncoder.java index f0d58434e0..aaa21b396f 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/encoder/JsonEncoder.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/encoder/JsonEncoder.java @@ -18,7 +18,6 @@ import ch.qos.logback.classic.spi.IThrowableProxy; import ch.qos.logback.classic.spi.LoggerContextVO; import ch.qos.logback.classic.spi.StackTraceElementProxy; -import ch.qos.logback.classic.spi.ThrowableProxy; import ch.qos.logback.core.CoreConstants; import ch.qos.logback.core.encoder.EncoderBase; import org.slf4j.Marker; @@ -31,7 +30,6 @@ import static ch.qos.logback.core.CoreConstants.COLON_CHAR; import static ch.qos.logback.core.CoreConstants.COMMA_CHAR; import static ch.qos.logback.core.CoreConstants.DOUBLE_QUOTE_CHAR; -import static ch.qos.logback.core.CoreConstants.SUPPRESSED; import static ch.qos.logback.core.CoreConstants.UTF_8_CHARSET; import static ch.qos.logback.core.encoder.JsonEscapeUtil.jsonEscapeString; import static ch.qos.logback.core.model.ModelConstants.NULL_STR; @@ -73,6 +71,8 @@ public class JsonEncoder extends EncoderBase { public static final String THROWABLE_ATTR_NAME = "throwable"; + private static final String CYCLIC_THROWABLE_ATTR_NAME = "cyclic"; + public static final String CAUSE_ATTR_NAME = "cause"; public static final String SUPPRESSED_ATTR_NAME = "suppressed"; @@ -199,6 +199,9 @@ private void appendMap(StringBuilder sb, String attrName, Map ma private void appendThrowableProxy(StringBuilder sb, String attributeName, IThrowableProxy itp) { + + // in the nominal case, attributeName != null. However, attributeName will be null for suppressed + // IThrowableProxy array, in which case no attribute name is needed if(attributeName != null) { sb.append(QUOTE).append(attributeName).append(QUOTE_COL); if (itp == null) { @@ -210,10 +213,19 @@ private void appendThrowableProxy(StringBuilder sb, String attributeName, IThrow sb.append(OPEN_OBJ); appenderMember(sb, CLASS_NAME_ATTR_NAME, nullSafeStr(itp.getClassName())); + sb.append(VALUE_SEPARATOR); appenderMember(sb, MESSAGE_ATTR_NAME, jsonEscape(itp.getMessage())); + + + if(itp.isCyclic()) { + sb.append(VALUE_SEPARATOR); + appenderMember(sb, CYCLIC_THROWABLE_ATTR_NAME, jsonEscape("true")); + } + sb.append(VALUE_SEPARATOR); appendSTEPArray(sb, itp.getStackTraceElementProxyArray(), itp.getCommonFrames()); + if(itp.getCommonFrames() != 0) { sb.append(VALUE_SEPARATOR); appenderMemberWithIntValue(sb, COMMON_FRAMES_COUNT_ATTR_NAME, itp.getCommonFrames()); diff --git a/logback-classic/src/main/java/ch/qos/logback/classic/spi/ThrowableProxy.java b/logback-classic/src/main/java/ch/qos/logback/classic/spi/ThrowableProxy.java index b94e9703d3..b4b816c16e 100644 --- a/logback-classic/src/main/java/ch/qos/logback/classic/spi/ThrowableProxy.java +++ b/logback-classic/src/main/java/ch/qos/logback/classic/spi/ThrowableProxy.java @@ -41,7 +41,8 @@ public class ThrowableProxy implements IThrowableProxy { private transient PackagingDataCalculator packagingDataCalculator; private boolean calculatedPackageData = false; - private boolean circular; + // the getter is called isCyclic + private boolean cyclic; private static final ThrowableProxy[] NO_SUPPRESSED = new ThrowableProxy[0]; @@ -51,12 +52,12 @@ public ThrowableProxy(Throwable throwable) { } // used for circular exceptions - private ThrowableProxy(Throwable circular, boolean isCircular) { + private ThrowableProxy(Throwable circular, boolean isCyclic) { this.throwable = circular; this.className = circular.getClass().getName(); this.message = circular.getMessage(); this.stackTraceElementProxyArray = EMPTY_STEP; - this.circular = true; + this.cyclic = true; } public ThrowableProxy(Throwable throwable, Set alreadyProcessedSet) { @@ -65,7 +66,7 @@ public ThrowableProxy(Throwable throwable, Set alreadyProcessedSet) { this.className = throwable.getClass().getName(); this.message = throwable.getMessage(); this.stackTraceElementProxyArray = ThrowableProxyUtil.steArrayToStepArray(throwable.getStackTrace()); - this.circular = false; + this.cyclic = false; alreadyProcessedSet.add(throwable); @@ -123,7 +124,7 @@ public StackTraceElementProxy[] getStackTraceElementProxyArray() { @Override public boolean isCyclic() { - return circular; + return cyclic; } public int getCommonFrames() { diff --git a/logback-classic/src/test/java/ch/qos/logback/classic/encoder/JsonEncoderTest.java b/logback-classic/src/test/java/ch/qos/logback/classic/encoder/JsonEncoderTest.java index 932cb5f166..a7ed08da6a 100644 --- a/logback-classic/src/test/java/ch/qos/logback/classic/encoder/JsonEncoderTest.java +++ b/logback-classic/src/test/java/ch/qos/logback/classic/encoder/JsonEncoderTest.java @@ -168,30 +168,6 @@ private static boolean compareKeyValuePairLists(List leftList, Lis } - // private JsonLoggingEvent stringToJsonLoggingEvent(String resultString) throws JsonProcessingException { - // ObjectMapper objectMapper = new ObjectMapper(); - // JsonNode jsonNode = objectMapper.readTree(resultString); - // JsonLoggingEvent resultEvent = objectMapper.treeToValue(jsonNode, JsonLoggingEvent.class); - // String levelStr = jsonNode.at("/level").asText(); - // Level level = Level.toLevel(levelStr); - // - // JsonNode markersNode = jsonNode.at("/markers"); - // if(markersNode!=null && markersNode.isArray()) { - // List markerList = new ArrayList<>(); - // Iterator itr = markersNode.iterator(); - // while (itr.hasNext()) { - // JsonNode item=itr.next(); - // String markerStr = item.asText(); - // Marker marker = markerFactory.getMarker(markerStr); - // markerList.add(marker); - // } - // resultEvent.markerList = markerList; - // } - // - // resultEvent.level = level; - // return resultEvent; - // } - @Test void withMarkers() throws JsonProcessingException { LoggingEvent event = new LoggingEvent("x", logger, Level.WARN, "hello", null, null); @@ -280,6 +256,23 @@ void withThrowableHavingCause() throws JsonProcessingException { compareEvents(event, resultEvent); } + @Test + void withThrowableHavingCyclicCause() throws JsonProcessingException { + Throwable cause = new IllegalStateException("test cause"); + + Throwable t = new RuntimeException("test", cause); + cause.initCause(t); + + LoggingEvent event = new LoggingEvent("in withThrowableHavingCyclicCause test", logger, Level.WARN, "hello kvp", t, null); + + byte[] resultBytes = jsonEncoder.encode(event); + String resultString = new String(resultBytes, StandardCharsets.UTF_8); + //System.out.println(resultString); + JsonLoggingEvent resultEvent = stringToLoggingEventMapper.mapStringToLoggingEvent(resultString); + compareEvents(event, resultEvent); + } + + @Test void withThrowableHavingSuppressed() throws JsonProcessingException { Throwable suppressed = new IllegalStateException("test suppressed");