Exception message improvements (#7116)

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,178 +1646,174 @@ 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<PluginDescriptionFile> 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<PluginDescriptionFile> identifyPluginsInStackTrace(StackTraceElement[] stackTrace) {
 		Set<PluginDescriptionFile> stackPlugins = new HashSet<>();
-		for (StackTraceElement s : stackTrace) { // Look through stack trace
-			for (Entry<String,PluginDescriptionFile> 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<PluginDescriptionFile> 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<PluginDescriptionFile> 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) {
 			Trigger trigger = item.getTrigger();
 			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() {

src/main/java/ch/njol/skript/update/ReleaseStatus.java

@@ -1,52 +1,46 @@
-/**
- *   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 <http://www.gnu.org/licenses/>.
- *
- * Copyright Peter Güttinger, SkriptLang team and contributors
- */
 package ch.njol.skript.update;
 
 
 /**
  * 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;
+	}
+
 }