From 98f27b73b48c2e676b0f990323679c0bb7e13b11 Mon Sep 17 00:00:00 2001 From: filoghost Date: Sun, 25 Jul 2021 11:57:25 +0200 Subject: [PATCH] Improve error logging --- .../common/nms/NMSErrors.java | 3 +- .../nms/v1_17_R1/VersionNMSManager.java | 4 +- .../plugin/HolographicDisplays.java | 2 +- .../plugin/log/ErrorDisplayInfo.java | 34 ++++++++ .../plugin/log/ErrorPrintInfo.java | 40 --------- .../plugin/log/MessagePartJoiner.java | 30 ++----- .../plugin/log/PrintableErrorCollector.java | 83 ++++++++++--------- 7 files changed, 88 insertions(+), 108 deletions(-) create mode 100644 plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorDisplayInfo.java delete mode 100644 plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorPrintInfo.java diff --git a/common/src/main/java/me/filoghost/holographicdisplays/common/nms/NMSErrors.java b/common/src/main/java/me/filoghost/holographicdisplays/common/nms/NMSErrors.java index 449dbaff..f415ac05 100644 --- a/common/src/main/java/me/filoghost/holographicdisplays/common/nms/NMSErrors.java +++ b/common/src/main/java/me/filoghost/holographicdisplays/common/nms/NMSErrors.java @@ -7,8 +7,7 @@ package me.filoghost.holographicdisplays.common.nms; public class NMSErrors { - public static final String GETTING_ENTITY_ID_GENERATOR_SHORT = "Could not get the NMS entity ID generator."; - public static final String GETTING_ENTITY_ID_GENERATOR_LONG = GETTING_ENTITY_ID_GENERATOR_SHORT + public static final String EXCEPTION_GETTING_ENTITY_ID_GENERATOR = "Could not get the NMS entity ID generator." + " There is a small chance of entity ID conflicts, causing client-side issues on single entities."; } diff --git a/nms/v1_17_r1/src/main/java/me/filoghost/holographicdisplays/nms/v1_17_R1/VersionNMSManager.java b/nms/v1_17_r1/src/main/java/me/filoghost/holographicdisplays/nms/v1_17_R1/VersionNMSManager.java index 5d3d17ee..fad79eb5 100644 --- a/nms/v1_17_r1/src/main/java/me/filoghost/holographicdisplays/nms/v1_17_R1/VersionNMSManager.java +++ b/nms/v1_17_r1/src/main/java/me/filoghost/holographicdisplays/nms/v1_17_R1/VersionNMSManager.java @@ -6,7 +6,6 @@ package me.filoghost.holographicdisplays.nms.v1_17_R1; import me.filoghost.fcommons.logging.ErrorCollector; -import me.filoghost.fcommons.logging.Log; import me.filoghost.fcommons.reflection.ReflectField; import me.filoghost.holographicdisplays.common.nms.EntityID; import me.filoghost.holographicdisplays.common.nms.FallbackEntityIDGenerator; @@ -35,8 +34,7 @@ public class VersionNMSManager implements NMSManager { AtomicInteger nmsEntityIDCounter = ENTITY_ID_COUNTER_FIELD.getStatic(); return nmsEntityIDCounter::incrementAndGet; } catch (ReflectiveOperationException e) { - Log.warning(NMSErrors.GETTING_ENTITY_ID_GENERATOR_SHORT, e); - errorCollector.add(NMSErrors.GETTING_ENTITY_ID_GENERATOR_LONG); + errorCollector.add(e, NMSErrors.EXCEPTION_GETTING_ENTITY_ID_GENERATOR); return new FallbackEntityIDGenerator(); } } diff --git a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/HolographicDisplays.java b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/HolographicDisplays.java index bc2d56bd..79b0835e 100644 --- a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/HolographicDisplays.java +++ b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/HolographicDisplays.java @@ -139,7 +139,7 @@ public class HolographicDisplays extends FCommonsPlugin { if (errorCollector.hasErrors()) { errorCollector.logToConsole(); - Bukkit.getScheduler().runTaskLater(this, errorCollector::logErrorCount, 10L); + Bukkit.getScheduler().runTaskLater(this, errorCollector::logSummaryToConsole, 10L); } } diff --git a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorDisplayInfo.java b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorDisplayInfo.java new file mode 100644 index 00000000..f5f70119 --- /dev/null +++ b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorDisplayInfo.java @@ -0,0 +1,34 @@ +/* + * Copyright (C) filoghost and contributors + * + * SPDX-License-Identifier: GPL-3.0-or-later + */ +package me.filoghost.holographicdisplays.plugin.log; + +import java.util.List; + +class ErrorDisplayInfo { + + private final List messageParts; + private final String details; + private final Throwable exception; + + ErrorDisplayInfo(List messageParts, String details, Throwable exception) { + this.messageParts = messageParts; + this.details = details; + this.exception = exception; + } + + List getMessageParts() { + return messageParts; + } + + String getDetails() { + return details; + } + + Throwable getException() { + return exception; + } + +} diff --git a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorPrintInfo.java b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorPrintInfo.java deleted file mode 100644 index 1a6b320f..00000000 --- a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorPrintInfo.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * Copyright (C) filoghost and contributors - * - * SPDX-License-Identifier: GPL-3.0-or-later - */ -package me.filoghost.holographicdisplays.plugin.log; - -import java.util.List; - -class ErrorPrintInfo { - - private final int index; - private final List message; - private final String details; - private final Throwable cause; - - protected ErrorPrintInfo(int index, List message, String details, Throwable cause) { - this.index = index; - this.message = message; - this.details = details; - this.cause = cause; - } - - public int getIndex() { - return index; - } - - public List getMessage() { - return message; - } - - public String getDetails() { - return details; - } - - public Throwable getCause() { - return cause; - } - -} diff --git a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/MessagePartJoiner.java b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/MessagePartJoiner.java index 0a59de15..5d31d534 100644 --- a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/MessagePartJoiner.java +++ b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/MessagePartJoiner.java @@ -16,31 +16,11 @@ class MessagePartJoiner { private String previousMessagePart; private boolean appendedFirstSentenceSeparator; - public static String join(List messageParts) { - int estimateLength = getEstimateLength(messageParts); - MessagePartJoiner errorMessageBuilder = new MessagePartJoiner(estimateLength); + MessagePartJoiner(List messageParts) { + this.output = new StringBuilder(); for (String messagePart : messageParts) { - errorMessageBuilder.append(messagePart); + append(messagePart); } - return errorMessageBuilder.build(); - } - - private static int getEstimateLength(List messageParts) { - int estimateLength = 0; - - // Length of message parts - for (String messagePart : messageParts) { - estimateLength += messagePart.length(); - } - - // Length of separators in between - estimateLength += (messageParts.size() - 1) * 2; - - return estimateLength; - } - - private MessagePartJoiner(int estimateLength) { - output = new StringBuilder(estimateLength); } private void append(String messagePart) { @@ -76,8 +56,8 @@ class MessagePartJoiner { } } - private String build() { - return output.toString(); + StringBuilder getOutput() { + return output; } } diff --git a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/PrintableErrorCollector.java b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/PrintableErrorCollector.java index 915cdfe4..033bdf62 100644 --- a/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/PrintableErrorCollector.java +++ b/plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/PrintableErrorCollector.java @@ -6,6 +6,7 @@ package me.filoghost.holographicdisplays.plugin.log; import me.filoghost.fcommons.ExceptionUtils; +import me.filoghost.fcommons.Strings; import me.filoghost.fcommons.config.exception.ConfigException; import me.filoghost.fcommons.config.exception.ConfigSyntaxException; import me.filoghost.fcommons.logging.ErrorCollector; @@ -21,7 +22,7 @@ public class PrintableErrorCollector extends ErrorCollector { private static final String ERROR_PREFIX = ChatColor.RED + "[HolographicDisplays] "; - public void logErrorCount() { + public void logSummaryToConsole() { Bukkit.getConsoleSender().sendMessage(ERROR_PREFIX + "Encountered " + getErrorsCount() + " error(s) on load. " + "Check previous console logs for more information."); @@ -29,63 +30,71 @@ public class PrintableErrorCollector extends ErrorCollector { @Override public void logToConsole() { - StringBuilder output = new StringBuilder(); + List outputLines = new ArrayList<>(); if (errors.size() > 0) { - output.append(ERROR_PREFIX).append("Encountered ").append(errors.size()).append(" error(s) on load:\n"); - output.append(" \n"); + outputLines.add(ERROR_PREFIX + "Encountered " + errors.size() + " error(s) on load:"); + outputLines.add(" "); - int index = 1; - for (ErrorLog error : errors) { - ErrorPrintInfo printFormat = getErrorPrintInfo(index, error); - printError(output, printFormat); - index++; + for (int i = 0; i < errors.size(); i++) { + ErrorDisplayInfo errorDisplayInfo = getDisplayInfo(errors.get(i)); + displayError(outputLines, i + 1, errorDisplayInfo); } } - Bukkit.getConsoleSender().sendMessage(output.toString()); + Bukkit.getConsoleSender().sendMessage(String.join(ChatColor.RESET + "\n", outputLines)); } - private ErrorPrintInfo getErrorPrintInfo(int index, ErrorLog error) { - List message = new ArrayList<>(error.getMessage().asList()); + private ErrorDisplayInfo getDisplayInfo(ErrorLog error) { + List messageParts = new ArrayList<>(error.getMessage()); String details = null; - Throwable cause = error.getCause(); + Throwable exception = error.getCause(); - // Recursively inspect the cause until an unknown or null exception is found + // Inspect the exception cause chain until an unknown exception or the last one in the chain while (true) { - if (cause instanceof ConfigSyntaxException) { - message.add(cause.getMessage()); - details = ((ConfigSyntaxException) cause).getSyntaxErrorDetails(); - cause = null; // Do not print stacktrace for syntax exceptions + if (exception instanceof ConfigSyntaxException) { + // Stop inspecting the cause chain, only show details instead of stack traces for syntax exceptions + messageParts.add(exception.getMessage()); + details = ((ConfigSyntaxException) exception).getSyntaxErrorDetails(); + break; - } else if (cause instanceof ConfigException || cause instanceof HologramLoadException) { - message.add(cause.getMessage()); - cause = cause.getCause(); // Print the cause (or nothing if null), not our "known" exception + } else if (exception instanceof ConfigException || exception instanceof HologramLoadException) { + // Known exceptions, add the message and inspect the cause + messageParts.add(exception.getMessage()); + exception = exception.getCause(); } else { - return new ErrorPrintInfo(index, message, details, cause); + // Unknown exception or last one in the chain + break; } } + + return new ErrorDisplayInfo(messageParts, details, exception); } - private static void printError(StringBuilder output, ErrorPrintInfo error) { - output.append(ChatColor.YELLOW).append(error.getIndex()).append(") "); - output.append(ChatColor.WHITE).append(MessagePartJoiner.join(error.getMessage())); + private static void displayError(List outputLines, int index, ErrorDisplayInfo errorDisplayInfo) { + StringBuilder message = new MessagePartJoiner(errorDisplayInfo.getMessageParts()).getOutput(); + if (!Strings.hasSentenceEnding(message.toString())) { + message.append("."); + } + if (errorDisplayInfo.getDetails() != null) { + message.append(" Details:"); + } - if (error.getDetails() != null) { - output.append(". Details:\n"); - output.append(ChatColor.YELLOW).append(error.getDetails()).append("\n"); - } else { - output.append(".\n"); + outputLines.add("" + ChatColor.YELLOW + index + ") " + ChatColor.WHITE + message); + + if (errorDisplayInfo.getDetails() != null) { + outputLines.add(ChatColor.YELLOW + errorDisplayInfo.getDetails()); } - if (error.getCause() != null) { - output.append(ChatColor.DARK_GRAY); - output.append("--------[ Exception details ]--------\n"); - output.append(ExceptionUtils.getStackTraceOutput(error.getCause())); - output.append("-------------------------------------\n"); + + if (errorDisplayInfo.getException() != null) { + outputLines.add(ChatColor.DARK_GRAY + "------------[ Exception details ]------------"); + for (String stackTraceLine : ExceptionUtils.getStackTraceOutputLines(errorDisplayInfo.getException())) { + outputLines.add(ChatColor.DARK_GRAY + stackTraceLine); + } + outputLines.add(ChatColor.DARK_GRAY + "---------------------------------------------"); } - output.append(" \n"); - output.append(ChatColor.RESET); + outputLines.add(" "); } }