Revise handling of KILL and QUIT hooks

- Both of these now always contain a non-empty userdata argument.
- If we receive both a KILL and a QUIT for any client, only the one received first will be sent as a hook.
- Also, adjust _remove_client() to return the data of the user that was removed.

classes.py

@@ -631,7 +631,10 @@ class PyLinkNetworkCore(structures.CamelCaseToSnakeCase):
         self.to_lower.cache_clear()
 
     def _remove_client(self, numeric):
-        """Internal function to remove a client from our internal state."""
+        """
+        Internal function to remove a client from our internal state.
+
+        If the removal was successful, return the User object for the given numeric (UID)."""
         for c, v in self.channels.copy().items():
             v.remove_user(numeric)
             # Clear empty non-permanent channels.
@@ -640,6 +643,7 @@ class PyLinkNetworkCore(structures.CamelCaseToSnakeCase):
 
         sid = self.get_server(numeric)
         try:
+            userobj = self.users[numeric]
             del self.users[numeric]
             self.servers[sid].users.discard(numeric)
         except KeyError:
@@ -647,6 +651,7 @@ class PyLinkNetworkCore(structures.CamelCaseToSnakeCase):
                       exc_info=True)
         else:
             log.debug('(%s) Removing client %s from user + server state', self.name, numeric)
+            return userobj
 
     ## State checking functions
     def nick_to_uid(self, nick, multi=False, filterfunc=None):

docs/technical/hooks-reference.md

@@ -1,6 +1,6 @@
 # PyLink hooks reference
 
-***Last updated for 2.1-dev (2018-12-27).***
+***Last updated for 2.1-alpha2 (2019-07-01).***
 
 In PyLink, protocol modules communicate with plugins through a system of hooks. This has the benefit of being IRCd-independent, allowing most plugins to function regardless of the IRCd being used.
 Each hook payload is formatted as a Python `list`, with three arguments: `numeric`, `command`, and `args`.
@@ -64,9 +64,9 @@ The following hooks represent regular IRC commands sent between servers.
 - **KICK**: `{'channel': '#channel', 'target': 'UID1', 'text': 'some reason'}`
     - `text` refers to the kick reason. The `target` and `channel` fields send the target's UID and the channel they were kicked from, and the sender of the hook payload is the kicker.
 
-- **KILL**: `{'target': killed, 'text': 'Killed (james (absolutely not))', 'userdata': data}`
+- **KILL**: `{'target': killed, 'text': 'Killed (james (absolutely not))', 'userdata': User(...)}`
     - `text` refers to the kill reason. `target` is the target's UID.
-    - The `userdata` key may include an `classes.User` instance, depending on the IRCd. On IRCds where QUITs are explicitly sent (e.g InspIRCd), `userdata` will be `None`. Other IRCds do not explicitly send QUIT messages for killed clients, so the daemon must assume that they've quit, and deliver their last state to plugins that require this info.
+    - `userdata` includes a `classes.User` instance, containing the information of the killed user.
 
 - **MODE**: `{'target': '#channel', 'modes': [('+m', None), ('+i', None), ('+t', None), ('+l', '3'), ('-o', 'person')], 'channeldata': Channel(...)}`
     - `target` is the target the mode is being set on: it may be either a channel (for channel modes) *or* a UID (for user modes).
@@ -86,8 +86,9 @@ The following hooks represent regular IRC commands sent between servers.
 - **PRIVMSG**: `{'target': 'UID3', 'text': 'hi there!'}`
     - Ditto with NOTICE: STATUSMSG targets (e.g. `@#lounge`) are also allowed here.
 
-- **QUIT**: `{'text': 'Quit: Bye everyone!'}`
+- **QUIT**: `{'text': 'Quit: Bye everyone!', 'userdata': User(...)}`
     - `text` corresponds to the quit reason.
+    - `userdata` includes a `classes.User` instance, containing the information of the killed user.
 
 - **SQUIT**: `{'target': '800', 'users': ['UID1', 'UID2', 'UID6'], 'name': 'some.server', 'uplink': '24X', 'nicks': {'#channel1: ['tester1', 'tester2'], '#channel3': ['somebot']}, 'serverdata': Server(...), 'affected_servers': ['SID1', 'SID2', 'SID3']`
     - `target` is the SID of the server being split, while `name` is the server's name.
@@ -161,6 +162,9 @@ Some hooks do not map directly to IRC commands, but to events that protocol modu
 At this time, commands that are handled by protocol modules without returning any hook data include PING, PONG, and various commands sent during the initial server linking phase.
 
 ## Changes
+
+* 2019-07-01 (2.1-alpha2)
+   - KILL and QUIT hooks now always include a non-empty `userdata` key. Now, if a QUIT message for a killed user is received before the corresponding KILL (or vice versa), only the first message received will have the corresponding hook payload broadcasted.
 * 2018-12-27 (2.1-dev)
    - Add the `affected_servers` argument to SQUIT hooks.
 * 2018-07-11 (2.0.0)

protocols/clientbot.py

@@ -162,8 +162,7 @@ class ClientbotBaseProtocol(PyLinkNetworkCoreWithUtils):
 
     def quit(self, source, reason):
         """STUB: Quits a client."""
-        userdata = self.users[source]
-        self._remove_client(source)
+        userdata = self._remove_client(source)
         self.call_hooks([source, 'CLIENTBOT_QUIT', {'text': reason, 'userdata': userdata}])
 
     def _stub(self, *args):
@@ -1107,9 +1106,13 @@ class ClientbotWrapperProtocol(ClientbotBaseProtocol, IRCCommonProtocol):
         if self.pseudoclient and source == self.pseudoclient.uid:
             # Someone faked a quit from us? We should abort.
             raise ProtocolError("Received QUIT from uplink (%s)" % args[0])
+        elif source not in self.users:
+            log.debug('(%s) Ignoring QUIT on non-existent user %s', self.name, source)
+            return
 
+        userdata = self.users[source]
         self.quit(source, args[0])
-        return {'text': args[0]}
+        return {'text': args[0], 'userdata': userdata}
 
     def handle_404(self, source, command, args):
         """

protocols/ircs2s_common.py

@@ -488,14 +488,15 @@ class IRCS2SProtocol(IRCCommonProtocol):
     def handle_kill(self, source, command, args):
         """Handles incoming KILLs."""
         killed = self._get_UID(args[0])
-        # Depending on whether the IRCd sends explicit QUIT messages for
-        # killed clients, the user may or may not have automatically been
-        # removed from our user list.
-        # If not, we have to assume that KILL = QUIT and remove them
-        # ourselves.
-        data = self.users.get(killed)
-        if data:
-            self._remove_client(killed)
+        # Some IRCds send explicit QUIT messages for their killed clients in addition to KILL,
+        # meaning that our target client may have been removed already. If this is the case,
+        # don't bother forwarding this message on.
+        # Generally, we only need to distinguish between KILL and QUIT if the target is
+        # one of our clients, in which case the above statement isn't really applicable.
+        if killed in self.users:
+            userdata = self._remove_client(killed)
+        else:
+            return
 
         # TS6-style kills look something like this:
         # <- :GL KILL 38QAAAAAA :hidden-1C620195!GL (test)
@@ -526,9 +527,9 @@ class IRCS2SProtocol(IRCCommonProtocol):
             # <- :GL KILL PyLink-devel :KILLed by GL: ?
             killmsg = args[1]
 
-        return {'target': killed, 'text': killmsg, 'userdata': data}
+        return {'target': killed, 'text': killmsg, 'userdata': userdata}
 
-    def _check_cloak_change(self, uid):
+    def _check_cloak_change(self, uid):  # Stub by default
         return
 
     def _check_umode_away_change(self, uid):
@@ -676,8 +677,9 @@ class IRCS2SProtocol(IRCCommonProtocol):
         # <- :1SRAAGB4T QUIT :Quit: quit message goes here
         # P10:
         # <- ABAAB Q :Killed (GL_ (bangbang))
-        self._remove_client(numeric)
-        return {'text': args[0]}
+        userdata = self._remove_client(numeric)
+        if userdata:
+            return {'text': args[0], 'userdata': userdata}
 
     def handle_stats(self, numeric, command, args):
         """Handles the IRC STATS command."""