From 694ff4f8750362da443630e5eedc3eb255a197aa Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 9 Aug 2023 11:43:58 +0100 Subject: [PATCH] Include throwables from Logback errors when reporting invalid config Closes gh-32562 --- .../logging/logback/LogbackLoggingSystem.java | 21 +++++++++++++++---- .../logback/LogbackLoggingSystemTests.java | 13 +++++++++++- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index 6a73a973ac..468fbd6c86 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -167,17 +167,30 @@ public class LogbackLoggingSystem extends Slf4JLoggingSystem { catch (Exception ex) { throw new IllegalStateException("Could not initialize Logback logging from " + location, ex); } - List statuses = loggerContext.getStatusManager().getCopyOfStatusList(); + reportConfigurationErrorsIfNecessary(loggerContext); + } + + private void reportConfigurationErrorsIfNecessary(LoggerContext loggerContext) { StringBuilder errors = new StringBuilder(); - for (Status status : statuses) { + List suppressedExceptions = new ArrayList<>(); + for (Status status : loggerContext.getStatusManager().getCopyOfStatusList()) { if (status.getLevel() == Status.ERROR) { errors.append((errors.length() > 0) ? String.format("%n") : ""); errors.append(status.toString()); + if (status.getThrowable() != null) { + suppressedExceptions.add(status.getThrowable()); + } } } - if (errors.length() > 0) { - throw new IllegalStateException(String.format("Logback configuration error detected: %n%s", errors)); + if (errors.length() == 0) { + return; + } + IllegalStateException ex = new IllegalStateException( + String.format("Logback configuration error detected: %n%s", errors)); + for (Throwable suppressedException : suppressedExceptions) { + ex.addSuppressed(suppressedException); } + throw ex; } private void configureByResourceUrl(LoggingInitializationContext initializationContext, LoggerContext loggerContext, diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index dd2d67c186..a584b852de 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -38,6 +38,7 @@ import ch.qos.logback.core.ConsoleAppender; import ch.qos.logback.core.encoder.LayoutWrappingEncoder; import ch.qos.logback.core.rolling.RollingFileAppender; import ch.qos.logback.core.rolling.SizeAndTimeBasedRollingPolicy; +import ch.qos.logback.core.util.DynamicClassLoadingException; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -519,7 +520,7 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { initialize(this.initializationContext, null, null); LoggerContext loggerContext = (LoggerContext) StaticLoggerBinder.getSingleton().getLoggerFactory(); Map properties = loggerContext.getCopyOfPropertyMap(); - Set expectedProperties = new HashSet(); + Set expectedProperties = new HashSet<>(); ReflectionUtils.doWithFields(LogbackLoggingSystemProperties.class, (field) -> expectedProperties.add((String) field.get(null)), this::isPublicStaticFinal); expectedProperties.removeAll(Arrays.asList("LOG_FILE", "LOG_PATH")); @@ -624,6 +625,16 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { assertThat(encoder.getCharset()).isEqualTo(StandardCharsets.UTF_16); } + @Test + void whenConfigurationErrorIsDetectedUnderlyingCausesAreIncludedAsSuppressedExceptions() { + this.loggingSystem.beforeInitialize(); + assertThatIllegalStateException() + .isThrownBy(() -> initialize(this.initializationContext, "classpath:logback-broken.xml", + getLogFile(tmpDir() + "/tmp.log", null))) + .satisfies((ex) -> assertThat(ex.getSuppressed()) + .hasAtLeastOneElementOfType(DynamicClassLoadingException.class)); + } + private void initialize(LoggingInitializationContext context, String configLocation, LogFile logFile) { this.loggingSystem.getSystemProperties((ConfigurableEnvironment) context.getEnvironment()).apply(logFile); this.loggingSystem.initialize(context, configLocation, logFile);