Throw UnrecoverableMessageHandlingException on 4xx

When we try to deliver an activity to an inbox and receive a 4xx status code other than the 429 for rate limiting we now discard that message, so it will not get retried. This also doesn't count as a failed delivery to the instance anymore, because the instance is up, it just didn't accept the activity. TransportExceptions are now caught as well, since those are the ones we get when an instance is down

src/Exception/InvalidApPostException.php

@@ -6,4 +6,23 @@
 
 final class InvalidApPostException extends \Exception
 {
+    public function __construct(public ?string $messageStart = '', public ?string $url = null, public ?int $responseCode = null, public ?array $payload = null, int $code = 0, ?\Throwable $previous = null)
+    {
+        $message = $this->messageStart;
+        $additions = [];
+        if ($url) {
+            $additions[] = $url;
+        }
+        if ($responseCode) {
+            $additions[] = "status code: $responseCode";
+        }
+        if ($payload) {
+            $jsonPayload = json_encode($this->payload);
+            $additions[] = $jsonPayload;
+        }
+        if (0 < \sizeof($additions)) {
+            $message .= ': '.implode(', ', $additions);
+        }
+        parent::__construct($message, $code, $previous);
+    }
 }

src/MessageHandler/ActivityPub/Outbox/DeliverHandler.php

@@ -5,7 +5,9 @@
 namespace App\MessageHandler\ActivityPub\Outbox;
 
 use App\Entity\User;
+use App\Exception\InstanceBannedException;
 use App\Exception\InvalidApPostException;
+use App\Exception\InvalidWebfingerException;
 use App\Message\ActivityPub\Outbox\DeliverMessage;
 use App\Message\Contracts\MessageInterface;
 use App\MessageHandler\MbinMessageHandler;
@@ -14,8 +16,12 @@
 use App\Service\ActivityPubManager;
 use App\Service\SettingsManager;
 use Doctrine\ORM\EntityManagerInterface;
+use Psr\Cache\InvalidArgumentException;
 use Psr\Log\LoggerInterface;
 use Symfony\Component\Messenger\Attribute\AsMessageHandler;
+use Symfony\Component\Messenger\Exception\RecoverableMessageHandlingException;
+use Symfony\Component\Messenger\Exception\UnrecoverableMessageHandlingException;
+use Symfony\Contracts\HttpClient\Exception\TransportExceptionInterface;
 
 #[AsMessageHandler]
 class DeliverHandler extends MbinMessageHandler
@@ -53,8 +59,21 @@ public function workWrapper(MessageInterface $message): void
             $this->doWork($message);
             $conn->commit();
         } catch (InvalidApPostException $e) {
+            if (400 <= $e->responseCode && 500 > $e->responseCode && 429 !== $e->responseCode) {
+                $conn->rollBack();
+                throw new UnrecoverableMessageHandlingException('There is a problem with the request which will stay the same, so discarding', previous: $e);
+            } elseif (429 === $e->responseCode) {
+                $conn->rollBack();
+                // a rate limit is always recoverable
+                throw new RecoverableMessageHandlingException(previous: $e);
+            } else {
+                // we don't roll back on an InvalidApPostException, so the failed delivery attempt gets written to the DB
+                $conn->commit();
+                throw $e;
+            }
+        } catch (TransportExceptionInterface $e) {
+            // we don't roll back on an TransportExceptionInterface, so the failed delivery attempt gets written to the DB
             $conn->commit();
-            // we don't roll back on an InvalidApPostException, so the failed delivery attempt gets written to the DB
             throw $e;
         } catch (\Exception $e) {
             $conn->rollBack();
@@ -64,6 +83,13 @@ public function workWrapper(MessageInterface $message): void
         $conn->close();
     }
 
+    /**
+     * @throws InvalidApPostException
+     * @throws TransportExceptionInterface
+     * @throws InvalidArgumentException
+     * @throws InstanceBannedException
+     * @throws InvalidWebfingerException
+     */
     public function doWork(MessageInterface $message): void
     {
         if (!($message instanceof DeliverMessage)) {

src/Service/ActivityPub/ApHttpClient.php

@@ -102,7 +102,7 @@ private function getActivityObjectImpl(string $url): ?string
             // Accepted status code are 2xx or 410 (used Tombstone types)
             if (!str_starts_with((string) $statusCode, '2') && 410 !== $statusCode) {
                 // Do NOT include the response content in the error message, this will be often a full HTML page
-                throw new InvalidApPostException("Invalid status code while getting: $url, status code: $statusCode");
+                throw new InvalidApPostException('Invalid status code while getting', $url, $statusCode);
             }
 
             // Read also non-OK responses (like 410) by passing 'false'
@@ -318,7 +318,7 @@ private function getCollectionObjectImpl(string $apAddress): ?string
             // Accepted status code are 2xx or 410 (used Tombstone types)
             if (!str_starts_with((string) $statusCode, '2') && 410 !== $statusCode) {
                 // Do NOT include the response content in the error message, this will be often a full HTML page
-                throw new InvalidApPostException("Invalid status code while getting: $apAddress, status code: $statusCode");
+                throw new InvalidApPostException('Invalid status code while getting', $apAddress, $statusCode);
             }
         } catch (\Exception $e) {
             $this->logRequestException($response, $apAddress, 'ApHttpClient:getCollectionObject', $e);
@@ -374,7 +374,8 @@ private function logRequestException(?ResponseInterface $response, string $reque
      * @param User|Magazine $actor The actor initiating the request, either a User or Magazine object
      * @param array|null    $body  (Optional) The body of the POST request. Defaults to null.
      *
-     * @throws InvalidApPostException if the POST request fails with a non-2xx response status code
+     * @throws InvalidApPostException      if the POST request fails with a non-2xx response status code
+     * @throws TransportExceptionInterface
      */
     public function post(string $url, User|Magazine $actor, ?array $body = null): void
     {
@@ -407,7 +408,7 @@ public function post(string $url, User|Magazine $actor, ?array $body = null): vo
             $statusCode = $response->getStatusCode();
             if (!str_starts_with((string) $statusCode, '2')) {
                 // Do NOT include the response content in the error message, this will be often a full HTML page
-                throw new InvalidApPostException("Post failed: $url, status code: $statusCode, request body: $jsonBody");
+                throw new InvalidApPostException('Post failed', $url, $statusCode, $body);
             }
         } catch (\Exception $e) {
             $this->logRequestException($response, $url, 'ApHttpClient:post', $e);