From 973a3e10dfc4efa2cde3817cec2b70bdcbb6d3ed Mon Sep 17 00:00:00 2001 From: Asleepp <119438940+Asleeepp@users.noreply.github.com> Date: Wed, 1 Jan 2025 18:24:18 -0300 Subject: [PATCH] Exception message improvements (#7116) --- src/main/java/ch/njol/skript/Skript.java | 226 +++++++++--------- .../ch/njol/skript/update/ReleaseStatus.java | 44 ++-- 2 files changed, 131 insertions(+), 139 deletions(-) diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index b93303ec5b9..a1af697af00 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -118,6 +118,7 @@ import java.nio.file.Paths; import java.nio.file.StandardOpenOption; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -134,6 +135,7 @@ import java.util.logging.Level; import java.util.regex.Matcher; import java.util.regex.Pattern; +import java.util.stream.Collectors; import java.util.zip.ZipEntry; import java.util.zip.ZipException; import java.util.zip.ZipFile; @@ -1644,161 +1646,164 @@ public static void markErrored() { * @param info Description of the error and additional information * @return an EmptyStacktraceException to throw if code execution should terminate. */ - public static EmptyStacktraceException exception(@Nullable Throwable cause, final @Nullable Thread thread, final @Nullable TriggerItem item, final String... info) { + public static EmptyStacktraceException exception(@Nullable Throwable cause, @Nullable Thread thread, @Nullable TriggerItem item, String... info) { errored = true; - // Don't send full exception message again, when caught exception (likely) comes from this method + // Avoid re-throwing the same exception if (cause instanceof EmptyStacktraceException) { return new EmptyStacktraceException(); } // First error: gather plugin package information if (!checkedPlugins) { - for (Plugin plugin : Bukkit.getPluginManager().getPlugins()) { - if (plugin.getName().equals("Skript")) // Don't track myself! - continue; - - PluginDescriptionFile desc = plugin.getDescription(); - if (desc.getDepend().contains("Skript") || desc.getSoftDepend().contains("Skript")) { - // Take actual main class out from the qualified name - String[] parts = desc.getMain().split("\\."); // . is special in regexes... - StringBuilder name = new StringBuilder(desc.getMain().length()); - for (int i = 0; i < parts.length - 1; i++) { - name.append(parts[i]).append('.'); - } + initializePluginPackages(); + checkedPlugins = true; // No need to do this next time + } + + logErrorDetails(cause, info, thread, item); + return new EmptyStacktraceException(); + } + + private static void initializePluginPackages() { + for (Plugin plugin : Bukkit.getPluginManager().getPlugins()) { + if (plugin.getName().equals("Skript")) // Skip self + continue; - // Put this to map - pluginPackages.put(name.toString(), desc); - if (Skript.debug()) - Skript.info("Identified potential addon: " + desc.getFullName() + " (" + name.toString() + ")"); + PluginDescriptionFile desc = plugin.getDescription(); + if (desc.getDepend().contains("Skript") || desc.getSoftDepend().contains("Skript")) { + String mainClassPackage = getPackageName(desc.getMain()); + pluginPackages.put(mainClassPackage, desc); + if (Skript.debug()) { + Skript.info("Identified potential addon: " + desc.getFullName() + " (" + mainClassPackage + ")"); } } - - checkedPlugins = true; // No need to do this next time } + } + private static String getPackageName(String qualifiedClassName) { + int lastDotIndex = qualifiedClassName.lastIndexOf('.'); + return (lastDotIndex == -1) ? "" : qualifiedClassName.substring(0, lastDotIndex); + } + + private static void logErrorDetails(@Nullable Throwable cause, String[] info, @Nullable Thread thread, @Nullable TriggerItem item) { String issuesUrl = "https://github.com/SkriptLang/Skript/issues"; + String downloadUrl = "https://github.com/SkriptLang/Skript/releases/latest"; //TODO grab this from the update checker logEx(); logEx("[Skript] Severe Error:"); logEx(info); logEx(); - // Parse something useful out of the stack trace - StackTraceElement[] stackTrace = Thread.currentThread().getStackTrace(); + Set stackPlugins = identifyPluginsInStackTrace(Thread.currentThread().getStackTrace()); + + logPlatformSupportInfo(issuesUrl, downloadUrl, stackPlugins); + + logEx(); + logEx("Stack trace:"); + logStackTrace(cause); + + logEx(); + logVersionInfo(); + logEx(); + logCurrentState(thread, item); + logEx("End of Error."); + logEx(); + } + + private static Set identifyPluginsInStackTrace(StackTraceElement[] stackTrace) { Set stackPlugins = new HashSet<>(); - for (StackTraceElement s : stackTrace) { // Look through stack trace - for (Entry e : pluginPackages.entrySet()) { // Look through plugins - if (s.getClassName().contains(e.getKey())) // Hey, is this plugin in that stack trace? - stackPlugins.add(e.getValue()); // Yes? Add it to list - } + for (StackTraceElement element : stackTrace) { + pluginPackages.entrySet().stream() + .filter(entry -> element.getClassName().startsWith(entry.getKey())) + .forEach(entry -> stackPlugins.add(entry.getValue())); } + return stackPlugins; + } + + private static void logPlatformSupportInfo(String issuesUrl, String downloadUrl, Set stackPlugins) { SkriptUpdater updater = Skript.getInstance().getUpdater(); - // Check if server platform is supported if (tainted) { - logEx("Skript is running with developer command-line options."); - logEx("If you are not a developer, consider disabling them."); + logEx("Skript is running with developer command-line options. Consider disabling them if not a developer."); } else if (getInstance().getDescription().getVersion().contains("nightly")) { - logEx("You're running a (buggy) nightly version of Skript."); - logEx("If this is not a test server, switch to a more stable release NOW!"); - logEx("Your players are unlikely to appreciate crashes and/or data loss due to Skript bugs."); - logEx(""); - logEx("Just testing things? Good. Please report this bug, so that we can fix it before a stable release."); - logEx("Issue tracker: " + issuesUrl); - } else if (!isRunningMinecraft(1, 9)) { - logEx("You are running an outdated Minecraft version not supported by Skript."); - logEx("Please update to Minecraft 1.9.4 or later or fix this yourself and send us a pull request."); - logEx("Alternatively, use an older Skript version; do note that those are also unsupported by us."); - logEx(""); - logEx("Again, we do not support Minecraft versions this old."); - } else if (!serverPlatform.supported){ - logEx("Your server platform appears to be unsupported by Skript. It might not work reliably."); - logEx("You can report this at " + issuesUrl + ". However, we may be unable to fix the issue."); - logEx("It is recommended that you switch to Paper or Spigot, should you encounter more problems."); + logEx("You're running a (buggy) nightly version of Skript. If this is not a test server, switch to a stable release."); + logEx("Please report this bug to: " + issuesUrl); + } else if (!serverPlatform.supported) { + String supportedPlatforms = getSupportedPlatforms(); + logEx("Your server platform appears to be unsupported by Skript. Consider switching to one of the supported platforms (" + supportedPlatforms + ") for better compatibility."); } else if (updater != null && updater.getReleaseStatus() == ReleaseStatus.OUTDATED) { - logEx("You're running outdated version of Skript! Please try updating it NOW; it might fix this."); - logEx("Run /sk update check to get a download link to latest Skript!"); - logEx("You will be given instructions how to report this error if it persists after update."); + logEx("You're running an outdated version of Skript! Update to the latest version here: " + downloadUrl); } else { - logEx("Something went horribly wrong with Skript."); - logEx("This issue is NOT your fault! You probably can't fix it yourself, either."); - if (pluginPackages.isEmpty()) { - logEx("You should report it at " + issuesUrl + ". Please copy paste this report there (or use paste service)."); - logEx("This ensures that your issue is noticed and will be fixed as soon as possible."); - } else { - logEx("It looks like you are using some plugin(s) that alter how Skript works (addons)."); - if (stackPlugins.isEmpty()) { - logEx("Here is full list of them:"); - StringBuilder pluginsMessage = new StringBuilder(); - for (PluginDescriptionFile desc : pluginPackages.values()) { - pluginsMessage.append(desc.getFullName()); - String website = desc.getWebsite(); - if (website != null && !website.isEmpty()) // Add website if found - pluginsMessage.append(" (").append(desc.getWebsite()).append(")"); - - pluginsMessage.append(" "); - } - logEx(pluginsMessage.toString()); - logEx("We could not identify which of those are specially related, so this might also be Skript issue."); - } else { - logEx("Following plugins are probably related to this error in some way:"); - StringBuilder pluginsMessage = new StringBuilder(); - for (PluginDescriptionFile desc : stackPlugins) { - pluginsMessage.append(desc.getName()); - String website = desc.getWebsite(); - if (website != null && !website.isEmpty()) // Add website if found - pluginsMessage.append(" (").append(desc.getWebsite()).append(")"); - - pluginsMessage.append(" "); - } - logEx(pluginsMessage.toString()); - } + logEx("An unexpected error occurred with Skript. This issue is likely not your fault."); + logExAddonInfo(issuesUrl, stackPlugins); + } + } + + private static String getSupportedPlatforms() { + return Arrays.stream(ServerPlatform.values()) + .filter(platform -> platform.supported) + .map(ServerPlatform::name) + .collect(Collectors.joining(", ")); + } - logEx("You should try disabling those plugins one by one, trying to find which one causes it."); - logEx("If the error doesn't disappear even after disabling all listed plugins, it is probably Skript issue."); - logEx("In that case, you will be given instruction on how should you report it."); - logEx("On the other hand, if the error disappears when disabling some plugin, report it to author of that plugin."); - logEx("Only if the author tells you to do so, report it to Skript's issue tracker."); + private static void logExAddonInfo(String issuesUrl, Set stackPlugins) { + if (pluginPackages.isEmpty()) { + logEx("Report the issue: " + issuesUrl); + } else { + logEx("You are using some plugins that alter how Skript works (addons)."); + if (stackPlugins.isEmpty()) { + logEx("Full list of addons:"); + pluginPackages.values().forEach(desc -> logEx(getPluginDescription(desc))); + logEx("We could not identify related addons, it might also be a Skript issue."); + } else { + logEx("The following plugins are likely related to this error:"); + stackPlugins.forEach(desc -> logEx(getPluginDescription(desc))); } + logEx("Try temporarily removing the listed plugins one by one to identify the cause."); + logEx("If removing a plugin resolves the issue, please report the problem to the plugin developer."); } + } - logEx(); - logEx("Stack trace:"); + private static String getPluginDescription(PluginDescriptionFile desc) { + String website = desc.getWebsite(); + return desc.getFullName() + (website != null && !website.isEmpty() ? " (" + website + ")" : ""); + } + + private static void logStackTrace(@Nullable Throwable cause) { if (cause == null || cause.getStackTrace().length == 0) { - logEx(" warning: no/empty exception given, dumping current stack trace instead"); - cause = new Exception(cause); + logEx("Warning: no/empty exception given, dumping current stack trace instead"); + cause = new Exception("EmptyStacktraceException cause"); } - boolean first = true; while (cause != null) { - logEx((first ? "" : "Caused by: ") + cause.toString()); - for (final StackTraceElement e : cause.getStackTrace()) - logEx(" at " + e.toString()); + logEx((cause == null ? "" : "Caused by: ") + cause.toString()); + for (StackTraceElement element : cause.getStackTrace()) { + logEx(" at " + element.toString()); + } cause = cause.getCause(); - first = false; } + } - logEx(); - logEx("Version Information:"); + private static void logVersionInfo() { + SkriptUpdater updater = Skript.getInstance().getUpdater(); if (updater != null) { ReleaseStatus status = updater.getReleaseStatus(); - logEx(" Skript: " + getVersion() + (status == ReleaseStatus.LATEST ? " (latest)" - : status == ReleaseStatus.OUTDATED ? " (OUTDATED)" - : status == ReleaseStatus.CUSTOM ? " (custom version)" : "")); + logEx("Skript: " + getVersion() + " (" + status.toString() + ")"); ReleaseManifest current = updater.getCurrentRelease(); logEx(" Flavor: " + current.flavor); logEx(" Date: " + current.date); } else { - logEx(" Skript: " + getVersion() + " (unknown; likely custom)"); + logEx("Skript: " + getVersion() + " (unknown; likely custom)"); } - logEx(" Bukkit: " + Bukkit.getBukkitVersion()); - logEx(" Minecraft: " + getMinecraftVersion()); - logEx(" Java: " + System.getProperty("java.version") + " (" + System.getProperty("java.vm.name") + " " + System.getProperty("java.vm.version") + ")"); - logEx(" OS: " + System.getProperty("os.name") + " " + System.getProperty("os.arch") + " " + System.getProperty("os.version")); + logEx("Bukkit: " + Bukkit.getBukkitVersion()); + logEx("Minecraft: " + getMinecraftVersion()); + logEx("Java: " + System.getProperty("java.version") + " (" + System.getProperty("java.vm.name") + " " + System.getProperty("java.vm.version") + ")"); + logEx("OS: " + System.getProperty("os.name") + " " + System.getProperty("os.arch") + " " + System.getProperty("os.version")); logEx(); logEx("Server platform: " + serverPlatform.name + (serverPlatform.supported ? "" : " (unsupported)")); - logEx(); + } + + private static void logCurrentState(@Nullable Thread thread, @Nullable TriggerItem item) { logEx("Current node: " + SkriptLogger.getNode()); logEx("Current item: " + (item == null ? "null" : item.toString(null, true))); if (item != null && item.getTrigger() != null) { @@ -1806,16 +1811,9 @@ public static EmptyStacktraceException exception(@Nullable Throwable cause, fina Script script = trigger.getScript(); logEx("Current trigger: " + trigger.toString(null, true) + " (" + (script == null ? "null" : script.getConfig().getFileName()) + ", line " + trigger.getLineNumber() + ")"); } - logEx(); logEx("Thread: " + (thread == null ? Thread.currentThread() : thread).getName()); - logEx(); logEx("Language: " + Language.getName()); logEx("Link parse mode: " + ChatMessages.linkParseMode); - logEx(); - logEx("End of Error."); - logEx(); - - return new EmptyStacktraceException(); } static void logEx() { diff --git a/src/main/java/ch/njol/skript/update/ReleaseStatus.java b/src/main/java/ch/njol/skript/update/ReleaseStatus.java index 72838333a35..aad575b3491 100644 --- a/src/main/java/ch/njol/skript/update/ReleaseStatus.java +++ b/src/main/java/ch/njol/skript/update/ReleaseStatus.java @@ -1,21 +1,3 @@ -/** - * This file is part of Skript. - * - * Skript is free software: you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Skript is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with Skript. If not, see . - * - * Copyright Peter Güttinger, SkriptLang team and contributors - */ package ch.njol.skript.update; @@ -23,30 +5,42 @@ * Status of currently installed release. */ public enum ReleaseStatus { - + /** * Latest release in channel. This is a good thing. */ - LATEST, + LATEST("latest"), /** * Old, probably unsupported release. */ - OUTDATED, + OUTDATED("outdated"), /** - * Updates have not been checked, so it not known if any exist. + * Updates have not been checked, so it is not known if any exist. */ - UNKNOWN, + UNKNOWN("unknown"), /** * Updates have been checked, but this release was not found at all. * It might be not yet published. */ - CUSTOM, + CUSTOM("custom"), /** * Running a developer/nightly build, updates will not be checked. */ - DEVELOPMENT + DEVELOPMENT("development"); + + private final String name; + + ReleaseStatus(String name) { + this.name = name; + } + + @Override + public String toString() { + return name; + } + }