Civcraft / NameLayer

Do not open issues here; open them on the maintained fork @ DevotedMC
https://github.com/DevotedMC/NameLayer
BSD 3-Clause "New" or "Revised" License
4 stars 14 forks source link

NPE after almost every command #140

Closed Maxopoly closed 8 years ago

Maxopoly commented 8 years ago

Tried to fix this on my own for quite a while, but I have no idea why this isn't working. So for almost every command, a stack like this comes up on civtest.

[14:11:10 ERROR]: Could not pass event AsyncPluginBroadcastMessageEvent to NameL ayer v2.5.0 org.bukkit.event.EventException at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.ja va:310) ~[spigot.jar:git-Spigot-d97e08b-880a532] at org.bukkit.plugin.RegisteredListener.callEvent(RegisteredListener.jav a:62) ~[spigot.jar:git-Spigot-d97e08b-880a532] at org.bukkit.plugin.SimplePluginManager.fireEvent(SimplePluginManager.j ava:502) [spigot.jar:git-Spigot-d97e08b-880a532] at org.bukkit.plugin.SimplePluginManager.callEvent(SimplePluginManager.j ava:487) [spigot.jar:git-Spigot-d97e08b-880a532] at vg.civcraft.mc.mercury.events.BukkitEventManager.fireMessage(BukkitEv entManager.java:14) [Mercury-1.1.91.jar:?] at vg.civcraft.mc.mercury.events.EventManager.fireMessage(EventManager.j ava:15) [Mercury-1.1.91.jar:?] at vg.civcraft.mc.mercury.rabbitmq.RabbitListenerThread.processNextDeliv ery(RabbitListenerThread.java:42) [Mercury-1.1.91.jar:?] at vg.civcraft.mc.mercury.rabbitmq.RabbitListenerThread.run(RabbitListen erThread.java:56) [Mercury-1.1.91.jar:?] Caused by: java.lang.NullPointerException at vg.civcraft.mc.namelayer.listeners.MercuryMessageListener.onMercuryMe ssage(MercuryMessageListener.java:43) ~[?:?] at sun.reflect.GeneratedMethodAccessor10.invoke(Unknown Source) ~[?:?] at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAcces sorImpl.java:43) ~[?:1.7.0_79] at java.lang.reflect.Method.invoke(Method.java:606) ~[?:1.7.0_79] at org.bukkit.plugin.java.JavaPluginLoader$1.execute(JavaPluginLoader.ja va:306) ~[spigot.jar:git-Spigot-d97e08b-880a532] ... 7 more

The NPE is caused by getName on the group here: https://github.com/Civcraft/NameLayer/blob/master/src/vg/civcraft/mc/namelayer/listeners/MercuryMessageListener.java#L43

This also happens for almost every other command, for some reason the groupmanager just returns null as a group, even though it should even check the db. This occurs on all shards, even the one the command was initially executed on.

At the same time an exception is also thrown here: https://github.com/Civcraft/JukeAlert/blob/master/src/com/untamedears/JukeAlert/listener/JukeAlertListener.java#L255 upon deleting a group, because the GroupDeleteEvent contains null as a group.

rourke750 commented 8 years ago

lol. So looks like I derped. Before the merc message NameLayer removes the group from the db so getting the group will in fact return null. Lets make a new event one that is lets say GroupInValidateEvent and then it has the reason a group was invalidated. Then get rid of this https://github.com/Civcraft/NameLayer/blob/master/src/vg/civcraft/mc/namelayer/listeners/MercuryMessageListener.java#L36 and just pass the name instead of the group object.

Can you do this or should I?

Maxopoly commented 8 years ago

Uh, I think I see what you mean, I'll try.

Maxopoly commented 8 years ago

What should I do with the GroupDeleteEvent that JukeAlert needs? I can't give it a group.

rourke750 commented 8 years ago

Make it have a new listener and then make it do what it needs to.

Maxopoly commented 8 years ago

Semirelated: I guess we could wipe out all the group events at some point, they won't be consistent with stuff happening in other shards, we can just listen for that GroupInvalidationEvent instead, which actually is consistent across all shards and we never wanted to cancel any of those events anyway.

rourke750 commented 8 years ago

they should not implement cancelable as they cant be.

Maxopoly commented 8 years ago

Tested it and it still doesn't work. I realized earlier that the NPE we were dealing with was actually caused by the groupmanager in the listener being null. The listener was initialized before the NameLayerAPI, so the API just returned null as groupmanager when the listener was setting that reference. I fixed that here https://github.com/Civcraft/NameLayer/commit/64f15febc245fa1bff2a5a91fde412dd8c1c6d20 , but that just moved the NPE a bit. Our new fun stack:

Caused by: java.lang.NullPointerException at vg.civcraft.mc.namelayer.GroupManager.invalidateCache(GroupManager.ja va:230) ~[?:?] at vg.civcraft.mc.namelayer.listeners.MercuryMessageListener.onMercuryMe ssage(MercuryMessageListener.java:46) ~[?:?]

I didn't go through all of the commands, but this happens at least for adding members and deleting groups, same stack in both cases.

rourke750 commented 8 years ago

Replace each of these methods https://github.com/Civcraft/NameLayer/blob/master/src/vg/civcraft/mc/namelayer/listeners/MercuryMessageListener.java#L41 in the four reasons with

if (GroupManager.getGroup(group) != null) gm.invalidateCache(group);

Maxopoly commented 8 years ago

Okay, I did that and as far as I can see everything seems to work now. Until we get some final sharding testing we can probably view Namelayer as finished.