Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved handling of throwables in message formatting. #391

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 3 additions & 7 deletions slf4j-api/src/main/java/org/slf4j/helpers/AbstractLogger.java
Original file line number Diff line number Diff line change
Expand Up @@ -395,13 +395,7 @@ private void handle2ArgsCall(Level level, Marker marker, String msg, Object arg1
}

private void handleArgArrayCall(Level level, Marker marker, String msg, Object[] args) {
Throwable throwableCandidate = MessageFormatter.getThrowableCandidate(args);
if (throwableCandidate != null) {
Object[] trimmedCopy = MessageFormatter.trimmedCopy(args);
handleNormalizedLoggingCall(level, marker, msg, trimmedCopy, throwableCandidate);
} else {
handleNormalizedLoggingCall(level, marker, msg, args, null);
}
handleNormalizedLoggingCall(level, marker, msg, args, null);
}

abstract protected String getFullyQualifiedCallerName();
Expand All @@ -411,6 +405,8 @@ private void handleArgArrayCall(Level level, Marker marker, String msg, Object[]
*
* <p>This method assumes that the separation of the args array into actual
* objects and a throwable has been already operated.
*
* TODO: I think it should accept formatted message rather than pattern and arguments.
*
* @param level the SLF4J level for this event
* @param marker The marker to be used for this event, may be null.
Expand Down
10 changes: 2 additions & 8 deletions slf4j-api/src/main/java/org/slf4j/helpers/FormattingTuple.java
Original file line number Diff line number Diff line change
Expand Up @@ -35,26 +35,20 @@ public class FormattingTuple {

private final String message;
private final Throwable throwable;
private final Object[] argArray;

public FormattingTuple(String message) {
this(message, null, null);
this(message, null);
}

public FormattingTuple(String message, Object[] argArray, Throwable throwable) {
public FormattingTuple(String message, Throwable throwable) {
this.message = message;
this.throwable = throwable;
this.argArray = argArray;
}

public String getMessage() {
return message;
}

public Object[] getArgArray() {
return argArray;
}

public Throwable getThrowable() {
return throwable;
}
Expand Down
62 changes: 23 additions & 39 deletions slf4j-api/src/main/java/org/slf4j/helpers/MessageFormatter.java
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,7 @@ final public static FormattingTuple format(final String messagePattern, Object a
}

final public static FormattingTuple arrayFormat(final String messagePattern, final Object[] argArray) {
Throwable throwableCandidate = MessageFormatter.getThrowableCandidate(argArray);
Object[] args = argArray;
if (throwableCandidate != null) {
args = MessageFormatter.trimmedCopy(argArray);
}
return arrayFormat(messagePattern, args, throwableCandidate);
return arrayFormat(messagePattern, argArray, null);
}

/**
Expand All @@ -166,26 +161,40 @@ final public static FormattingTuple arrayFormat(final String messagePattern, fin
* @param messagePattern
* @param argArray
*/
@Deprecated
final public static String basicArrayFormat(final String messagePattern, final Object[] argArray) {
FormattingTuple ft = arrayFormat(messagePattern, argArray, null);
return ft.getMessage();
}

public static String basicArrayFormat(NormalizedParameters np) {
return basicArrayFormat(np.getMessage(), np.getArguments());
/** Resolves throwable to be used in log message.
*
* @param index index of first array entry that was not bound to format placeholder
*/
final static Throwable resolveThrowable(final Object[] argArray, final int index, final Throwable throwable) {
if (throwable != null) {
return throwable;
}
if(argArray != null && argArray.length > index) {
final Object lastEntry = argArray[argArray.length - 1];
if (lastEntry instanceof Throwable) {
return (Throwable) lastEntry;
}
}
return null;
}

final public static FormattingTuple arrayFormat(final String messagePattern, final Object[] argArray, Throwable throwable) {

if (messagePattern == null) {
return new FormattingTuple(null, argArray, throwable);
return new FormattingTuple(null, resolveThrowable(argArray, 0, throwable));
}

if (argArray == null) {
return new FormattingTuple(messagePattern);
return new FormattingTuple(messagePattern, throwable);
}

int i = 0;
int i = 0; // index in the pattern
int j;
// use string builder for better multicore performance
StringBuilder sbuf = new StringBuilder(messagePattern.length() + 50);
Expand All @@ -198,11 +207,11 @@ final public static FormattingTuple arrayFormat(final String messagePattern, fin
if (j == -1) {
// no more variables
if (i == 0) { // this is a simple string
return new FormattingTuple(messagePattern, argArray, throwable);
return new FormattingTuple(messagePattern, resolveThrowable(argArray, L, throwable));
} else { // add the tail string which contains no variables and return
// the result.
sbuf.append(messagePattern, i, messagePattern.length());
return new FormattingTuple(sbuf.toString(), argArray, throwable);
return new FormattingTuple(sbuf.toString(), resolveThrowable(argArray, L, throwable));
}
} else {
if (isEscapedDelimeter(messagePattern, j)) {
Expand All @@ -229,7 +238,7 @@ final public static FormattingTuple arrayFormat(final String messagePattern, fin
}
// append the characters following the last {} pair.
sbuf.append(messagePattern, i, messagePattern.length());
return new FormattingTuple(sbuf.toString(), argArray, throwable);
return new FormattingTuple(sbuf.toString(), throwable);
}

final static boolean isEscapedDelimeter(String messagePattern, int delimeterStartIndex) {
Expand Down Expand Up @@ -402,29 +411,4 @@ private static void doubleArrayAppend(StringBuilder sbuf, double[] a) {
}
sbuf.append(']');
}

/**
* Helper method to determine if an {@link Object} array contains a {@link Throwable} as last element
*
* @param argArray
* The arguments off which we want to know if it contains a {@link Throwable} as last element
* @return if the last {@link Object} in argArray is a {@link Throwable} this method will return it,
* otherwise it returns null
*/
public static Throwable getThrowableCandidate(final Object[] argArray) {
return NormalizedParameters.getThrowableCandidate(argArray);
}

/**
* Helper method to get all but the last element of an array
*
* @param argArray
* The arguments from which we want to remove the last element
*
* @return a copy of the array without the last element
*/
public static Object[] trimmedCopy(final Object[] argArray) {
return NormalizedParameters.trimmedCopy(argArray);
}

}
116 changes: 0 additions & 116 deletions slf4j-api/src/main/java/org/slf4j/helpers/NormalizedParameters.java

This file was deleted.