Don't waste DB rows on bad inbox actors

Seems Sidekiq will keep trying to deliver messages even when the actor
no longer exists?

5 files changed, 59 insertions, 20 deletions

diff --git a/core/exceptions.py b/core/exceptions.py
index 4475538..f8c0c50 100644
--- a/core/exceptions.py
+++ b/core/exceptions.py
@@ -1,3 +1,9 @@
+import traceback
+
+from asgiref.sync import sync_to_async
+from django.conf import settings
+
+
 class ActivityPubError(BaseException):
     """
     A problem with an ActivityPub message
@@ -8,3 +14,30 @@ class ActorMismatchError(ActivityPubError):
     """
     The actor is not authorised to do the action we saw
     """
+
+
+def capture_message(message: str):
+    """
+    Sends the informational message to Sentry if it's configured
+    """
+    if settings.SENTRY_ENABLED:
+        from sentry_sdk import capture_message
+
+        capture_message(message)
+    elif settings.DEBUG:
+        print(message)
+
+
+def capture_exception(exception: BaseException):
+    """
+    Sends the exception to Sentry if it's configured
+    """
+    if settings.SENTRY_ENABLED:
+        from sentry_sdk import capture_exception
+
+        capture_exception(exception)
+    elif settings.DEBUG:
+        traceback.print_exc()
+
+
+acapture_exception = sync_to_async(capture_exception, thread_sensitive=False)
diff --git a/stator/models.py b/stator/models.py
index 426803a..bbff395 100644
--- a/stator/models.py
+++ b/stator/models.py
@@ -4,11 +4,11 @@ import traceback
 from typing import ClassVar, List, Optional, Type, Union, cast
 
 from asgiref.sync import sync_to_async
-from django.conf import settings
 from django.db import models, transaction
 from django.utils import timezone
 from django.utils.functional import classproperty
 
+from core import exceptions
 from stator.graph import State, StateGraph
 
 
@@ -155,10 +155,7 @@ class StatorModel(models.Model):
             next_state = await current_state.handler(self)
         except BaseException as e:
             await StatorError.acreate_from_instance(self, e)
-            if settings.SENTRY_ENABLED:
-                from sentry_sdk import capture_exception
-
-                await sync_to_async(capture_exception, thread_sensitive=False)(e)
+            await exceptions.acapture_exception(e)
             traceback.print_exc()
         else:
             if next_state:
diff --git a/stator/runner.py b/stator/runner.py
index 7daf921..21c6128 100644
--- a/stator/runner.py
+++ b/stator/runner.py
@@ -5,10 +5,9 @@ import traceback
 import uuid
 from typing import List, Optional, Type
 
-from asgiref.sync import sync_to_async
-from django.conf import settings
 from django.utils import timezone
 
+from core import exceptions
 from stator.models import StatorModel
 
 
@@ -93,10 +92,7 @@ class StatorRunner:
             )
             await instance.atransition_attempt()
         except BaseException as e:
-            if settings.SENTRY_ENABLED:
-                from sentry_sdk import capture_exception
-
-                await sync_to_async(capture_exception, thread_sensitive=False)(e)
+            await exceptions.acapture_exception(e)
             traceback.print_exc()
 
     def remove_completed_tasks(self):
diff --git a/users/models/identity.py b/users/models/identity.py
index 510b947..c80d9d9 100644
--- a/users/models/identity.py
+++ b/users/models/identity.py
@@ -176,12 +176,17 @@ class Identity(StatorModel):
             return None
 
     @classmethod
-    def by_actor_uri(cls, uri, create=False) -> "Identity":
+    def by_actor_uri(cls, uri, create=False, transient=False) -> "Identity":
         try:
             return cls.objects.get(actor_uri=uri)
         except cls.DoesNotExist:
             if create:
-                return cls.objects.create(actor_uri=uri, local=False)
+                if transient:
+                    # Some code (like inbox fetching) doesn't need this saved
+                    # to the DB until the fetch succeeds
+                    return cls(actor_uri=uri, local=False)
+                else:
+                    return cls.objects.create(actor_uri=uri, local=False)
             else:
                 raise cls.DoesNotExist(f"No identity found with actor_uri {uri}")
 
@@ -329,7 +334,8 @@ class Identity(StatorModel):
                 return False
             if response.status_code == 410:
                 # Their account got deleted, so let's do the same.
-                await Identity.objects.filter(pk=self.pk).adelete()
+                if self.pk:
+                    await Identity.objects.filter(pk=self.pk).adelete()
                 return False
             if response.status_code >= 400:
                 return False
diff --git a/users/views/activitypub.py b/users/views/activitypub.py
index 2719f17..c0fcd98 100644
--- a/users/views/activitypub.py
+++ b/users/views/activitypub.py
@@ -8,6 +8,7 @@ from django.views.decorators.csrf import csrf_exempt
 from django.views.generic import View
 
 from activities.models import Post
+from core import exceptions
 from core.ld import canonicalise
 from core.models import Config
 from core.signatures import (
@@ -131,22 +132,26 @@ class Inbox(View):
         # Find the Identity by the actor on the incoming item
         # This ensures that the signature used for the headers matches the actor
         # described in the payload.
-        identity = Identity.by_actor_uri(document["actor"], create=True)
+        identity = Identity.by_actor_uri(document["actor"], create=True, transient=True)
         if not identity.public_key:
             # See if we can fetch it right now
             async_to_sync(identity.fetch_actor)()
         if not identity.public_key:
-            print("Cannot get actor", document["actor"])
+            exceptions.capture_message(
+                f"Inbox error: cannot fetch actor {document['actor']}"
+            )
             return HttpResponseBadRequest("Cannot retrieve actor")
         # If there's a "signature" payload, verify against that
         if "signature" in document:
             try:
                 LDSignature.verify_signature(document, identity.public_key)
             except VerificationFormatError as e:
-                print("Bad LD signature format:", e.args[0])
+                exceptions.capture_message(
+                    f"Inbox error: Bad LD signature format: {e.args[0]}"
+                )
                 return HttpResponseBadRequest(e.args[0])
             except VerificationError:
-                print("Bad LD signature")
+                exceptions.capture_message("Inbox error: Bad LD signature")
                 return HttpResponseUnauthorized("Bad signature")
         # Otherwise, verify against the header (assuming it's the same actor)
         else:
@@ -156,10 +161,12 @@ class Inbox(View):
                     identity.public_key,
                 )
             except VerificationFormatError as e:
-                print("Bad HTTP signature format:", e.args[0])
+                exceptions.capture_message(
+                    f"Inbox error: Bad HTTP signature format: {e.args[0]}"
+                )
                 return HttpResponseBadRequest(e.args[0])
             except VerificationError:
-                print("Bad HTTP signature")
+                exceptions.capture_message("Inbox error: Bad HTTP signature")
                 return HttpResponseUnauthorized("Bad signature")
         # Hand off the item to be processed by the queue
         InboxMessage.objects.create(message=document)