diff --git a/reactor-core/src/main/java/reactor/util/Loggers.java b/reactor-core/src/main/java/reactor/util/Loggers.java index e481d91fdc..3b01fc754f 100644 --- a/reactor-core/src/main/java/reactor/util/Loggers.java +++ b/reactor-core/src/main/java/reactor/util/Loggers.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2022 VMware Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2016-2025 VMware Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -353,7 +353,7 @@ public void trace(String msg) { @Override public void trace(String format, Object... arguments) { - logger.log(Level.FINEST, format(format, arguments)); + logWithPotentialThrowable(Level.FINEST, format, arguments); } @Override @@ -373,7 +373,7 @@ public void debug(String msg) { @Override public void debug(String format, Object... arguments) { - logger.log(Level.FINE, format(format, arguments)); + logWithPotentialThrowable(Level.FINE, format, arguments); } @Override @@ -393,7 +393,7 @@ public void info(String msg) { @Override public void info(String format, Object... arguments) { - logger.log(Level.INFO, format(format, arguments)); + logWithPotentialThrowable(Level.INFO, format, arguments); } @Override @@ -413,7 +413,7 @@ public void warn(String msg) { @Override public void warn(String format, Object... arguments) { - logger.log(Level.WARNING, format(format, arguments)); + logWithPotentialThrowable(Level.WARNING, format, arguments); } @Override @@ -433,7 +433,7 @@ public void error(String msg) { @Override public void error(String format, Object... arguments) { - logger.log(Level.SEVERE, format(format, arguments)); + logWithPotentialThrowable(Level.SEVERE, format, arguments); } @Override @@ -442,18 +442,52 @@ public void error(String msg, Throwable t) { } @Nullable - final String format(@Nullable String from, @Nullable Object... arguments){ - if(from != null) { + private String format(@Nullable String from, @Nullable Object[] arguments) { + return format(from, arguments, false); + } + + @Nullable + private String format(@Nullable String from, @Nullable Object[] arguments, boolean skipLast) { + if (from != null) { String computed = from; if (arguments != null && arguments.length != 0) { - for (Object argument : arguments) { - computed = computed.replaceFirst("\\{\\}", Matcher.quoteReplacement(String.valueOf(argument))); + int lastIndex = arguments.length; + if (skipLast) { + --lastIndex; + } + + for (int index = 0; index < lastIndex; ++index) { + computed = computed.replaceFirst("\\{\\}", Matcher.quoteReplacement(String.valueOf(arguments[index]))); } } return computed; } return null; } + + private void logWithPotentialThrowable(Level level, String format, Object... arguments) { + Throwable t = getPotentialThrowable(arguments); + if (t != null) { + logger.log(level, format(format, arguments, true), t); + return; + } + + logger.log(level, format(format, arguments)); + } + + @Nullable + private Throwable getPotentialThrowable(Object... arguments) { + if (arguments == null) { + return null; + } + + int length = arguments.length; + if (length > 0 && arguments[length - 1] instanceof Throwable) { + return (Throwable) arguments[length - 1]; + } + + return null; + } } private static class JdkLoggerFactory implements Function { @@ -494,12 +528,22 @@ public String getName() { } @Nullable - final String format(@Nullable String from, @Nullable Object... arguments){ - if(from != null) { + private String format(@Nullable String from, @Nullable Object[] arguments) { + return format(from, arguments, false); + } + + @Nullable + private String format(@Nullable String from, @Nullable Object[] arguments, boolean skipLast) { + if (from != null) { String computed = from; if (arguments != null && arguments.length != 0) { - for (Object argument : arguments) { - computed = computed.replaceFirst("\\{\\}", Matcher.quoteReplacement(String.valueOf(argument))); + int lastIndex = arguments.length; + if (skipLast) { + --lastIndex; + } + + for (int index = 0; index < lastIndex; ++index) { + computed = computed.replaceFirst("\\{\\}", Matcher.quoteReplacement(String.valueOf(arguments[index]))); } } return computed; @@ -507,6 +551,41 @@ final String format(@Nullable String from, @Nullable Object... arguments){ return null; } + private synchronized void logWithPotentialThrowable(PrintStream logger, String level, String format, Object... arguments) { + Throwable t = getPotentialThrowable(arguments); + if (t != null) { + logger.format( + "[%s] (%s) %s\n", + level.toUpperCase(), + Thread.currentThread().getName(), + format(format, arguments, true) + ); + t.printStackTrace(logger); + return; + } + + logger.format( + "[%s] (%s) %s\n", + level.toUpperCase(), + Thread.currentThread().getName(), + format(format, arguments) + ); + } + + @Nullable + private static Throwable getPotentialThrowable(Object... arguments) { + if (arguments == null) { + return null; + } + + int length = arguments.length; + if (length > 0 && arguments[length - 1] instanceof Throwable) { + return (Throwable) arguments[length - 1]; + } + + return null; + } + @Override public boolean isTraceEnabled() { return identifier.verbose; @@ -521,12 +600,13 @@ public synchronized void trace(String msg) { } @Override - public synchronized void trace(String format, Object... arguments) { + public void trace(String format, Object... arguments) { if (!identifier.verbose) { return; } - this.log.format("[TRACE] (%s) %s\n", Thread.currentThread().getName(), format(format, arguments)); + logWithPotentialThrowable(this.log, "TRACE", format, arguments); } + @Override public synchronized void trace(String msg, Throwable t) { if (!identifier.verbose) { @@ -550,11 +630,11 @@ public synchronized void debug(String msg) { } @Override - public synchronized void debug(String format, Object... arguments) { + public void debug(String format, Object... arguments) { if (!identifier.verbose) { return; } - this.log.format("[DEBUG] (%s) %s\n", Thread.currentThread().getName(), format(format, arguments)); + logWithPotentialThrowable(this.log, "DEBUG", format, arguments); } @Override @@ -577,8 +657,8 @@ public synchronized void info(String msg) { } @Override - public synchronized void info(String format, Object... arguments) { - this.log.format("[ INFO] (%s) %s\n", Thread.currentThread().getName(), format(format, arguments)); + public void info(String format, Object... arguments) { + logWithPotentialThrowable(this.log, " INFO", format, arguments); } @Override @@ -598,8 +678,8 @@ public synchronized void warn(String msg) { } @Override - public synchronized void warn(String format, Object... arguments) { - this.err.format("[ WARN] (%s) %s\n", Thread.currentThread().getName(), format(format, arguments)); + public void warn(String format, Object... arguments) { + logWithPotentialThrowable(this.err, " WARN", format, arguments); } @Override @@ -619,8 +699,8 @@ public synchronized void error(String msg) { } @Override - public synchronized void error(String format, Object... arguments) { - this.err.format("[ERROR] (%s) %s\n", Thread.currentThread().getName(), format(format, arguments)); + public void error(String format, Object... arguments) { + logWithPotentialThrowable(this.err, "ERROR", format, arguments); } @Override diff --git a/reactor-core/src/test/java/reactor/util/ConsoleLoggerTest.java b/reactor-core/src/test/java/reactor/util/ConsoleLoggerTest.java index c4d8b087bd..16a66debbc 100644 --- a/reactor-core/src/test/java/reactor/util/ConsoleLoggerTest.java +++ b/reactor-core/src/test/java/reactor/util/ConsoleLoggerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2017-2022 VMware Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2017-2025 VMware Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -80,6 +80,17 @@ public void trace2() throws Exception { "\tat reactor.util.ConsoleLoggerTest"); } + @Test + public void traceWithThrowable() throws Exception { + logger.trace("{} with cause {}", "test", CAUSE); + + assertThat(errContent.size()).isZero(); + assertThat(outContent.toString()) + .startsWith("[TRACE] (" + Thread.currentThread().getName() + ") test with cause {}" + + "\njava.lang.IllegalStateException: cause\n" + + "\tat reactor.util.ConsoleLoggerTest"); + } + @Test public void traceNulls() { logger.trace("vararg {} is {}", (Object[]) null); @@ -136,6 +147,17 @@ public void debug2() throws Exception { "\tat reactor.util.ConsoleLoggerTest"); } + @Test + public void debugWithThrowable() throws Exception { + logger.debug("{} with cause {}", "test", CAUSE); + + assertThat(errContent.size()).isZero(); + assertThat(outContent.toString()) + .startsWith("[DEBUG] (" + Thread.currentThread().getName() + ") test with cause {}" + + "\njava.lang.IllegalStateException: cause\n" + + "\tat reactor.util.ConsoleLoggerTest"); + } + @Test public void debugNulls() { logger.debug("vararg {} is {}", (Object[]) null); @@ -192,6 +214,17 @@ public void info2() throws Exception { "\tat reactor.util.ConsoleLoggerTest"); } + @Test + public void infoWithThrowable() throws Exception { + logger.info("{} with cause {}", "test", CAUSE); + + assertThat(errContent.size()).isZero(); + assertThat(outContent.toString()) + .startsWith("[ INFO] (" + Thread.currentThread().getName() + ") test with cause {}" + + "\njava.lang.IllegalStateException: cause\n" + + "\tat reactor.util.ConsoleLoggerTest"); + } + @Test public void infoNulls() { logger.info("vararg {} is {}", (Object[]) null); @@ -236,6 +269,17 @@ public void warn2() throws Exception { "\tat reactor.util.ConsoleLoggerTest"); } + @Test + public void warnWithThrowable() throws Exception { + logger.warn("{} with cause {}", "test", CAUSE); + + assertThat(outContent.size()).isZero(); + assertThat(errContent.toString()) + .startsWith("[ WARN] (" + Thread.currentThread().getName() + ") test with cause {}" + + "\njava.lang.IllegalStateException: cause\n" + + "\tat reactor.util.ConsoleLoggerTest"); + } + @Test public void warnNulls() { logger.warn("vararg {} is {}", (Object[]) null); @@ -279,6 +323,17 @@ public void error2() throws Exception { "\tat reactor.util.ConsoleLoggerTest"); } + @Test + public void errorWithThrowable() throws Exception { + logger.error("{} with cause {}", "test", CAUSE); + + assertThat(outContent.size()).isZero(); + assertThat(errContent.toString()) + .startsWith("[ERROR] (" + Thread.currentThread().getName() + ") test with cause {}" + + "\njava.lang.IllegalStateException: cause\n" + + "\tat reactor.util.ConsoleLoggerTest"); + } + @Test public void errorNulls() { logger.error("vararg {} is {}", (Object[]) null); diff --git a/reactor-core/src/test/java/reactor/util/JdkLoggerTest.java b/reactor-core/src/test/java/reactor/util/JdkLoggerTest.java index aad18ac5c2..51b3afc218 100644 --- a/reactor-core/src/test/java/reactor/util/JdkLoggerTest.java +++ b/reactor-core/src/test/java/reactor/util/JdkLoggerTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2018-2021 VMware Inc. or its affiliates, All Rights Reserved. + * Copyright (c) 2018-2025 VMware Inc. or its affiliates, All Rights Reserved. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -22,7 +22,6 @@ import java.util.logging.Logger; import org.junit.jupiter.api.Test; -import org.mockito.Mockito; import static org.assertj.core.api.Assertions.assertThat; @@ -30,33 +29,18 @@ public class JdkLoggerTest { @Test public void formatNullFormat() { - Loggers.JdkLogger jdkLogger = new Loggers.JdkLogger(Mockito.mock(java.util.logging.Logger.class)); + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINE); + + jdkLogger.debug(null, (Object[]) null); - assertThat(jdkLogger.format(null, (Object[]) null)) - .as("null format should be interpreted as null") - .isNull(); + assertThat(log.toString()).isEqualTo("null"); } @Test public void nullFormatIsAcceptedByUnderlyingLogger() { StringBuilder log = new StringBuilder(); - Logger underlyingLogger = Logger.getAnonymousLogger(); - underlyingLogger.setLevel(Level.FINEST); - underlyingLogger.addHandler( - new Handler() { - @Override - public void publish(LogRecord record) { - log.append(record.getMessage()); - } - - @Override - public void flush() { } - - @Override - public void close() throws SecurityException { } - }); - - Loggers.JdkLogger jdkLogger = new Loggers.JdkLogger(underlyingLogger); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINEST); jdkLogger.trace(null, null, null); @@ -65,20 +49,163 @@ public void close() throws SecurityException { } @Test public void formatNullVararg() { - Loggers.JdkLogger jdkLogger= new Loggers.JdkLogger(Mockito.mock(java.util.logging.Logger.class)); + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.INFO); - assertThat(jdkLogger.format("test {} is {}", (Object[]) null)) - .as("format should be returned as is") - .isEqualTo("test {} is {}"); + jdkLogger.info("test {} is {}", (Object[]) null); + + assertThat(log.toString()).isEqualTo("test {} is {}"); } @Test public void formatNullParamInVararg() { - Loggers.JdkLogger jdkLogger= new Loggers.JdkLogger(Mockito.mock(java.util.logging.Logger.class)); + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINEST); + + jdkLogger.trace("test {} is {}", null, null); + + assertThat(log.toString()).isEqualTo("test null is null"); + } + + @Test + public void trace() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINEST); - assertThat(jdkLogger.format("test {} is {}", null, null)) - .as("placeholders should be replaced by null") - .isEqualTo("test null is null"); + jdkLogger.trace("test {} is {}", "foo", "bar"); + + assertThat(log.toString()).isEqualTo("test foo is bar"); } + @Test + public void traceWithThrowable() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINEST); + + Throwable t = new IllegalAccessError(); + jdkLogger.trace("test {} is {}", "foo", "bar", t); + + assertThat(log.toString()).startsWith("test foo is bar" + + "\njava.lang.IllegalAccessError\n" + + "\tat reactor.util.JdkLoggerTest.traceWithThrowable"); + } + + @Test + public void debug() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINE); + + jdkLogger.debug("test {} is {}", "foo", "bar"); + + assertThat(log.toString()).isEqualTo("test foo is bar"); + } + + @Test + public void debugWithThrowable() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINE); + + Throwable t = new IllegalAccessError(); + jdkLogger.debug("test {} is {}", "foo", "bar", t); + + assertThat(log.toString()).startsWith("test foo is bar" + + "\njava.lang.IllegalAccessError\n" + + "\tat reactor.util.JdkLoggerTest.debugWithThrowable"); + } + + @Test + public void info() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINE); + + jdkLogger.info("test {} is {}", "foo", "bar"); + + assertThat(log.toString()).isEqualTo("test foo is bar"); + } + + @Test + public void infoWithThrowable() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.FINE); + + Throwable t = new IllegalAccessError(); + jdkLogger.info("test {} is {}", "foo", "bar", t); + + assertThat(log.toString()).startsWith("test foo is bar" + + "\njava.lang.IllegalAccessError\n" + + "\tat reactor.util.JdkLoggerTest.infoWithThrowable"); + } + + @Test + public void warn() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.WARNING); + + jdkLogger.warn("test {} is {}", "foo", "bar"); + + assertThat(log.toString()).isEqualTo("test foo is bar"); + } + + @Test + public void warnWithThrowable() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.WARNING); + + Throwable t = new IllegalAccessError(); + jdkLogger.warn("test {} is {}", "foo", "bar", t); + + assertThat(log.toString()).startsWith("test foo is bar" + + "\njava.lang.IllegalAccessError\n" + + "\tat reactor.util.JdkLoggerTest.warnWithThrowable"); + } + + @Test + public void error() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.SEVERE); + + jdkLogger.error("test {} is {}", "foo", "bar"); + + assertThat(log.toString()).isEqualTo("test foo is bar"); + } + + @Test + public void errorWithThrowable() { + StringBuilder log = new StringBuilder(); + Loggers.JdkLogger jdkLogger = getJdkLogger(log, Level.SEVERE); + + Throwable t = new IllegalAccessError(); + jdkLogger.error("test {} is {}", "foo", "bar", t); + + assertThat(log.toString()).startsWith("test foo is bar" + + "\njava.lang.IllegalAccessError\n" + + "\tat reactor.util.JdkLoggerTest.errorWithThrowable"); + } + + private Loggers.JdkLogger getJdkLogger(StringBuilder log, Level level) { + Logger underlyingLogger = Logger.getAnonymousLogger(); + underlyingLogger.setLevel(level); + underlyingLogger.addHandler( + new Handler() { + @Override + public void publish(LogRecord record) { + log.append(record.getMessage()); + + if (record.getThrown() != null) { + log.append("\n").append(record.getThrown().toString()); + for (StackTraceElement element : record.getThrown().getStackTrace()) { + log.append("\n\tat ").append(element.toString()); + } + } + } + + @Override + public void flush() { } + + @Override + public void close() throws SecurityException { } + }); + + return new Loggers.JdkLogger(underlyingLogger); + } }