Skip to content

Commit

Permalink
Log exception stack trace for Throwable as last vararg (#3960)
Browse files Browse the repository at this point in the history 
Unifies `ConsoleLogger` and `JdkLogger` with `Slf4JLogger` to also
append the stack trace in case the last argument of the varargs
overloaded methods is a `Throwable`. 
Check https://www.slf4j.org/faq.html#paramException for details.

---------

Signed-off-by: raccoonback <kosb15@naver.com>
  • Loading branch information
@raccoonback
raccoonback authored Feb 4, 2025
1 parent adac9d9 commit fb3d31d
Show file tree
Hide file tree
Showing 3 changed files with 318 additions and 56 deletions.
128 changes: 104 additions & 24 deletions reactor-core/src/main/java/reactor/util/Loggers.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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<String, Logger> {
Expand Down Expand Up @@ -494,19 +528,64 @@ 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;
}
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;
Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
57 changes: 56 additions & 1 deletion reactor-core/src/test/java/reactor/util/ConsoleLoggerTest.java
View file
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
Loading

0 comments on commit fb3d31d

Please sign in to comment.