Solved Hooking plugin loggers - events vs. manual hooking

Overall, I'm looking for a solution to this that will hook all loggers once with absolute certainty.

My initial approach was to have a single "hookBukkit" method that would be called after all plugins were loaded:

Code:java

    /**
     * Hooks into bukkit's plugin system and adds a handler to all plugin
     * loggers to allow catching unreported exceptions. If already hooked,
     * will do nothing.
     *
     * @since 0.1.0
     * @version 0.1.0
     *
     * @throws IllegalAccessException If something other than
     *                                {@link CodelanxLib} calls this method
     */
    public static void hookBukkit() throws IllegalAccessException {
        if (Debugger.hooked) {
            return;
        }
        //Check to make sure CodelanxLib is calling it
        StackTraceElement[] elems = Thread.currentThread().getStackTrace();
        if (elems.length < 3) {
            return;
        }
        if (!elems[2].getClass().getName().equals(CodelanxLib.class.getName())) {
            throw new IllegalAccessException("Debugger#hookBukkit may only be called by CodelanxLib!");
        }
        //end check - add hook
        Debugger.hooked = true;
        for (Plugin p : Bukkit.getServer().getPluginManager().getPlugins()) {
            p.getLogger().addHandler(new ExceptionHandler(p));
        }
    }

However, this wouldn't catch plugins that are dynamically loaded after a server start. So my followup idea was to hook on plugin enabling:

Code:java

    static {
        Bukkit.getServer().getPluginManager().registerEvents(new Listener() {
 
            @EventHandler
            public void onEnable(PluginEnableEvent event) {
                for (Handler h : event.getPlugin().getLogger().getHandlers()) {
                    if (h instanceof ExceptionHandler) {
                        return;
                    }
                }
                event.getPlugin().getLogger().addHandler(new ExceptionHandler(event.getPlugin()));
            }
 
        }, CodelanxLib.get()); //potentially would fail if reference is resolved too early
    }

But since the internals for CraftBukkit are now no longer public, I have no way of knowing when the PluginManager is initialized, which raises two issues: I don't know when it is too early to call upon the PluginManager, and I can't register it too late or else miss the event being fired.

A combination (with delayed listener registration and verification in the hook method) seems sloppy, but that's what I'm currently operating with. So I'm putting this out here for kudos if anyone can come up with a better solution.

For anyone curious about the current solution:

Show Spoiler Hide Spoiler

Code:java

    /**
     * Hooks into bukkit's plugin system and adds a handler to all plugin
     * loggers to allow catching unreported exceptions. If already hooked,
     * will do nothing.
     *
     * @since 0.1.0
     * @version 0.1.0
     *
     * @throws IllegalAccessException If something other than
     *                                {@link CodelanxLib} calls this method
     */
    public static void hookBukkit() throws IllegalAccessException {
        //Check to make sure CodelanxLib is calling it
        StackTraceElement[] elems = Thread.currentThread().getStackTrace();
        if (elems.length < 3) {
            return;
        }
        if (!elems[2].getClass().getName().equals(CodelanxLib.class.getName())) {
            throw new IllegalAccessException("Debugger#hookBukkit may only be called by CodelanxLib!");
        }
        //end check - add hook
        if (!Debugger.listenerHooked) {
            Bukkit.getServer().getPluginManager().registerEvents(new Listener() {
 
                @EventHandler
                public void onEnable(PluginEnableEvent event) {
                    for (Handler h : event.getPlugin().getLogger().getHandlers()) {
                        if (h instanceof ExceptionHandler) {
                            return;
                        }
                    }
                    event.getPlugin().getLogger().addHandler(new ExceptionHandler(event.getPlugin()));
                }
 
            }, CodelanxLib.get());
        }
        Debugger.listenerHooked = true;
        pluginLoop:
        for (Plugin p : Bukkit.getServer().getPluginManager().getPlugins()) {
            for (Handler h : p.getLogger().getHandlers()) {
                if (h instanceof ExceptionHandler) {
                    continue pluginLoop;
                }
            }
            p.getLogger().addHandler(new ExceptionHandler(p));
        }
    }

 

To be completely frank with you I don't really see a combination as sloppy. It's really the best way I can think of.

If I may comment on your style, though, I would create a method for adding the handler to a plugin's logger. This shortens your code, applies the DRY principle, creates an abstraction layer AND gets rid of that annoying label.

Off-Topic: Love your reference to The Hitchhiker's Guide to the Galaxy in your signature

 

Following up on this, this is my end solution:

Code:java

/**
* Hooks into Bukkit's plugin system and adds a handler to all plugin
* loggers to allow catching unreported exceptions. If already hooked, will
* do nothing. This method will continue to hook new plugins via a listener
*
* @since 0.1.0
* @version 0.1.0
*
* @throws IllegalPluginAccessException If something other than
*                                      {@link CodelanxLib} calls this
*                                      method
*/
public static void hookBukkit() {
    //Check to make sure CodelanxLib is calling it
    Exceptions.illegalPluginAccess(Reflections.accessedFrom(CodelanxLib.class),
            "Debugger#hookBukkit may only be called by CodelanxLib!");
    Listener l = new BukkitPluginListener();
    if (!ListenerManager.isRegisteredToBukkit(CodelanxLib.get(), l)) {
        Bukkit.getServer().getPluginManager().registerEvents(l, CodelanxLib.get());
    }
    //Hook any current plugins without a handler
    for (Plugin p : Bukkit.getServer().getPluginManager().getPlugins()) { //boo arrays
        ExceptionHandler.apply(p);
    }
}

In the end it was a combination, additional changes:

• Instead of using a static boolean, check bukkit's registered listeners
• Applied DRY principle, because code shouldn't be wet
• Lots of cleanup since other aspects of the API have developed as well (e.g. Reflections)

The main thing is getting rid of the static boolean, as my primary concern was that the behavior would be undefined through a bukkit reload, and thus could potentially make the plugin not re-register a listener. Dynamically checking myself via:

Code:java

/**
* Returns {@code true} if the passed {@link Listener} has another Listener
* of the same class type already registered for bukkit. This should not be
* used with any listeners that are from an anonymous class, as this will
* return {@code true} for any other anonymous classes as well
*
* @since 0.1.0
* @version 0.1.0
*
* @param p The {@link Plugin} that registers this {@link Listener}
* @param l The {@link Listener} to check
* @return {@code true} if registered to bukkit
*/
public static boolean isRegisteredToBukkit(Plugin p, Listener l) {
    if (l.getClass().isAnonymousClass()) {
        StackTraceElement t = Reflections.getCaller();
        Logging.simple().here().print(Level.WARNING, "Passed an anonymous class from %s:%d", t.getClass().getName(), t.getLineNumber());
    }
    return HandlerList.getRegisteredListeners(p).stream().anyMatch(r -> r.getListener().getClass() == l.getClass());
}

Seems to have been the better idea.