exceptionfactory commented on a change in pull request #5093:
URL: https://github.com/apache/nifi/pull/5093#discussion_r637902460
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/logging/repository/StandardLogRepository.java
##########
@@ -82,14 +82,32 @@ public void addLogMessage(final LogLevel level, final
String format, final Objec
addLogMessage(level, formattedMessage, t);
}
- private void replaceThrowablesWithMessage(Object[] params) {
+ private void replaceThrowablesWithMessage(final Object[] params) {
for (int i = 0; i < params.length; i++) {
- if(params[i] instanceof Throwable) {
- params[i] = ((Throwable) params[i]).getLocalizedMessage();
+ if (params[i] instanceof Throwable) {
+ params[i] = getCauses((Throwable) params[i]);
}
}
}
+ private static String getCauses(final Throwable throwable) {
+ final StringBuilder builder = new StringBuilder();
+ if (throwable == null || throwable.getLocalizedMessage() == null) {
+ return "";
+ }
+ builder.append(throwable.getLocalizedMessage());
+ if (throwable.getCause() != null) {
+ builder.append("\n");
+ }
+ final Throwable firstCause = throwable.getCause();
+ for (Throwable t = firstCause; t != null; t = t.getCause()) {
+ builder.append("┗━▶ causes: ");
+ builder.append(t.getLocalizedMessage());
+ builder.append("\n");
Review comment:
This formatting approach will leave a trailing newline, which does not
seem optimal for message formatting.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/logging/repository/StandardLogRepository.java
##########
@@ -82,14 +82,32 @@ public void addLogMessage(final LogLevel level, final
String format, final Objec
addLogMessage(level, formattedMessage, t);
}
- private void replaceThrowablesWithMessage(Object[] params) {
+ private void replaceThrowablesWithMessage(final Object[] params) {
for (int i = 0; i < params.length; i++) {
- if(params[i] instanceof Throwable) {
- params[i] = ((Throwable) params[i]).getLocalizedMessage();
+ if (params[i] instanceof Throwable) {
+ params[i] = getCauses((Throwable) params[i]);
}
}
}
+ private static String getCauses(final Throwable throwable) {
+ final StringBuilder builder = new StringBuilder();
+ if (throwable == null || throwable.getLocalizedMessage() == null) {
+ return "";
+ }
+ builder.append(throwable.getLocalizedMessage());
+ if (throwable.getCause() != null) {
+ builder.append("\n");
+ }
+ final Throwable firstCause = throwable.getCause();
+ for (Throwable t = firstCause; t != null; t = t.getCause()) {
+ builder.append("┗━▶ causes: ");
Review comment:
The code appears to have a special character prior to `causes:` that
should be removed and replaced with a standard separator of some kind.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/logging/repository/StandardLogRepository.java
##########
@@ -82,14 +82,32 @@ public void addLogMessage(final LogLevel level, final
String format, final Objec
addLogMessage(level, formattedMessage, t);
}
- private void replaceThrowablesWithMessage(Object[] params) {
+ private void replaceThrowablesWithMessage(final Object[] params) {
for (int i = 0; i < params.length; i++) {
- if(params[i] instanceof Throwable) {
- params[i] = ((Throwable) params[i]).getLocalizedMessage();
+ if (params[i] instanceof Throwable) {
+ params[i] = getCauses((Throwable) params[i]);
}
}
}
+ private static String getCauses(final Throwable throwable) {
+ final StringBuilder builder = new StringBuilder();
+ if (throwable == null || throwable.getLocalizedMessage() == null) {
Review comment:
The `throwable` argument should never be null based on how this private
method is called.
##########
File path:
nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-framework-components/src/main/java/org/apache/nifi/logging/repository/StandardLogRepository.java
##########
@@ -82,14 +82,32 @@ public void addLogMessage(final LogLevel level, final
String format, final Objec
addLogMessage(level, formattedMessage, t);
}
- private void replaceThrowablesWithMessage(Object[] params) {
+ private void replaceThrowablesWithMessage(final Object[] params) {
for (int i = 0; i < params.length; i++) {
- if(params[i] instanceof Throwable) {
- params[i] = ((Throwable) params[i]).getLocalizedMessage();
+ if (params[i] instanceof Throwable) {
+ params[i] = getCauses((Throwable) params[i]);
}
}
}
+ private static String getCauses(final Throwable throwable) {
+ final StringBuilder builder = new StringBuilder();
+ if (throwable == null || throwable.getLocalizedMessage() == null) {
+ return "";
+ }
+ builder.append(throwable.getLocalizedMessage());
+ if (throwable.getCause() != null) {
+ builder.append("\n");
Review comment:
Instead of using the platform-dependent newline, what do you think about
using `System.lineSeparator()` here and on line 106? On the other hand, the
line separator may not translate well when it comes to formatting the message
for presentation in the browser, so it may be necessary to adjust the object
model instead of building a string.
```suggestion
builder.append(System.lineSeparator());
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
