Improve error logging

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 EXCEPTION_GETTING_ENTITY_ID_GENERATOR = "Could not get the NMS entity ID generator."
-    public static final String GETTING_ENTITY_ID_GENERATOR_LONG = GETTING_ENTITY_ID_GENERATOR_SHORT
             + " There is a small chance of entity ID conflicts, causing client-side issues on single entities.";
 
 }

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(e, NMSErrors.EXCEPTION_GETTING_ENTITY_ID_GENERATOR);
-            errorCollector.add(NMSErrors.GETTING_ENTITY_ID_GENERATOR_LONG);
             return new FallbackEntityIDGenerator();
         }
     }

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);
         }
     }
 

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<String> messageParts;
+    private final String details;
+    private final Throwable exception;
+
+    ErrorDisplayInfo(List<String> messageParts, String details, Throwable exception) {
+        this.messageParts = messageParts;
+        this.details = details;
+        this.exception = exception;
+    }
+
+    List<String> getMessageParts() {
+        return messageParts;
+    }
+
+    String getDetails() {
+        return details;
+    }
+
+    Throwable getException() {
+        return exception;
+    }
+
+}

plugin/src/main/java/me/filoghost/holographicdisplays/plugin/log/ErrorPrintInfo.java

@@ -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<String> message;
-    private final String details;
-    private final Throwable cause;
-
-    protected ErrorPrintInfo(int index, List<String> message, String details, Throwable cause) {
-        this.index = index;
-        this.message = message;
-        this.details = details;
-        this.cause = cause;
-    }
-
-    public int getIndex() {
-        return index;
-    }
-
-    public List<String> getMessage() {
-        return message;
-    }
-
-    public String getDetails() {
-        return details;
-    }
-
-    public Throwable getCause() {
-        return cause;
-    }
-
-}

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<String> messageParts) {
+    MessagePartJoiner(List<String> messageParts) {
-        int estimateLength = getEstimateLength(messageParts);
+        this.output = new StringBuilder();
-        MessagePartJoiner errorMessageBuilder = new MessagePartJoiner(estimateLength);
         for (String messagePart : messageParts) {
-            errorMessageBuilder.append(messagePart);
+            append(messagePart);
         }
-        return errorMessageBuilder.build();
-    }
-
-    private static int getEstimateLength(List<String> 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() {
+    StringBuilder getOutput() {
-        return output.toString();
+        return output;
     }
 
 }

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<String> outputLines = new ArrayList<>();
 
         if (errors.size() > 0) {
-            output.append(ERROR_PREFIX).append("Encountered ").append(errors.size()).append(" error(s) on load:\n");
+            outputLines.add(ERROR_PREFIX + "Encountered " + errors.size() + " error(s) on load:");
-            output.append(" \n");
+            outputLines.add(" ");
 
-            int index = 1;
+            for (int i = 0; i < errors.size(); i++) {
-            for (ErrorLog error : errors) {
+                ErrorDisplayInfo errorDisplayInfo = getDisplayInfo(errors.get(i));
-                ErrorPrintInfo printFormat = getErrorPrintInfo(index, error);
+                displayError(outputLines, i + 1, errorDisplayInfo);
-                printError(output, printFormat);
-                index++;
             }
         }
 
-        Bukkit.getConsoleSender().sendMessage(output.toString());
+        Bukkit.getConsoleSender().sendMessage(String.join(ChatColor.RESET + "\n", outputLines));
     }
 
-    private ErrorPrintInfo getErrorPrintInfo(int index, ErrorLog error) {
+    private ErrorDisplayInfo getDisplayInfo(ErrorLog error) {
-        List<String> message = new ArrayList<>(error.getMessage().asList());
+        List<String> 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) {
+            if (exception instanceof ConfigSyntaxException) {
-                message.add(cause.getMessage());
+                // Stop inspecting the cause chain, only show details instead of stack traces for syntax exceptions
-                details = ((ConfigSyntaxException) cause).getSyntaxErrorDetails();
+                messageParts.add(exception.getMessage());
-                cause = null; // Do not print stacktrace for syntax exceptions
+                details = ((ConfigSyntaxException) exception).getSyntaxErrorDetails();
+                break;
 
-            } else if (cause instanceof ConfigException || cause instanceof HologramLoadException) {
+            } else if (exception instanceof ConfigException || exception instanceof HologramLoadException) {
-                message.add(cause.getMessage());
+                // Known exceptions, add the message and inspect the cause
-                cause = cause.getCause(); // Print the cause (or nothing if null), not our "known" exception
+                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) {
+    private static void displayError(List<String> outputLines, int index, ErrorDisplayInfo errorDisplayInfo) {
-        output.append(ChatColor.YELLOW).append(error.getIndex()).append(") ");
+        StringBuilder message = new MessagePartJoiner(errorDisplayInfo.getMessageParts()).getOutput();
-        output.append(ChatColor.WHITE).append(MessagePartJoiner.join(error.getMessage()));
+        if (!Strings.hasSentenceEnding(message.toString())) {
+            message.append(".");
+        }
+        if (errorDisplayInfo.getDetails() != null) {
+            message.append(" Details:");
+        }
 
-        if (error.getDetails() != null) {
+        outputLines.add("" + ChatColor.YELLOW + index + ") " + ChatColor.WHITE + message);
-            output.append(". Details:\n");
+
-            output.append(ChatColor.YELLOW).append(error.getDetails()).append("\n");
+        if (errorDisplayInfo.getDetails() != null) {
-        } else {
+            outputLines.add(ChatColor.YELLOW + errorDisplayInfo.getDetails());
-            output.append(".\n");
         }
-        if (error.getCause() != null) {
+
-            output.append(ChatColor.DARK_GRAY);
+        if (errorDisplayInfo.getException() != null) {
-            output.append("--------[ Exception details ]--------\n");
+            outputLines.add(ChatColor.DARK_GRAY + "------------[ Exception details ]------------");
-            output.append(ExceptionUtils.getStackTraceOutput(error.getCause()));
+            for (String stackTraceLine : ExceptionUtils.getStackTraceOutputLines(errorDisplayInfo.getException())) {
-            output.append("-------------------------------------\n");
+                outputLines.add(ChatColor.DARK_GRAY + stackTraceLine);
+            }
+            outputLines.add(ChatColor.DARK_GRAY + "---------------------------------------------");
         }
-        output.append(" \n");
+        outputLines.add(" ");
-        output.append(ChatColor.RESET);
     }
 
 }