From cecf80250cf4edebdb088275d3e4a4973d8dd564 Mon Sep 17 00:00:00 2001 From: Kristian Date: Sat, 27 Apr 2013 02:24:49 +0200 Subject: [PATCH] Don't print pointless warnings on Spigot. --- .../comphenix/protocol/ProtocolLibrary.java | 9 +- .../protocol/error/BasicErrorReporter.java | 96 +++++++++++++++++++ .../error/DelegatedErrorReporter.java | 80 ++++++++++++++++ .../protocol/error/DetailedErrorReporter.java | 1 - .../injector/player/InjectedArrayList.java | 12 +-- .../injector/player/PlayerInjector.java | 2 +- .../injector/spigot/SpigotPacketInjector.java | 25 ++++- .../reflect/compiler/BackgroundCompiler.java | 17 ++-- 8 files changed, 216 insertions(+), 26 deletions(-) create mode 100644 ProtocolLib/src/main/java/com/comphenix/protocol/error/BasicErrorReporter.java create mode 100644 ProtocolLib/src/main/java/com/comphenix/protocol/error/DelegatedErrorReporter.java diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java index 8fead494..6d4eb893 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/ProtocolLibrary.java @@ -33,6 +33,7 @@ import org.bukkit.plugin.PluginManager; import org.bukkit.plugin.java.JavaPlugin; import com.comphenix.protocol.async.AsyncFilterManager; +import com.comphenix.protocol.error.BasicErrorReporter; import com.comphenix.protocol.error.DetailedErrorReporter; import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.error.Report; @@ -92,7 +93,7 @@ public class ProtocolLibrary extends JavaPlugin { private static PacketFilterManager protocolManager; // Error reporter - private static ErrorReporter reporter; + private static ErrorReporter reporter = new BasicErrorReporter(); // Metrics and statistisc private Statistics statistisc; @@ -490,7 +491,9 @@ public class ProtocolLibrary extends JavaPlugin { unhookTask.close(); protocolManager = null; statistisc = null; - reporter = null; + + // To clean up global parameters + reporter = new BasicErrorReporter(); // Leaky ClassLoader begone! if (updater == null || updater.getResult() != UpdateResult.SUCCESS) { @@ -515,6 +518,8 @@ public class ProtocolLibrary extends JavaPlugin { /** * Retrieve the current error reporter. + *

+ * This is guaranteed to not be NULL in 2.5.0 and later. * @return Current error reporter. */ public static ErrorReporter getErrorReporter() { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/error/BasicErrorReporter.java b/ProtocolLib/src/main/java/com/comphenix/protocol/error/BasicErrorReporter.java new file mode 100644 index 00000000..69226d57 --- /dev/null +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/error/BasicErrorReporter.java @@ -0,0 +1,96 @@ +package com.comphenix.protocol.error; + +import java.io.PrintStream; +import org.bukkit.plugin.Plugin; + +import com.comphenix.protocol.error.Report.ReportBuilder; +import com.comphenix.protocol.reflect.PrettyPrinter; + +/** + * Represents a basic error reporter that prints error reports to the standard error stream. + *

+ * Note that this implementation doesn't distinguish between {@link #reportWarning(Object, Report)} + * and {@link #reportDetailed(Object, Report)} - they both have the exact same behavior. + * @author Kristian + */ +public class BasicErrorReporter implements ErrorReporter { + private final PrintStream output; + + /** + * Construct a new basic error reporter that prints directly the standard error stream. + */ + public BasicErrorReporter() { + this(System.err); + } + + /** + * Construct a error reporter that prints to the given output stream. + * @param output - the output stream. + */ + public BasicErrorReporter(PrintStream output) { + this.output = output; + } + + @Override + public void reportMinimal(Plugin sender, String methodName, Throwable error) { + output.println("Unhandled exception occured in " + methodName + " for " + sender.getName()); + error.printStackTrace(output); + } + + @Override + public void reportMinimal(Plugin sender, String methodName, Throwable error, Object... parameters) { + reportMinimal(sender, methodName, error); + + // Also print parameters + printParameters(parameters); + } + + @Override + public void reportWarning(Object sender, Report report) { + // Basic warning + output.println("[" + sender.getClass().getSimpleName() + "] " + report.getReportMessage()); + + if (report.getException() != null) { + report.getException().printStackTrace(output); + } + printParameters(report.getCallerParameters()); + } + + @Override + public void reportWarning(Object sender, ReportBuilder reportBuilder) { + reportWarning(sender, reportBuilder.build()); + } + + @Override + public void reportDetailed(Object sender, Report report) { + // No difference from warning + reportWarning(sender, report); + } + + @Override + public void reportDetailed(Object sender, ReportBuilder reportBuilder) { + reportWarning(sender, reportBuilder); + } + + /** + * Print the given parameters to the standard error stream. + * @param parameters - the output parameters. + */ + private void printParameters(Object[] parameters) { + if (parameters != null && parameters.length > 0) { + output.println("Parameters: "); + + try { + for (Object parameter : parameters) { + if (parameter == null) + output.println("[NULL]"); + else + output.println(PrettyPrinter.printObject(parameter)); + } + } catch (IllegalAccessException e) { + // Damn it + e.printStackTrace(); + } + } + } +} diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/error/DelegatedErrorReporter.java b/ProtocolLib/src/main/java/com/comphenix/protocol/error/DelegatedErrorReporter.java new file mode 100644 index 00000000..7584f5d0 --- /dev/null +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/error/DelegatedErrorReporter.java @@ -0,0 +1,80 @@ +package com.comphenix.protocol.error; + +import org.bukkit.plugin.Plugin; + +import com.comphenix.protocol.error.Report.ReportBuilder; + +/** + * Construct an error reporter that delegates to another error reporter. + * @author Kristian + */ +public class DelegatedErrorReporter implements ErrorReporter { + private final ErrorReporter delegated; + + /** + * Construct a new error reporter that forwards all reports to a given reporter. + * @param delegated - the delegated reporter. + */ + public DelegatedErrorReporter(ErrorReporter delegated) { + this.delegated = delegated; + } + + /** + * Retrieve the underlying error reporter. + * @return Underlying error reporter. + */ + public ErrorReporter getDelegated() { + return delegated; + } + + @Override + public void reportMinimal(Plugin sender, String methodName, Throwable error) { + delegated.reportMinimal(sender, methodName, error); + } + + @Override + public void reportMinimal(Plugin sender, String methodName, Throwable error, Object... parameters) { + delegated.reportMinimal(sender, methodName, error, parameters); + } + + @Override + public void reportWarning(Object sender, Report report) { + Report transformed = filterReport(sender, report, false); + + if (transformed != null) { + delegated.reportWarning(sender, transformed); + } + } + + @Override + public void reportDetailed(Object sender, Report report) { + Report transformed = filterReport(sender, report, true); + + if (transformed != null) { + delegated.reportDetailed(sender, transformed); + } + } + + /** + * Invoked before an error report is passed on to the underlying error reporter. + *

+ * To cancel a report, return NULL. + * @param sender - the sender component. + * @param report - the error report. + * @param detailed - whether or not the report will be displayed in detail. + * @return The report to pass on, or NULL to cancel it. + */ + protected Report filterReport(Object sender, Report report, boolean detailed) { + return report; + } + + @Override + public void reportWarning(Object sender, ReportBuilder reportBuilder) { + reportWarning(sender, reportBuilder.build()); + } + + @Override + public void reportDetailed(Object sender, ReportBuilder reportBuilder) { + reportDetailed(sender, reportBuilder.build()); + } +} diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java b/ProtocolLib/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java index ce3574a8..6953d437 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/error/DetailedErrorReporter.java @@ -342,7 +342,6 @@ public class DetailedErrorReporter implements ErrorReporter { * @return String representation. */ protected String getStringDescription(Object value) { - // We can't only rely on toString. if (value == null) { return "[NULL]"; diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedArrayList.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedArrayList.java index d8534215..33e45f63 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedArrayList.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/InjectedArrayList.java @@ -23,7 +23,6 @@ import java.util.ArrayList; import java.util.Set; import com.comphenix.protocol.ProtocolLibrary; -import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.error.Report; import com.comphenix.protocol.error.ReportType; import com.comphenix.protocol.injector.ListenerInvoker; @@ -88,15 +87,10 @@ class InjectedArrayList extends ArrayList { return true; } catch (InvocationTargetException e) { - ErrorReporter reporter = ProtocolLibrary.getErrorReporter(); - // Prefer to report this to the user, instead of risking sending it to Minecraft - if (reporter != null) { - reporter.reportDetailed(this, Report.newBuilder(REPORT_CANNOT_REVERT_CANCELLED_PACKET).error(e).callerParam(packet)); - } else { - System.out.println("[ProtocolLib] Reverting cancelled packet failed."); - e.printStackTrace(); - } + ProtocolLibrary.getErrorReporter().reportDetailed(this, + Report.newBuilder(REPORT_CANNOT_REVERT_CANCELLED_PACKET).error(e).callerParam(packet) + ); // Failure return false; diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java index f1f177da..02036a07 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/player/PlayerInjector.java @@ -368,7 +368,7 @@ public abstract class PlayerInjector implements SocketInjector { return null; hasProxyType = true; - reporter.reportWarning(this, Report.newBuilder(REPORT_DETECTED_CUSTOM_SERVER_HANDLER).callerParam(notchEntity, serverField)); + reporter.reportWarning(this, Report.newBuilder(REPORT_DETECTED_CUSTOM_SERVER_HANDLER).callerParam(serverField)); // No? Is it a Proxy type? try { diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java index 30d68021..71b3a1e3 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/injector/spigot/SpigotPacketInjector.java @@ -22,7 +22,9 @@ import net.sf.cglib.proxy.NoOp; import com.comphenix.protocol.Packets; import com.comphenix.protocol.concurrency.IntegerSet; +import com.comphenix.protocol.error.DelegatedErrorReporter; import com.comphenix.protocol.error.ErrorReporter; +import com.comphenix.protocol.error.Report; import com.comphenix.protocol.events.PacketContainer; import com.comphenix.protocol.events.PacketEvent; import com.comphenix.protocol.injector.ListenerInvoker; @@ -275,7 +277,8 @@ public class SpigotPacketInjector implements SpigotPacketListener { if (dummyInjector == null) { // Inject the network manager try { - NetworkObjectInjector created = new NetworkObjectInjector(classLoader, reporter, null, invoker, null); + NetworkObjectInjector created = new NetworkObjectInjector( + classLoader, filterImpossibleWarnings(reporter), null, invoker, null); if (MinecraftReflection.isLoginHandler(connection)) { created.initialize(connection); @@ -303,6 +306,23 @@ public class SpigotPacketInjector implements SpigotPacketListener { return dummyInjector; } + /** + * Return a delegated error reporter that ignores certain warnings that are irrelevant on Spigot. + * @param reporter - error reporter to delegate. + * @return The filtered error reporter. + */ + private ErrorReporter filterImpossibleWarnings(ErrorReporter reporter) { + return new DelegatedErrorReporter(reporter) { + @Override + protected Report filterReport(Object sender, Report report, boolean detailed) { + // This doesn't matter - ignore it + if (report.getType() == NetworkObjectInjector.REPORT_DETECTED_CUSTOM_SERVER_HANDLER) + return null; + return report; + } + }; + } + /** * Save a given player injector for later. * @param networkManager - the associated network manager. @@ -400,7 +420,8 @@ public class SpigotPacketInjector implements SpigotPacketListener { */ void injectPlayer(Player player) { try { - NetworkObjectInjector dummy = new NetworkObjectInjector(classLoader, reporter, player, invoker, null); + NetworkObjectInjector dummy = new NetworkObjectInjector( + classLoader, filterImpossibleWarnings(reporter), player, invoker, null); dummy.initializePlayer(player); // Save this player for the network manager diff --git a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/compiler/BackgroundCompiler.java b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/compiler/BackgroundCompiler.java index 6653916d..19e9e56c 100644 --- a/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/compiler/BackgroundCompiler.java +++ b/ProtocolLib/src/main/java/com/comphenix/protocol/reflect/compiler/BackgroundCompiler.java @@ -29,8 +29,6 @@ import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.ThreadFactory; import java.util.concurrent.TimeUnit; -import javax.annotation.Nullable; - import com.comphenix.protocol.error.ErrorReporter; import com.comphenix.protocol.error.Report; import com.comphenix.protocol.error.ReportType; @@ -122,11 +120,13 @@ public class BackgroundCompiler { } // Avoid "Constructor call must be the first statement". - private void initializeCompiler(ClassLoader loader, @Nullable ErrorReporter reporter, ExecutorService executor) { + private void initializeCompiler(ClassLoader loader, ErrorReporter reporter, ExecutorService executor) { if (loader == null) throw new IllegalArgumentException("loader cannot be NULL"); if (executor == null) throw new IllegalArgumentException("executor cannot be NULL"); + if (reporter == null) + throw new IllegalArgumentException("reporter cannot be NULL."); this.compiler = new StructureCompiler(loader); this.reporter = reporter; @@ -226,14 +226,9 @@ public class BackgroundCompiler { setEnabled(false); // Inform about this error as best as we can - if (reporter != null) { - reporter.reportDetailed(BackgroundCompiler.this, - Report.newBuilder(REPORT_CANNOT_COMPILE_STRUCTURE_MODIFIER).callerParam(uncompiled).error(e) - ); - } else { - System.err.println("Exception occured in structure compiler: "); - e.printStackTrace(); - } + reporter.reportDetailed(BackgroundCompiler.this, + Report.newBuilder(REPORT_CANNOT_COMPILE_STRUCTURE_MODIFIER).callerParam(uncompiled).error(e) + ); } // We'll also return the new structure modifier