Throw UnrecoverableMessageHandlingException on 4xx (#1235)

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,12 +16,18 @@
 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
 {
+    public const HTTP_RESPONSE_CODE_RATE_LIMITED = 429;
+
     public function __construct(
         private readonly EntityManagerInterface $entityManager,
         private readonly ApHttpClient $client,
@@ -53,8 +61,26 @@ public function workWrapper(MessageInterface $message): void
             $this->doWork($message);
             $conn->commit();
         } catch (InvalidApPostException $e) {
+            if (400 <= $e->responseCode && 500 > $e->responseCode && self::HTTP_RESPONSE_CODE_RATE_LIMITED !== $e->responseCode) {
+                $conn->rollBack();
+                $this->logger->debug('{domain} responded with {code} for our request, rolling back the changes and not trying again, request: {body}', [
+                    'domain' => $e->url,
+                    'code' => $e->responseCode,
+                    'body' => $e->payload,
+                ]);
+                throw new UnrecoverableMessageHandlingException('There is a problem with the request which will stay the same, so discarding', previous: $e);
+            } elseif (self::HTTP_RESPONSE_CODE_RATE_LIMITED === $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 +90,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);