Fediverse: Deduplicate username formatting and add some error handling.

plugins/Fediverse/plugin.py

@@ -154,25 +154,43 @@ class Fediverse(callbacks.PluginRegexp):
             if match:
                 # TODO: error handling
                 actor = ap.get_resource_from_url(match.group(0))
-                username = self._format_actor_username(actor)
+                try:
+                    hostname = urllib.parse.urlparse(actor.get("id")).hostname
+                    username = "@%s@%s" % (
+                        hostname,
+                        actor.get["preferredUsername"],
+                    )
+                except Exception:
+                    username = None
             else:
                 irc.errorInvalid("fediverse username", username)
 
+        if username:
             self._actor_cache[username] = actor
         self._actor_cache[actor["id"]] = actor
 
         return actor
 
-    def _format_actor_username(self, actor):
+    def _format_actor_fullname(self, actor):
-        hostname = urllib.parse.urlparse(actor["id"]).hostname
+        try:
-        return "@%s@%s" % (actor["preferredUsername"], hostname)
+            hostname = urllib.parse.urlparse(actor.get("id")).hostname
+        except Exception:
+            hostname = "<unknown>"
+        username = actor.get("preferredUsername", "<unknown>")
+        name = actor.get("name", username)
+        return "\x02%s\x02 (@%s@%s)" % (name, username, hostname)
 
     def _format_status(self, irc, msg, status):
         if status["type"] == "Create":
             return self._format_status(irc, msg, status["object"])
         elif status["type"] == "Note":
             author_url = status["attributedTo"]
+            try:
                 author = self._get_actor(irc, author_url)
+            except ap.ActivityPubError as e:
+                author_fullname = _("<error: %s>") % str(e)
+            else:
+                author_fullname = self._format_actor_fullname(author)
             cw = status.get("summary")
             if cw:
                 if self.registryValue(
@@ -181,24 +199,18 @@ class Fediverse(callbacks.PluginRegexp):
                     irc.network,
                 ):
                     # show CW and content
-                    return _("\x02%s (%s)\x02: \x02[CW %s]\x02 %s") % (
+                    return _("%s: \x02[CW %s]\x02 %s") % (
-                        author["name"],
+                        author_fullname,
-                        self._format_actor_username(author),
                         cw,
                         utils.web.htmlToText(status["content"]),
                     )
                 else:
                     # show CW but not content
-                    return _("\x02%s (%s)\x02: CW %s") % (
+                    return _("%s: CW %s") % (author_fullname, cw)
-                        author["name"],
-                        self._format_actor_username(author),
-                        cw,
-                    )
             else:
                 # no CW, show content
-                return _("\x02%s (%s)\x02: %s") % (
+                return _("%s: %s") % (
-                    author["name"],
+                    author_fullname,
-                    self._format_actor_username(author),
                     utils.web.htmlToText(status["content"]),
                 )
         elif status["type"] == "Announce":
@@ -225,18 +237,16 @@ class Fediverse(callbacks.PluginRegexp):
         actor = self._get_actor(irc, username)
 
         irc.reply(
-            _("\x02%s\x02 (%s): %s")
+            _("%s: %s")
             % (
-                actor["name"],
+                self._format_actor_fullname(actor),
-                self._format_actor_username(actor),
                 utils.web.htmlToText(actor["summary"]),
             )
         )
 
     def _format_profile(self, irc, msg, actor):
-        return _("\x02%s\x02 (%s): %s") % (
+        return _("%s: %s") % (
-            actor["name"],
+            self._format_actor_fullname(actor),
-            self._format_actor_username(actor),
             utils.web.htmlToText(actor["summary"]),
         )
 

plugins/Fediverse/test.py

@@ -34,7 +34,7 @@ import functools
 import contextlib
 from multiprocessing import Manager
 
-from supybot import conf, utils
+from supybot import commands, conf, utils
 from supybot.test import ChannelPluginTestCase, network
 
 from .test_data import (
@@ -59,6 +59,10 @@ from .test_data import (
 
 
 class FediverseTestCase(ChannelPluginTestCase):
+    config = {
+        # Allow snarfing the same URL twice in a row
+        "supybot.snarfThrottle": 0.0
+    }
     plugins = ("Fediverse",)
 
     def setUp(self):
@@ -286,7 +290,7 @@ class FediverseTestCase(ChannelPluginTestCase):
         with self.mockRequests(expected_requests):
             self.assertResponse(
                 "status https://example.org/users/someuser/statuses/1234",
-                "\x02someuser (@someuser@example.org)\x02: "
+                "\x02someuser\x02 (@someuser@example.org): "
                 + "@ FirstAuthor I am replying to you",
             )
 
@@ -299,6 +303,17 @@ class FediverseTestCase(ChannelPluginTestCase):
                 "Error: Could not get status: blah",
             )
 
+        expected_requests = [
+            (STATUS_URL, STATUS_DATA),
+            (ACTOR_URL, utils.web.Error("blah")),
+        ]
+
+        with self.mockRequests(expected_requests):
+            self.assertResponse(
+                "status https://example.org/users/someuser/statuses/1234",
+                "<error: blah>: " + "@ FirstAuthor I am replying to you",
+            )
+
     def testStatuses(self):
         expected_requests = [
             (HOSTMETA_URL, HOSTMETA_DATA),
@@ -313,12 +328,12 @@ class FediverseTestCase(ChannelPluginTestCase):
         with self.mockRequests(expected_requests):
             self.assertResponse(
                 "statuses @someuser@example.org",
-                "\x02someuser (@someuser@example.org)\x02: "
+                "\x02someuser\x02 (@someuser@example.org): "
                 + "@ FirstAuthor I am replying to you, "
-                + "\x02someuser (@someuser@example.org)\x02: "
+                + "\x02someuser\x02 (@someuser@example.org): "
                 + "\x02[CW This is a content warning]\x02 "
                 + "This is a status with a content warning, and "
-                + "\x02Boosted User (@BoostedUser@example.net)\x02: "
+                + "\x02Boosted User\x02 (@BoostedUser@example.net): "
                 + "Status Content",
             )
 
@@ -335,33 +350,23 @@ class FediverseTestCase(ChannelPluginTestCase):
             ):
                 self.assertResponse(
                     "statuses @someuser@example.org",
-                    "\x02someuser (@someuser@example.org)\x02: "
+                    "\x02someuser\x02 (@someuser@example.org): "
                     + "@ FirstAuthor I am replying to you, "
-                    + "\x02someuser (@someuser@example.org)\x02: "
+                    + "\x02someuser\x02 (@someuser@example.org): "
                     + "CW This is a content warning, and "
-                    + "\x02Boosted User (@BoostedUser@example.net)\x02: "
+                    + "\x02Boosted User\x02 (@BoostedUser@example.net): "
                     + "Status Content",
                 )
 
-    def testStatusUrlSnarfer(self):
+    def testStatusUrlSnarferDisabled(self):
         with self.mockRequests([]):
             self.assertSnarfNoResponse(
                 "aaa https://example.org/users/someuser/statuses/1234 bbb",
                 timeout=1,
             )
 
+    def testStatusUrlSnarfer(self):
         with conf.supybot.plugins.Fediverse.snarfers.status.context(True):
-            expected_requests = [
-                (STATUS_URL, STATUS_DATA),
-                (ACTOR_URL, utils.web.Error("blah")),
-            ]
-
-            with self.mockRequests(expected_requests):
-                self.assertSnarfNoResponse(
-                    "aaa https://example.org/users/someuser/statuses/1234 bbb",
-                    timeout=1,
-                )
-
             expected_requests = [
                 (STATUS_URL, STATUS_DATA),
                 (ACTOR_URL, ACTOR_DATA),
@@ -370,10 +375,31 @@ class FediverseTestCase(ChannelPluginTestCase):
             with self.mockRequests(expected_requests):
                 self.assertSnarfResponse(
                     "aaa https://example.org/users/someuser/statuses/1234 bbb",
-                    "\x02someuser (@someuser@example.org)\x02: "
+                    "\x02someuser\x02 (@someuser@example.org): "
                     + "@ FirstAuthor I am replying to you",
                 )
 
+    def testStatusUrlSnarferErrors(self):
+        with conf.supybot.plugins.Fediverse.snarfers.status.context(True):
+            expected_requests = [(STATUS_URL, utils.web.Error("blah"))]
+
+            with self.mockRequests(expected_requests):
+                self.assertSnarfNoResponse(
+                    "aaa https://example.org/users/someuser/statuses/1234 bbb",
+                    timeout=1,
+                )
+
+            expected_requests = [
+                (STATUS_URL, STATUS_DATA),
+                (ACTOR_URL, utils.web.Error("blah")),
+            ]
+
+            with self.mockRequests(expected_requests):
+                self.assertSnarfResponse(
+                    "aaa https://example.org/users/someuser/statuses/1234 bbb",
+                    "<error: blah>: @ FirstAuthor I am replying to you",
+                )
+
     def testSnarferType(self):
         # Sends a request, notices it's a status, gives up
         with conf.supybot.plugins.Fediverse.snarfers.profile.context(True):