diff --git a/bukkit/example-plugin/src/main/java/com/example/ExamplePlugin.java b/bukkit/example-plugin/src/main/java/com/example/ExamplePlugin.java index ec58beb..28f91ea 100644 --- a/bukkit/example-plugin/src/main/java/com/example/ExamplePlugin.java +++ b/bukkit/example-plugin/src/main/java/com/example/ExamplePlugin.java @@ -2,7 +2,6 @@ import dev.faststats.bukkit.BukkitMetrics; import dev.faststats.core.ErrorTracker; -import dev.faststats.core.Metrics; import dev.faststats.core.chart.Chart; import org.bukkit.plugin.java.JavaPlugin; @@ -15,7 +14,7 @@ public class ExamplePlugin extends JavaPlugin { // context-unaware error tracker, does not automatically track errors public static final ErrorTracker CONTEXT_UNAWARE_ERROR_TRACKER = ErrorTracker.contextUnaware(); - private final Metrics metrics = BukkitMetrics.factory() + private final BukkitMetrics metrics = BukkitMetrics.factory() .url(URI.create("https://metrics.example.com/v1/collect")) // For self-hosted metrics servers only // Custom example charts @@ -36,9 +35,14 @@ public class ExamplePlugin extends JavaPlugin { .token("YOUR_TOKEN_HERE") // required -> token can be found in the settings of your project .create(this); + @Override + public void onEnable() { + metrics.ready(); // register additional error handlers + } + @Override public void onDisable() { - metrics.shutdown(); + metrics.shutdown(); // safely shut down metrics submission } public void doSomethingWrong() { diff --git a/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetrics.java b/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetrics.java index ea1bae6..810cca6 100644 --- a/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetrics.java +++ b/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetrics.java @@ -1,6 +1,7 @@ package dev.faststats.bukkit; import dev.faststats.core.Metrics; +import org.bukkit.plugin.IllegalPluginAccessException; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.Contract; @@ -21,6 +22,18 @@ static Factory factory() { return new BukkitMetricsImpl.Factory(); } + /** + * Registers additional exception handlers on Paper-based implementations. + * + * @throws IllegalPluginAccessException if the plugin is not yet enabled + * @apiNote This method may only be called {@link Plugin#onEnable() onEnable()}. + * @since 0.14.0 + */ + @Override + void ready() throws IllegalPluginAccessException; + interface Factory extends Metrics.Factory { + @Override + BukkitMetrics create(Plugin object) throws IllegalStateException; } } diff --git a/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetricsImpl.java b/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetricsImpl.java index ec8eda3..88f95d1 100644 --- a/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetricsImpl.java +++ b/bukkit/src/main/java/dev/faststats/bukkit/BukkitMetricsImpl.java @@ -1,9 +1,7 @@ package dev.faststats.bukkit; import com.google.gson.JsonObject; -import dev.faststats.core.Metrics; import dev.faststats.core.SimpleMetrics; -import org.bukkit.Server; import org.bukkit.plugin.Plugin; import org.jetbrains.annotations.Async; import org.jetbrains.annotations.Contract; @@ -13,11 +11,9 @@ import java.util.Optional; import java.util.function.Supplier; import java.util.logging.Level; -import java.util.logging.Logger; final class BukkitMetricsImpl extends SimpleMetrics implements BukkitMetrics { - private final Logger logger; - private final Server server; + private final Plugin plugin; private final String pluginVersion; private final String minecraftVersion; @@ -29,8 +25,8 @@ final class BukkitMetricsImpl extends SimpleMetrics implements BukkitMetrics { private BukkitMetricsImpl(final Factory factory, final Plugin plugin, final Path config) throws IllegalStateException { super(factory, config); - this.logger = plugin.getLogger(); - this.server = plugin.getServer(); + this.plugin = plugin; + var server = plugin.getServer(); this.pluginVersion = tryOrEmpty(() -> plugin.getPluginMeta().getVersion()) .orElseGet(() -> plugin.getDescription().getVersion()); @@ -42,7 +38,12 @@ private BukkitMetricsImpl(final Factory factory, final Plugin plugin, final Path startSubmitting(); } + Plugin plugin() { + return plugin; + } + private boolean checkOnlineMode() { + var server = plugin.getServer(); return tryOrEmpty(() -> server.getServerConfig().isProxyOnlineMode()) .or(() -> tryOrEmpty(this::isProxyOnlineMode)) .orElseGet(server::getOnlineMode); @@ -50,6 +51,7 @@ private boolean checkOnlineMode() { @SuppressWarnings("removal") private boolean isProxyOnlineMode() { + var server = plugin.getServer(); final var proxies = server.spigot().getPaperConfig().getConfigurationSection("proxies"); if (proxies == null) return false; @@ -72,7 +74,7 @@ protected void appendDefaultData(final JsonObject charts) { private int getPlayerCount() { try { - return server.getOnlinePlayers().size(); + return plugin.getServer().getOnlinePlayers().size(); } catch (final Throwable t) { error("Failed to get player count", t); return 0; @@ -81,17 +83,26 @@ private int getPlayerCount() { @Override protected void printError(final String message, @Nullable final Throwable throwable) { - logger.log(Level.SEVERE, message, throwable); + plugin.getLogger().log(Level.SEVERE, message, throwable); } @Override protected void printInfo(final String message) { - logger.info(message); + plugin.getLogger().info(message); } @Override protected void printWarning(final String message) { - logger.warning(message); + plugin.getLogger().warning(message); + } + + @Override + public void ready() { + if (getErrorTracker().isPresent()) try { + Class.forName("com.destroystokyo.paper.event.server.ServerExceptionEvent"); + plugin.getServer().getPluginManager().registerEvents(new PaperEventListener(this), plugin); + } catch (final ClassNotFoundException ignored) { + } } private Optional tryOrEmpty(final Supplier supplier) { @@ -104,7 +115,7 @@ private Optional tryOrEmpty(final Supplier supplier) { static final class Factory extends SimpleMetrics.Factory implements BukkitMetrics.Factory { @Override - public Metrics create(final Plugin plugin) throws IllegalStateException { + public BukkitMetrics create(final Plugin plugin) throws IllegalStateException { final var dataFolder = getPluginsFolder(plugin).resolve("faststats"); final var config = dataFolder.resolve("config.properties"); return new BukkitMetricsImpl(this, plugin, config); diff --git a/bukkit/src/main/java/dev/faststats/bukkit/PaperEventListener.java b/bukkit/src/main/java/dev/faststats/bukkit/PaperEventListener.java new file mode 100644 index 0000000..a170520 --- /dev/null +++ b/bukkit/src/main/java/dev/faststats/bukkit/PaperEventListener.java @@ -0,0 +1,16 @@ +package dev.faststats.bukkit; + +import com.destroystokyo.paper.event.server.ServerExceptionEvent; +import com.destroystokyo.paper.exception.ServerPluginException; +import org.bukkit.event.EventHandler; +import org.bukkit.event.EventPriority; +import org.bukkit.event.Listener; + +record PaperEventListener(BukkitMetricsImpl metrics) implements Listener { + @EventHandler(priority = EventPriority.MONITOR) + public void onServerException(final ServerExceptionEvent event) { + if (!(event.getException() instanceof final ServerPluginException exception)) return; + if (!exception.getResponsiblePlugin().equals(metrics.plugin())) return; + metrics.getErrorTracker().ifPresent(tracker -> tracker.trackError(exception)); + } +} diff --git a/bungeecord/example-plugin/src/main/java/com/example/ExamplePlugin.java b/bungeecord/example-plugin/src/main/java/com/example/ExamplePlugin.java index c0999e8..ffb3a1e 100644 --- a/bungeecord/example-plugin/src/main/java/com/example/ExamplePlugin.java +++ b/bungeecord/example-plugin/src/main/java/com/example/ExamplePlugin.java @@ -38,7 +38,7 @@ public class ExamplePlugin extends Plugin { @Override public void onDisable() { - metrics.shutdown(); + metrics.shutdown(); // safely shut down metrics submission } public void doSomethingWrong() { diff --git a/core/src/main/java/dev/faststats/core/ErrorHelper.java b/core/src/main/java/dev/faststats/core/ErrorHelper.java index eeb6927..988005d 100644 --- a/core/src/main/java/dev/faststats/core/ErrorHelper.java +++ b/core/src/main/java/dev/faststats/core/ErrorHelper.java @@ -6,7 +6,10 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; +import java.util.IdentityHashMap; import java.util.List; +import java.util.Set; final class ErrorHelper { private static final int MESSAGE_LENGTH = Math.min(1000, Integer.getInteger("faststats.message-length", 500)); @@ -104,18 +107,25 @@ private static List collapseConsecutiveDuplicates(final List lin } public static boolean isSameLoader(final ClassLoader loader, final Throwable error) { + return isSameLoader(loader, error, Collections.newSetFromMap(new IdentityHashMap<>())); + } + + private static boolean isSameLoader(final ClassLoader loader, @Nullable final Throwable error, final Set visited) { + if (error == null || !visited.add(error)) return false; + final var stackTrace = error.getStackTrace(); - if (stackTrace == null || stackTrace.length == 0) return false; + if (stackTrace == null || stackTrace.length == 0) + return isSameLoader(loader, error.getCause(), visited); final var firstNonLibraryIndex = findFirstNonLibraryFrameIndex(stackTrace); - if (firstNonLibraryIndex == -1) return false; + if (firstNonLibraryIndex == -1) return isSameLoader(loader, error.getCause(), visited); final var framesToCheck = Math.min(5, stackTrace.length - firstNonLibraryIndex); for (var i = 0; i < framesToCheck; i++) { final var frame = stackTrace[firstNonLibraryIndex + i]; if (isLibraryClass(frame.getClassName())) continue; - if (!isFromLoader(frame, loader)) return false; + if (!isFromLoader(frame, loader)) return isSameLoader(loader, error.getCause(), visited); } return true; diff --git a/core/src/main/java/dev/faststats/core/ErrorTracker.java b/core/src/main/java/dev/faststats/core/ErrorTracker.java index a5c419c..142475b 100644 --- a/core/src/main/java/dev/faststats/core/ErrorTracker.java +++ b/core/src/main/java/dev/faststats/core/ErrorTracker.java @@ -163,4 +163,17 @@ static ErrorTracker contextUnaware() { */ @Contract(pure = true) TrackingThreadPoolExecutor threadPoolExecutor(); + + /** + * Checks if the error occurred in the same class loader as the provided loader. + * + * @param loader the class loader + * @param error the error + * @return whether the error occurred in the same class loader + * @since 0.14.0 + */ + @Contract(pure = true) + static boolean isSameLoader(ClassLoader loader, Throwable error) { + return ErrorHelper.isSameLoader(loader, error); + } } diff --git a/core/src/main/java/dev/faststats/core/Metrics.java b/core/src/main/java/dev/faststats/core/Metrics.java index a0d6080..bc90c01 100644 --- a/core/src/main/java/dev/faststats/core/Metrics.java +++ b/core/src/main/java/dev/faststats/core/Metrics.java @@ -43,7 +43,19 @@ public interface Metrics { Config getConfig(); /** - * Shuts down the metrics submission. + * Performs additional post-startup tasks. + *

+ * This method may only be called when the application startup is complete. + *

+ * No-op in most implementations. + * + * @since 0.14.0 + */ + default void ready() { + } + + /** + * Safely shuts down the metrics submission. *

* This method should be called when the application is shutting down. * diff --git a/core/src/main/java/dev/faststats/core/SimpleErrorTracker.java b/core/src/main/java/dev/faststats/core/SimpleErrorTracker.java index 9e0dd22..8ebdd06 100644 --- a/core/src/main/java/dev/faststats/core/SimpleErrorTracker.java +++ b/core/src/main/java/dev/faststats/core/SimpleErrorTracker.java @@ -89,7 +89,7 @@ public synchronized void attachErrorContext(@Nullable final ClassLoader loader) final var handler = originalHandler; if (handler != null) handler.uncaughtException(thread, error); try { - if (loader != null && !ErrorHelper.isSameLoader(loader, error)) return; + if (loader != null && !ErrorTracker.isSameLoader(loader, error)) return; final var event = errorEvent; if (event != null) event.accept(loader, error); trackError(error); diff --git a/core/src/test/java/dev/faststats/ErrorTrackerTest.java b/core/src/test/java/dev/faststats/ErrorTrackerTest.java index b9684e5..9b6a098 100644 --- a/core/src/test/java/dev/faststats/ErrorTrackerTest.java +++ b/core/src/test/java/dev/faststats/ErrorTrackerTest.java @@ -3,14 +3,130 @@ import dev.faststats.core.ErrorTracker; import org.junit.jupiter.api.Test; +import java.net.URL; +import java.net.URLClassLoader; import java.time.Duration; import java.util.concurrent.CompletionException; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + public class ErrorTrackerTest { // todo: add redaction tests // todo: add nesting tests // todo: add duplicate tests + @Test + public void sameClassLoader() { + final var loader = getClass().getClassLoader(); + final var error = new RuntimeException("test"); + assertTrue(ErrorTracker.isSameLoader(loader, error)); + } + + @Test + public void childLoaderMatchesParentLoader() { + final var parentLoader = getClass().getClassLoader(); + final var childLoader = new URLClassLoader(new URL[0], parentLoader); + + final var errorFromParent = new RuntimeException("test from parent"); + assertTrue(ErrorTracker.isSameLoader(parentLoader, errorFromParent)); + assertFalse(ErrorTracker.isSameLoader(childLoader, errorFromParent)); + } + + @Test + public void differentClassLoader() { + final var isolatedLoader = new URLClassLoader(new URL[0], null); + final var error = new RuntimeException("test"); + + assertFalse(ErrorTracker.isSameLoader(isolatedLoader, error)); + } + + @Test + public void classLoaderHierarchyMatching() { + final var mainLoader = getClass().getClassLoader(); + final var submissionsLoader = new URLClassLoader(new URL[0], mainLoader); + final var virtualLoader = new URLClassLoader(new URL[0], mainLoader); + final var netLoader = new URLClassLoader(new URL[0], submissionsLoader); + + final var errorFromMain = new RuntimeException("from main"); + + assertTrue(ErrorTracker.isSameLoader(mainLoader, errorFromMain)); + assertFalse(ErrorTracker.isSameLoader(submissionsLoader, errorFromMain)); + assertFalse(ErrorTracker.isSameLoader(virtualLoader, errorFromMain)); + assertFalse(ErrorTracker.isSameLoader(netLoader, errorFromMain)); + + final var isolatedLoader = new URLClassLoader(new URL[0], null); + assertFalse(ErrorTracker.isSameLoader(isolatedLoader, errorFromMain)); + } + + @Test + public void siblingLoadersDoNotMatch() { + final var mainLoader = getClass().getClassLoader(); + final var submissionsLoader = new URLClassLoader(new URL[0], mainLoader); + final var virtualLoader = new URLClassLoader(new URL[0], mainLoader); + final var netLoader = new URLClassLoader(new URL[0], submissionsLoader); + + final var errorFromSubmissions = createErrorWithStackFrom("submissions.Plugin"); + final var errorFromVirtual = createErrorWithStackFrom("virtual.Handler"); + final var errorFromNet = createErrorWithStackFrom("net.Connection"); + + assertFalse(ErrorTracker.isSameLoader(virtualLoader, errorFromSubmissions)); + assertFalse(ErrorTracker.isSameLoader(submissionsLoader, errorFromVirtual)); + assertFalse(ErrorTracker.isSameLoader(virtualLoader, errorFromNet)); + } + + private RuntimeException createErrorWithStackFrom(final String className) { + final var error = new RuntimeException("test"); + error.setStackTrace(new StackTraceElement[]{ + new StackTraceElement(className, "test", "Test.java", 1) + }); + return error; + } + + @Test + public void nestedCauseSameLoader() { + final var loader = getClass().getClassLoader(); + final var cause = new IllegalArgumentException("cause"); + final var error = new RuntimeException("wrapper", cause); + + assertTrue(ErrorTracker.isSameLoader(loader, error)); + } + + @Test + public void emptyStackTrace() { + final var loader = getClass().getClassLoader(); + final var error = new RuntimeException("no stack"); + error.setStackTrace(new StackTraceElement[0]); + + assertFalse(ErrorTracker.isSameLoader(loader, error)); + } + + @Test + public void emptyStackTraceChecksCause() { + final var loader = getClass().getClassLoader(); + final var cause = createExceptionWithStack(); + final var error = new RuntimeException("no stack", cause); + error.setStackTrace(new StackTraceElement[0]); + + assertTrue(ErrorTracker.isSameLoader(loader, error)); + } + + @Test + public void libraryOnlyStackFallsThroughToCause() { + final var loader = getClass().getClassLoader(); + final var cause = createExceptionWithStack(); + final var error = new RuntimeException("library only", cause); + error.setStackTrace(new StackTraceElement[]{ + new StackTraceElement("java.lang.String", "valueOf", "String.java", 100) + }); + + assertTrue(ErrorTracker.isSameLoader(loader, error)); + } + + private IllegalArgumentException createExceptionWithStack() { + return new IllegalArgumentException("cause with stack"); + } + @Test // todo: fix this mess public void testCompile() throws InterruptedException { diff --git a/core/src/test/java/dev/faststats/MockMetrics.java b/core/src/test/java/dev/faststats/MockMetrics.java index d638288..ed8008c 100644 --- a/core/src/test/java/dev/faststats/MockMetrics.java +++ b/core/src/test/java/dev/faststats/MockMetrics.java @@ -7,7 +7,6 @@ import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; -import java.io.IOException; import java.net.URI; import java.util.Set; import java.util.UUID; diff --git a/gradle.properties b/gradle.properties index aa31273..3cbaf2a 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1 +1 @@ -version=0.13.1 +version=0.14.0 diff --git a/hytale/example-plugin/src/main/java/com/example/ExamplePlugin.java b/hytale/example-plugin/src/main/java/com/example/ExamplePlugin.java index 74ec675..b153fa4 100644 --- a/hytale/example-plugin/src/main/java/com/example/ExamplePlugin.java +++ b/hytale/example-plugin/src/main/java/com/example/ExamplePlugin.java @@ -43,7 +43,7 @@ public ExamplePlugin(final JavaPluginInit init) { @Override protected void shutdown() { - metrics.shutdown(); + metrics.shutdown(); // safely shut down metrics submission } public void doSomethingWrong() { diff --git a/minestom/src/main/java/dev/faststats/minestom/MinestomMetrics.java b/minestom/src/main/java/dev/faststats/minestom/MinestomMetrics.java index 0e55387..f4df6b9 100644 --- a/minestom/src/main/java/dev/faststats/minestom/MinestomMetrics.java +++ b/minestom/src/main/java/dev/faststats/minestom/MinestomMetrics.java @@ -1,6 +1,7 @@ package dev.faststats.minestom; import dev.faststats.core.Metrics; +import net.minestom.server.Auth; import net.minestom.server.MinecraftServer; import org.jetbrains.annotations.Contract; @@ -21,6 +22,15 @@ static Factory factory() { return new MinestomMetricsImpl.Factory(); } + /** + * Registers additional exception handlers. + * + * @apiNote This method may only be called after {@link MinecraftServer#init(Auth)}. + * @since 0.14.0 + */ + @Override + void ready(); + interface Factory extends Metrics.Factory { } } diff --git a/minestom/src/main/java/dev/faststats/minestom/MinestomMetricsImpl.java b/minestom/src/main/java/dev/faststats/minestom/MinestomMetricsImpl.java index d5594df..279a67b 100644 --- a/minestom/src/main/java/dev/faststats/minestom/MinestomMetricsImpl.java +++ b/minestom/src/main/java/dev/faststats/minestom/MinestomMetricsImpl.java @@ -1,6 +1,7 @@ package dev.faststats.minestom; import com.google.gson.JsonObject; +import dev.faststats.core.ErrorTracker; import dev.faststats.core.Metrics; import dev.faststats.core.SimpleMetrics; import net.minestom.server.Auth; @@ -47,6 +48,20 @@ protected void printWarning(final String message) { logger.warn(message); } + @Override + public void ready() { + getErrorTracker().ifPresent(this::registerExceptionHandler); + } + + private void registerExceptionHandler(ErrorTracker errorTracker) { + var handler = MinecraftServer.getExceptionManager().getExceptionHandler(); + MinecraftServer.getExceptionManager().setExceptionHandler(error -> { + handler.handleException(error); + if (!ErrorTracker.isSameLoader(getClass().getClassLoader(), error)) return; + errorTracker.trackError(error); + }); + } + static final class Factory extends SimpleMetrics.Factory implements MinestomMetrics.Factory { @Override public Metrics create(final MinecraftServer server) throws IllegalStateException { diff --git a/sponge/example-plugin/src/main/java/com/example/ExamplePlugin.java b/sponge/example-plugin/src/main/java/com/example/ExamplePlugin.java index d100584..650696a 100644 --- a/sponge/example-plugin/src/main/java/com/example/ExamplePlugin.java +++ b/sponge/example-plugin/src/main/java/com/example/ExamplePlugin.java @@ -55,7 +55,7 @@ public void onServerStart(final StartedEngineEvent event) { @Listener public void onServerStop(final StoppingEngineEvent event) { - if (metrics != null) metrics.shutdown(); + if (metrics != null) metrics.shutdown(); // safely shut down metrics submission } public void doSomethingWrong() { diff --git a/velocity/example-plugin/src/main/java/com/example/ExamplePlugin.java b/velocity/example-plugin/src/main/java/com/example/ExamplePlugin.java index e163a2e..94f49e8 100644 --- a/velocity/example-plugin/src/main/java/com/example/ExamplePlugin.java +++ b/velocity/example-plugin/src/main/java/com/example/ExamplePlugin.java @@ -57,7 +57,7 @@ public void onProxyInitialize(final ProxyInitializeEvent event) { @Subscribe public void onProxyStop(final ProxyShutdownEvent event) { - if (metrics != null) metrics.shutdown(); + if (metrics != null) metrics.shutdown(); // safely shut down metrics submission } public void doSomethingWrong() {